From accfb1db26d265c890afecc5f526e1e7e1186c9a Mon Sep 17 00:00:00 2001 From: Resurrected Trader Date: Mon, 24 Aug 2026 21:50:18 +0100 Subject: [PATCH] fix: Survive a failed WebView2 init and a stale adopted routing entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two faults that surfaced together when a manager running 100+ profiles took an update: the successor came back with a grey window behind a "Failed to initialize WebView2 ... RPC_E_DISCONNECTED" dialog, and then relaunched every profile that was already running. WebView2 serves one user data folder from one browser process, and a client whose browser process goes away mid-initialization gets RPC_E_DISCONNECTED. The folder was a fixed path under %LOCALAPPDATA%, so every manager on the machine shared a browser process and whichever instance owned it could take the others' windows down when it exited. Handoff adds a second overlap the port cannot separate, since a successor deliberately inherits its predecessor's port. On top of that a single failure was terminal: OnFormLoad showed a MessageBox and gave up, leaving a permanently blank window over a server that was running fine. - Partition the user data folder by server port, so managers running side by side no longer share a browser process. Two managers cannot hold the same port concurrently, so it is a sound instance key. - Retry initialization four times with linear backoff, then offer Retry rather than only OK, and say plainly that the profiles are unaffected and the web UI is still reachable. Each retry runs on a fresh WebView2 control: a control binds the environment from its first EnsureCoreWebView2Async call and rejects a different one later, which is exactly what a retry needs to supply. The relaunches were a routing entry inherited across the handoff. A successor restores each profile's entry from the predecessor's manifest, so it inherits whatever the predecessor believed — and a predecessor built before the handle was tracked on the instance reverse-looked it up from a map that leaked a dead row per game exit, returning an arbitrary one. The adopted games then sent from a window the successor was not listening on, no heartbeats arrived, and the watchdog killed and relaunched all of them about a minute later. Fixing the lookup does not help here because the predecessor is the old build by definition, so the successor has to be able to recover on its own. - On a missed heartbeat, check the profile's routing entry against the window its game actually owns and re-register if they disagree, granting one interval before counting the miss. A wrong entry and a dead bot are indistinguishable to the watchdog, and killing a healthy game over one is the worst available response. Co-Authored-By: Claude Opus 5 (1M context) --- src/D2BotNG/Engine/ProfileEngine.cs | 62 ++++++++++++ src/D2BotNG/UI/MainForm.cs | 148 ++++++++++++++++++++++------ 2 files changed, 180 insertions(+), 30 deletions(-) diff --git a/src/D2BotNG/Engine/ProfileEngine.cs b/src/D2BotNG/Engine/ProfileEngine.cs index 651be40..2755b68 100644 --- a/src/D2BotNG/Engine/ProfileEngine.cs +++ b/src/D2BotNG/Engine/ProfileEngine.cs @@ -141,6 +141,58 @@ private void RegisterHandle(ProfileInstance instance, nint handle) /// of the manager. The sweep by name is belt-and-braces for an entry registered under a /// different handle (e.g. restored from a handoff manifest recording a drifted top-level). /// + /// + /// Re-points a profile's routing entry at the window its game actually owns, if the one we + /// registered has gone stale. Returns true when something was repaired. + /// + /// + /// A wrong routing entry and a dead bot look identical from the watchdog's side: no + /// heartbeats arrive either way. The difference is that a wrong entry is ours to fix, and + /// killing a healthy game over it is the worst possible response — at fleet scale it is a + /// mass restart a minute after an update. + /// + /// The case this exists for is adoption. A successor restores the routing entry from the + /// predecessor's manifest, so it inherits whatever the predecessor believed — and a + /// predecessor built before the handle was tracked on the instance reverse-looked it up out + /// of a map that leaked a dead row per game exit, returning an arbitrary one. Every update + /// from such a build hands its successor a handle that may name a window that no longer + /// exists, which is not something the successor can fix by being correct itself. + /// + /// + private bool RepairRoutingIfStale(ProfileInstance instance, Process process) + { + nint liveWindow; + try + { + liveWindow = process.GameWindow; + } + catch (Exception ex) + { + _logger.LogDebug(ex, "Could not read game window for {Name} while checking routing", instance.ProfileName); + return false; + } + + // No game window means there is nothing to route to — that is a real fault, not a + // routing problem, so let the watchdog handle it. + if (liveWindow == 0) return false; + + if (instance.GameWindowHandle == liveWindow + && _handleToProfile.TryGetValue(liveWindow, out var mapped) + && mapped == instance.ProfileName) + { + return false; + } + + _logger.LogWarning( + "Profile {Name} went quiet while routed to window {Registered}, but its game owns {Live} — " + + "re-registering; messages from it were being discarded", + instance.ProfileName, instance.GameWindowHandle, liveWindow); + + UnregisterHandles(instance); + RegisterHandle(instance, liveWindow); + return true; + } + private void UnregisterHandles(ProfileInstance instance) { if (instance.GameWindowHandle != 0) @@ -972,6 +1024,16 @@ private async Task MonitorProcessAsync(ProfileInstance instance, CancellationTok } var elapsed = (now - (instance.LastHeartbeat ?? instance.StartedAt!.Value)).TotalSeconds; + if (heartbeatEnabled && elapsed > heartbeatTimeout + && RepairRoutingIfStale(instance, process)) + { + // We were listening on the wrong window, so the silence says nothing about + // the bot. Give it another interval on the repaired route before counting a + // miss, and re-push our handle in case the game is still aimed elsewhere. + process.SendMessage((MessageType)_messageWindow.Handle, "Handle"); + elapsed = 0; + } + if (heartbeatEnabled && elapsed > heartbeatTimeout) { process.SendMessage((MessageType)_messageWindow.Handle, "Handle"); diff --git a/src/D2BotNG/UI/MainForm.cs b/src/D2BotNG/UI/MainForm.cs index d140ecf..3ac37a2 100644 --- a/src/D2BotNG/UI/MainForm.cs +++ b/src/D2BotNG/UI/MainForm.cs @@ -2,6 +2,7 @@ using D2BotNG.Core.Protos; using D2BotNG.Data; using D2BotNG.Engine; +using D2BotNG.Logging; using Microsoft.Web.WebView2.Core; using Microsoft.Web.WebView2.WinForms; using static D2BotNG.Windows.NativeMethods; @@ -12,13 +13,16 @@ namespace D2BotNG.UI; public class MainForm : Form { + private static readonly Serilog.ILogger Logger = TrackingLoggerFactory.ForContext(typeof(MainForm)); + // ReSharper disable InconsistentNaming — Win32 API constants private const int WM_SYSCOMMAND = 0x112; private const int SC_MINIMIZE = 0xF020; private const int WM_EXITSIZEMOVE = 0x232; // ReSharper restore InconsistentNaming - private readonly WebView2 _webView; + // Not readonly: a failed initialization is retried on a fresh control (see RecreateWebView). + private WebView2 _webView; private readonly NotifyIcon _trayIcon; private readonly ContextMenuStrip _trayMenu; private readonly string _serverUrl; @@ -109,42 +113,126 @@ private bool ShouldMinimizeToTray() } private async void OnFormLoad(object? sender, EventArgs e) + { + // Retried rather than fatal. One WebView2 user data folder is served by one browser + // process, and a client whose browser process goes away mid-initialization gets + // RPC_E_DISCONNECTED (0x80010108). Handoff creates exactly that window: the successor + // builds its environment while the predecessor is still unwinding and releasing its own. + // Failing once used to leave a permanently blank window over a perfectly healthy server. + const int maxAttempts = 4; + + while (true) + { + for (var attempt = 1; attempt <= maxAttempts; attempt++) + { + try + { + await InitializeWebViewAsync(); + return; + } + catch (Exception ex) when (attempt < maxAttempts) + { + Logger.Warning(ex, + "WebView2 initialization attempt {Attempt}/{Max} failed, retrying", attempt, maxAttempts); + + // Linear backoff: the predecessor's browser process needs long enough to + // finish exiting so our next attempt spawns a fresh one instead of + // attaching to the corpse. + await Task.Delay(TimeSpan.FromMilliseconds(750 * attempt)); + RecreateWebView(); + } + catch (Exception ex) + { + Logger.Error(ex, "WebView2 initialization failed after {Max} attempts", maxAttempts); + + var choice = MessageBox.Show( + $"Failed to initialize WebView2: {ex.Message}\n\n" + + "The bot manager itself is still running and your profiles are unaffected — " + + "this is only the window.\n\n" + + "Retry, or Cancel to keep running without the UI (the web interface is still " + + $"available at {_serverUrl}).", + "Error", + MessageBoxButtons.RetryCancel, + MessageBoxIcon.Error); + + if (choice != DialogResult.Retry) return; + RecreateWebView(); + } + } + } + } + + /// + /// Replaces the WebView2 control with a fresh one before an initialization retry. + /// + /// + /// A control remembers the environment handed to its first + /// EnsureCoreWebView2Async call and rejects a different one afterwards. Since a retry + /// exists precisely to get away from an environment whose browser process died, the control + /// has to go with it — otherwise the second attempt fails on the stale binding rather than + /// on whatever we were retrying. + /// + private void RecreateWebView() { try { - // Place WebView2's user data (cache, cookies, IndexedDB, crash dumps) under - // %LOCALAPPDATA%\D2BotNG\WebView2 instead of next to the exe (which is the - // default and litters the install directory with a *.exe.WebView2 folder). - var userDataFolder = Path.Combine( - Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), - "D2BotNG", - "WebView2"); - Directory.CreateDirectory(userDataFolder); - var env = await CoreWebView2Environment.CreateAsync(null, userDataFolder); - await _webView.EnsureCoreWebView2Async(env); - - // Configure WebView2 - _webView.CoreWebView2.Settings.IsStatusBarEnabled = false; - _webView.CoreWebView2.Settings.AreDevToolsEnabled = true; - - // Listen for messages from web app - _webView.CoreWebView2.WebMessageReceived += OnWebMessageReceived; - - // Navigate to server - _webView.CoreWebView2.Navigate(_serverUrl); - - // Bring window to front after WebView2 init - Activate(); - BringToFront(); + Controls.Remove(_webView); + _webView.Dispose(); } catch (Exception ex) { - MessageBox.Show( - $"Failed to initialize WebView2: {ex.Message}\n\nPlease ensure WebView2 Runtime is installed.", - "Error", - MessageBoxButtons.OK, - MessageBoxIcon.Error); + Logger.Warning(ex, "Failed to dispose WebView2 control before retry"); } + + _webView = new WebView2 { Dock = DockStyle.Fill }; + Controls.Add(_webView); + } + + private async Task InitializeWebViewAsync() + { + var env = await CoreWebView2Environment.CreateAsync(null, ResolveUserDataFolder()); + await _webView.EnsureCoreWebView2Async(env); + + // Configure WebView2 + _webView.CoreWebView2.Settings.IsStatusBarEnabled = false; + _webView.CoreWebView2.Settings.AreDevToolsEnabled = true; + + // Listen for messages from web app + _webView.CoreWebView2.WebMessageReceived += OnWebMessageReceived; + + // Navigate to server + _webView.CoreWebView2.Navigate(_serverUrl); + + // Bring window to front after WebView2 init + Activate(); + BringToFront(); + } + + /// + /// Where WebView2 keeps its cache, cookies, IndexedDB and crash dumps. + /// + /// + /// Under %LOCALAPPDATA%\D2BotNG rather than next to the exe, which is the default and + /// litters the install directory with a *.exe.WebView2 folder. + /// + /// Partitioned by server port so that several managers running side by side on one machine + /// don't share a browser process. They used to: one folder means one browser process serving + /// every instance, so whichever one owned it could take the others' windows down with it + /// when it exited. The port is the right key because two managers cannot run concurrently on + /// the same one — they'd fail to bind. A predecessor and its successor DO share a port, and + /// deliberately share the folder with it; that overlap is what the retry above covers. + /// + /// + private string ResolveUserDataFolder() + { + var port = new Uri(_serverUrl).Port; + var folder = Path.Combine( + Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), + "D2BotNG", + "WebView2", + port.ToString()); + Directory.CreateDirectory(folder); + return folder; } private void OnWebMessageReceived(object? sender, CoreWebView2WebMessageReceivedEventArgs e)