Skip to content

Allow to Install When Closing Revit #137

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
vnoves merged 1 commit into from
Nov 11, 2025
Merged

Allow to Install When Closing Revit #137

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions Common_glTF_Exporter/ExternalApplication.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
using Autodesk.Windows;
using Common_glTF_Exporter;
using Common_glTF_Exporter.Service;
using Common_glTF_Exporter.Version;
using System;
using System.IO;
using System.Linq;
Expand All @@ -25,6 +26,7 @@ public class ExternalApplication : IExternalApplication
private static string pushButtonText = "Leia";
private static string addInPath = typeof(ExternalApplication).Assembly.Location;
private static string buttonIconsFolder = Path.GetDirectoryName(addInPath) + "\\Images\\";
public static string InstallerRoute;

internal Document Document { get; set; }
public static UIApplication UiApp { get; private set; }
Expand Down Expand Up @@ -57,6 +59,10 @@ public static void CreateRibbonTab(UIControlledApplication application, string r
/// <returns>Result.</returns>
public Result OnShutdown(UIControlledApplication application)
{
if (!string.IsNullOrEmpty(InstallerRoute))
{
RunLocalFile.Action(InstallerRoute);
}
Comment on lines +62 to +65
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Add error handling and consider timing implications.

The installer execution on shutdown has several concerns:

  1. Timing: Launching the installer immediately during shutdown may fail if Revit hasn't fully released file locks on its assemblies.
  2. No error handling: If RunLocalFile.Action fails silently, users won't know why the update didn't occur.
  3. State cleanup: The InstallerRoute field is never cleared, which could cause issues if checked in future sessions.

Consider this improved implementation:

 public Result OnShutdown(UIControlledApplication application)
 {
     if (!string.IsNullOrEmpty(InstallerRoute))
     {
-        RunLocalFile.Action(InstallerRoute);
+        try
+        {
+            RunLocalFile.Action(InstallerRoute);
+        }
+        catch (Exception ex)
+        {
+            TaskDialog.Show("Update Error", 
+                $"Failed to launch installer: {ex.Message}");
+        }
+        finally
+        {
+            InstallerRoute = null;
+        }
     }
     return Result.Succeeded;
 }

Verify that the installer can successfully launch while Revit is shutting down by testing on multiple machines with different configurations.

🏁 Script executed:

#!/bin/bash
# Find and inspect ExternalApplication.cs file
fd -t f -i "ExternalApplication.cs" --exec cat {} \;

Length of output: 9126

🏁 Script executed:

#!/bin/bash
# Search for RunLocalFile class and its Action method
rg -nP "class\s+RunLocalFile|\.Action\s*\(" --type=cs -A 5

Length of output: 1270

🏁 Script executed:

#!/bin/bash
# Find all uses of InstallerRoute field
rg -nP "InstallerRoute" --type=cs -B 3 -A 3

Length of output: 2098

🏁 Script executed:

#!/bin/bash
# Search for error handling patterns in shutdown/plugin code
rg -nP "OnShutdown|try\s*\{" --type=cs -A 10 | head -80

Length of output: 1837

🏁 Script executed:

#!/bin/bash
# Look for TaskDialog usage (showing how errors are reported elsewhere)
rg -nP "TaskDialog\." --type=cs -B 2 -A 2 | head -60

Length of output: 527

🏁 Script executed:

#!/bin/bash
# Get full RunLocalFile.cs implementation
fd -t f "RunLocalFile.cs" --exec cat {} \;

Length of output: 323

🏁 Script executed:

#!/bin/bash
# Check VersionWindow context - when/how InstallerRoute is set
rg -nP "VersionWindow" --type=cs -A 10 -B 5 | head -100

Length of output: 3216

🏁 Script executed:

#!/bin/bash
# Look for Revit shutdown documentation references or comments about OnShutdown
rg -nP "OnShutdown|shutdown|Revit.*release|file.*lock" --type=cs -i | head -40

Length of output: 4984

🏁 Script executed:

#!/bin/bash
# Check IExternalApplication contract and OnShutdown requirements
rg -nP "IExternalApplication|Result.Succeeded" --type=cs -A 3 -B 1 | head -50

Length of output: 2239

Add error handling and clear field after use.

The installer execution on shutdown lacks proper error handling and state cleanup:

  1. Missing error handling: RunLocalFile.Action calls Hyperlink.Run() without catching exceptions. If the installer fails to launch, the error goes unreported.
  2. No state cleanup: The static InstallerRoute field is never cleared after use, which could cause unintended behavior in subsequent sessions.

Wrap the call in try-catch-finally to handle failures gracefully and clear the field:

 public Result OnShutdown(UIControlledApplication application)
 {
     if (!string.IsNullOrEmpty(InstallerRoute))
     {
-        RunLocalFile.Action(InstallerRoute);
+        try
+        {
+            RunLocalFile.Action(InstallerRoute);
+        }
+        catch (Exception ex)
+        {
+            TaskDialog.Show("Update Error", 
+                $"Failed to launch installer: {ex.Message}");
+        }
+        finally
+        {
+            InstallerRoute = null;
+        }
     }
     return Result.Succeeded;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (!string.IsNullOrEmpty(InstallerRoute))
{
RunLocalFile.Action(InstallerRoute);
}
if (!string.IsNullOrEmpty(InstallerRoute))
{
try
{
RunLocalFile.Action(InstallerRoute);
}
catch (Exception ex)
{
TaskDialog.Show("Update Error",
$"Failed to launch installer: {ex.Message}");
}
finally
{
InstallerRoute = null;
}
}
🤖 Prompt for AI Agents 
In Common_glTF_Exporter/ExternalApplication.cs around lines 62 to 65, the
installer launch lacks exception handling and never clears the static
InstallerRoute; wrap the RunLocalFile.Action(InstallerRoute) call in a
try-catch-finally: in try call RunLocalFile.Action, in catch log or report the
exception (include exception message and context) so failures are visible, and
in finally clear the InstallerRoute field (set to null or empty) to remove state
after use.

return Result.Succeeded;
}

Expand Down
21 changes: 20 additions & 1 deletion Common_glTF_Exporter/Windows/VersionWindow.xaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,31 @@
</Button.Template>
</Button>

<Button Content="Update"
<StackPanel Orientation="Horizontal"
HorizontalAlignment="Center"
VerticalAlignment="Bottom">

<Button Content="Update on Exit"
Margin="10,0,10,13"
Click="Button_UpdateOnExit"
Height="30"
Width="100"
Background="Transparent"
Foreground="{DynamicResource WindowButtonColor}"
Style="{DynamicResource MainButtonStyle}"
VerticalAlignment="Bottom"/>

<Button Content="Update Now"
Margin="10,0,10,13"
Click="Button_ClickAsync"
Width="100"
Height="30"
Style="{DynamicResource MainButtonStyle}"
VerticalAlignment="Bottom"/>

</StackPanel>


</Grid>
</Border>

Expand Down
28 changes: 23 additions & 5 deletions Common_glTF_Exporter/Windows/VersionWindow.xaml.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,11 +30,7 @@ private async void Button_ClickAsync(object sender, RoutedEventArgs e)
{
this.Hide();

string tempPath = Path.GetTempPath();
string tempFolder = System.IO.Path.Combine(tempPath, "LeiaInstaller");
string versionFolder = System.IO.Path.Combine(tempFolder, _fileVersion);
DirectoryUtils.CreateDirectoryIfNotExists(versionFolder);
DirectoryUtils.DeleteFilesInDirectoyy(versionFolder);
string versionFolder = VersionFolder();
await DownloadFile.FromServer(versionFolder, _installerName);

string fileLocation = System.IO.Path.Combine(versionFolder, _installerName);;
Expand All @@ -43,6 +39,28 @@ private async void Button_ClickAsync(object sender, RoutedEventArgs e)
Close();
}


private async void Button_UpdateOnExit(object sender, RoutedEventArgs e)
{
this.Hide();
string versionFolder = VersionFolder();
await DownloadFile.FromServer(versionFolder, _installerName);

string fileLocation = System.IO.Path.Combine(versionFolder, _installerName);
ExternalApplication.InstallerRoute = fileLocation;
Close();
}
Comment on lines +43 to +52
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Critical timing issue: Window closes before download completes.

The window is hidden immediately (line 45) and then closed (line 51), but the download is awaited in between (line 47). This creates several problems:

  1. User confusion: No feedback that download is happening in background
  2. Race condition: If user closes Revit before download completes, installer won't be available to run on shutdown
  3. Silent failures: If download fails, user receives no notification

Consider one of these approaches:

Option 1 (Recommended): Keep window visible with progress feedback:

 private async void Button_UpdateOnExit(object sender, RoutedEventArgs e)
 {
-    this.Hide();
+    labelMessage.Text = "Downloading installer...";
+    CloseButton.IsEnabled = false;
     string versionFolder = VersionFolder();
     await DownloadFile.FromServer(versionFolder, _installerName);

     string fileLocation = System.IO.Path.Combine(versionFolder, _installerName);
     ExternalApplication.InstallerRoute = fileLocation;
+    labelMessage.Text = "Update will install when you close Revit.";
+    await Task.Delay(2000); // Brief confirmation
     Close();
 }

Option 2: Start download as fire-and-forget background task (not recommended due to reliability concerns):

 private void Button_UpdateOnExit(object sender, RoutedEventArgs e)
 {
     string versionFolder = VersionFolder();
-    await DownloadFile.FromServer(versionFolder, _installerName);
-
-    string fileLocation = System.IO.Path.Combine(versionFolder, _installerName);
-    ExternalApplication.InstallerRoute = fileLocation;
+    Task.Run(async () => {
+        await DownloadFile.FromServer(versionFolder, _installerName);
+        string fileLocation = System.IO.Path.Combine(versionFolder, _installerName);
+        ExternalApplication.InstallerRoute = fileLocation;
+    });
     Close();
 }

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In Common_glTF_Exporter/Windows/VersionWindow.xaml.cs around lines 43 to 52, the
window is hidden immediately and then closed while awaiting the download which
can confuse users and cause race conditions; change the flow to keep the window
visible and block close until download completes by disabling the window
controls (or showing a modal progress dialog), start the awaited download with
progress reporting, update a progress UI element as bytes arrive, handle
exceptions from DownloadFile.FromServer to show a user-facing error message (and
do not set ExternalApplication.InstallerRoute or close the window on failure),
and only set ExternalApplication.InstallerRoute and close the window after a
successful download completes; ensure UI updates are marshaled to the UI thread
and the Close/Hide calls occur after success.


private static string VersionFolder()
{
string tempPath = Path.GetTempPath();
string tempFolder = System.IO.Path.Combine(tempPath, "LeiaInstaller");
string versionFolder = System.IO.Path.Combine(tempFolder, _fileVersion);
DirectoryUtils.CreateDirectoryIfNotExists(versionFolder);
DirectoryUtils.DeleteFilesInDirectoyy(versionFolder);
return versionFolder;
}
Comment on lines +54 to +62
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Add error handling for directory operations.

The VersionFolder method performs file system operations without error handling. If directory creation or file deletion fails (e.g., due to permissions or locked files), the exception will bubble up unhandled.

Add try-catch to provide graceful error handling:

 private static string VersionFolder()
 {
-    string tempPath = Path.GetTempPath();
-    string tempFolder = System.IO.Path.Combine(tempPath, "LeiaInstaller");
-    string versionFolder = System.IO.Path.Combine(tempFolder, _fileVersion);
-    DirectoryUtils.CreateDirectoryIfNotExists(versionFolder);
-    DirectoryUtils.DeleteFilesInDirectoyy(versionFolder);
-    return versionFolder;
+    try
+    {
+        string tempPath = Path.GetTempPath();
+        string tempFolder = System.IO.Path.Combine(tempPath, "LeiaInstaller");
+        string versionFolder = System.IO.Path.Combine(tempFolder, _fileVersion);
+        DirectoryUtils.CreateDirectoryIfNotExists(versionFolder);
+        DirectoryUtils.DeleteFilesInDirectoyy(versionFolder);
+        return versionFolder;
+    }
+    catch (Exception ex)
+    {
+        TaskDialog.Show("Update Error", 
+            $"Failed to prepare installer directory: {ex.Message}");
+        throw;
+    }
 }

Note: There's a typo in DirectoryUtils.DeleteFilesInDirectoyy (should be "Directory"), but that's outside this PR's scope.

Committable suggestion skipped: line range outside the PR's diff.


private void Close_Click(object sender, RoutedEventArgs e)
{
this.Close();
Expand Down
Loading