From 89b6b32e3e0cefb241222a377586cdb9f4449ee2 Mon Sep 17 00:00:00 2001 From: Francesco Renzi Date: Wed, 6 May 2026 07:48:47 -0700 Subject: [PATCH] feedback --- src/Runner.Worker/Dap/DapDebugger.cs | 6 +---- src/Runner.Worker/Dap/IDapDebugger.cs | 1 + src/Runner.Worker/JobExtension.cs | 38 +++++++++++++-------------- src/Test/L0/Worker/JobExtensionL0.cs | 2 ++ 4 files changed, 23 insertions(+), 24 deletions(-) diff --git a/src/Runner.Worker/Dap/DapDebugger.cs b/src/Runner.Worker/Dap/DapDebugger.cs index 122c4c017..3b3ec7cbf 100644 --- a/src/Runner.Worker/Dap/DapDebugger.cs +++ b/src/Runner.Worker/Dap/DapDebugger.cs @@ -251,12 +251,10 @@ namespace GitHub.Runner.Worker.Dap { if (_jobContext != null) { - var cancellationToken = _jobContext.CancellationToken; - Trace.Info("Job completed — pausing for inspection"); SendStoppedEvent("completed", "Job completed — inspect variables before the session ends."); - await WaitForCommandAsync(cancellationToken); + await WaitForCommandAsync(_jobContext.CancellationToken); } } catch (Exception ex) @@ -274,8 +272,6 @@ namespace GitHub.Runner.Worker.Dap Trace.Warning($"DAP OnJobCompleted error: {ex.Message}"); } } - - await StopAsync(); } public async Task StopAsync() diff --git a/src/Runner.Worker/Dap/IDapDebugger.cs b/src/Runner.Worker/Dap/IDapDebugger.cs index 533626a42..07ebcab69 100644 --- a/src/Runner.Worker/Dap/IDapDebugger.cs +++ b/src/Runner.Worker/Dap/IDapDebugger.cs @@ -22,5 +22,6 @@ namespace GitHub.Runner.Worker.Dap Task OnStepStartingAsync(IStep step); void OnStepCompleted(IStep step); Task OnJobCompletedAsync(); + Task StopAsync(); } } diff --git a/src/Runner.Worker/JobExtension.cs b/src/Runner.Worker/JobExtension.cs index 059ef54fc..838009fc9 100644 --- a/src/Runner.Worker/JobExtension.cs +++ b/src/Runner.Worker/JobExtension.cs @@ -69,6 +69,7 @@ namespace GitHub.Runner.Worker List preJobSteps = new(); List jobSteps = new(); + var initSucceeded = false; using (var register = jobContext.CancellationToken.Register(() => { context.CancelToken(); })) { try @@ -494,20 +495,9 @@ namespace GitHub.Runner.Worker { _dapDebugger = HostContext.GetService(); await _dapDebugger.StartAsync(jobContext); - } - catch (Exception ex) - { - Trace.Error($"Failed to start DAP debugger: {ex.Message}"); - AddDebuggerConnectionTelemetry(jobContext, $"Failed: {ex.GetType().Name}"); - context.Error("Failed to start debugger."); - context.Result = TaskResult.Failed; - throw; - } - context.Output("Waiting for debugger client to connect…"); + context.Output("Waiting for debugger client to connect…"); - try - { await _dapDebugger.WaitUntilReadyAsync(); context.Output("Debugger connected."); AddDebuggerConnectionTelemetry(jobContext, "Connected"); @@ -517,19 +507,18 @@ namespace GitHub.Runner.Worker Trace.Info("Job was cancelled before debugger client connected."); AddDebuggerConnectionTelemetry(jobContext, "Canceled"); context.Error("Job was cancelled before debugger client connected."); - context.Result = TaskResult.Canceled; throw; } catch (Exception ex) { - Trace.Error($"DAP debugger failed to become ready: {ex.Message}"); + Trace.Error($"DAP debugger failed: {ex.Message}"); AddDebuggerConnectionTelemetry(jobContext, $"Failed: {ex.GetType().Name}"); context.Error("The debugger failed to start or no debugger client connected in time."); - context.Result = TaskResult.Failed; throw; } } + initSucceeded = true; return steps; } catch (OperationCanceledException ex) when (jobContext.CancellationToken.IsCancellationRequested) @@ -551,12 +540,12 @@ namespace GitHub.Runner.Worker finally { // If InitializeJob failed after the debugger was started, - // clean it up here since FinalizeJob won't run. - if (context.Result != null && _dapDebugger != null) + // tear down the transport here since FinalizeJob won't run. + if (!initSucceeded && _dapDebugger != null) { try { - await _dapDebugger.OnJobCompletedAsync(); + await _dapDebugger.StopAsync(); } catch (Exception ex) { @@ -867,7 +856,18 @@ namespace GitHub.Runner.Worker } catch (Exception ex) { - Trace.Warning($"DAP debugger cleanup error: {ex.Message}"); + Trace.Warning($"DAP debugger completion error: {ex.Message}"); + } + finally + { + try + { + await _dapDebugger.StopAsync(); + } + catch (Exception ex) + { + Trace.Warning($"DAP debugger stop error: {ex.Message}"); + } } _dapDebugger = null; } diff --git a/src/Test/L0/Worker/JobExtensionL0.cs b/src/Test/L0/Worker/JobExtensionL0.cs index d769d8b55..a286e62db 100644 --- a/src/Test/L0/Worker/JobExtensionL0.cs +++ b/src/Test/L0/Worker/JobExtensionL0.cs @@ -911,6 +911,7 @@ namespace GitHub.Runner.Common.Tests.Worker mockDebugger.Setup(x => x.StartAsync(It.IsAny())).Returns(Task.CompletedTask); mockDebugger.Setup(x => x.WaitUntilReadyAsync()).Returns(Task.CompletedTask); mockDebugger.Setup(x => x.OnJobCompletedAsync()).ThrowsAsync(new InvalidOperationException("tunnel disposed")); + mockDebugger.Setup(x => x.StopAsync()).Returns(Task.CompletedTask); hc.SetSingleton(mockDebugger.Object); _actionManager.Setup(x => x.PrepareActionsAsync(It.IsAny(), It.IsAny>(), It.IsAny())) @@ -922,6 +923,7 @@ namespace GitHub.Runner.Common.Tests.Worker await jobExtension.FinalizeJob(_jobEc, _message, DateTime.UtcNow); mockDebugger.Verify(x => x.OnJobCompletedAsync(), Times.Once); + mockDebugger.Verify(x => x.StopAsync(), Times.Once); } } }