feat(debugger): Add event tracing and runtime diagnostics#185
feat(debugger): Add event tracing and runtime diagnostics#185MCGPPeters wants to merge 14 commits intomainfrom
Conversation
Implements Phase 1+2 of the debugger UX proposal: - Rewrite debugger.js (browser + server) with timeline scrubber, filterable event list, details panel, keyboard nav, and ARIA - Replace pipe-delimited bridge protocol with JSON bridge protocol using shared DebuggerAdapterProtocol DTOs - Add source-generated DebuggerAdapterJsonContext to fix JsonSerializerIsReflectionDisabled in WASM AOT/trimming - Move DebuggerRuntimeBridge to shared Picea.Abies.Debugger namespace so both Browser and Server can use it - Extend DebuggerMachine with PatchCount, AtStart/AtEnd, initial model capture, and previous model snapshot preview - Wire push notifications (CaptureMessage -> timeline changed callback -> JS notifyTimelineChanged) for live updates - Add DebuggerBridgeRegressionTests E2E test to guard against JSON serialization regression in WASM - Add debugger-ux-review-and-proposal.md investigation doc Root cause fixed: System.Text.Json.JsonSerializer.Serialize() threw JsonSerializerIsReflectionDisabled in WASM because reflection-based serialization is disabled under trimming. All bridge serialize calls now use DebuggerAdapterJsonContext (source-generated). All tests passing: 204 core + 36 browser + 18 server + 53 kestrel + 22 E2E = 333 total.
… nav to panel - Play button now rewinds to start when pressed at end of timeline (like a media player), fixing WASM where cursor is always at end in recording mode (atEnd=true silently aborted playback) - Keyboard shortcuts (Space, Arrow, Home, End) now only fire when focus is inside the debugger mount point, preventing the panel from stealing keypresses from the host application - Removed redundant external input guard (containment check covers it)
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| catch (Exception ex) | ||
| { | ||
| context.Response.StatusCode = StatusCodes.Status500InternalServerError; | ||
| await context.Response.WriteAsync($"Unexpected proxy error: {ex.GetType().Name}: {ex.Message}"); | ||
| activity?.SetStatus(ActivityStatusCode.Error, ex.Message); | ||
| logger?.LogError(ex, "OTLP proxy unexpected failure forwarding {SignalType} to {TargetUrl}", signalType, targetUrl); | ||
| } |
Check notice
Code scanning / CodeQL
Generic catch clause Note
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI about 18 hours ago
In general, the way to fix an overly generic catch clause is to either (a) narrow it to specific exception types you actually expect and know how to handle, or (b) filter out critical exceptions so they can bubble up while still handling non-critical ones. In ASP.NET Core top-level handlers, it is often appropriate to keep a final catch, but to avoid swallowing exceptions like OutOfMemoryException, StackOverflowException, or ThreadAbortException.
For this particular method in Picea.Abies.Server.Kestrel/OtlpProxyEndpoint.cs, the best fix without changing observable behavior is to keep the current 500-response behavior for unexpected "normal" exceptions, while explicitly rethrowing critical runtime exceptions. We can achieve this using a catch filter: catch (Exception ex) when (ex is not OutOfMemoryException && ...). This keeps the functionally useful "shield" against unexpected failures for callers while satisfying the analyzer that we are not blindly catching everything, and ensures truly catastrophic conditions are not incorrectly turned into an HTTP 500.
Concretely:
- Edit the
catch (Exception ex)block starting at line 312. - Change it to use a
whenfilter that excludes at leastOutOfMemoryException,StackOverflowException, andThreadAbortException(the classic "do not catch" set). No new imports are needed, these types are inSystem. - Leave the body of the catch unchanged so the existing logging and 500 response stay the same.
| @@ -309,7 +309,10 @@ | ||
| activity?.SetStatus(ActivityStatusCode.Error, "Timeout"); | ||
| logger?.LogWarning("OTLP proxy timeout forwarding {SignalType} to {TargetUrl}", signalType, targetUrl); | ||
| } | ||
| catch (Exception ex) | ||
| catch (Exception ex) when ( | ||
| ex is not OutOfMemoryException && | ||
| ex is not StackOverflowException && | ||
| ex is not ThreadAbortException) | ||
| { | ||
| context.Response.StatusCode = StatusCodes.Status500InternalServerError; | ||
| await context.Response.WriteAsync("An unexpected error occurred while forwarding the request."); |
There was a problem hiding this comment.
Pull request overview
This PR expands Abies’ debugger and observability surface across WASM + InteractiveServer by adding runtime/debugger diagnostics, improving event payloads (keyboard repeat), introducing a shared debugger adapter protocol/bridge, and enabling end-to-end OpenTelemetry flows (including WebSocket trace context propagation and OTLP proxy hardening), with added automated coverage.
Changes:
- Add shared debugger adapter protocol + runtime bridge (Debug-only) and extend runtime to capture post-transition snapshots / apply debugger snapshots.
- Improve browser/server event tracing (handler diagnostics, keyboard repeat) and OTEL export behavior (pinned CDN ESM + explicit export-on-end + OTLP proxy plumbing).
- Add/extend unit + E2E/template tests to cover debugger startup, timeline replay, trace propagation, and template defaults.
Reviewed changes
Copilot reviewed 101 out of 106 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/debugger-live-check.spec.js | New Playwright spec for debugger shell toggle |
| scripts/debugger-live-check.mjs | New standalone live-check runner script |
| docs/investigations/debugger-ux-review-and-proposal.md | UX review/proposal doc for debugger improvements |
| aspire.config.json | Adds Aspire AppHost path config |
| Picea.Abies/Runtime.cs | Capture post-transition snapshots; add snapshot apply hook |
| Picea.Abies/Html/EventData.cs | Add keyboard repeat to KeyEventData |
| Picea.Abies/HandlerRegistry.cs | Debug-only handler registry diagnostics |
| Picea.Abies/Debugger/DebuggerRuntimeBridge.cs | Shared runtime bridge for debugger adapter commands |
| Picea.Abies/Debugger/DebuggerAdapterProtocol.cs | Shared debugger JSON protocol + source-gen context |
| Picea.Abies.UI.Demo/wwwroot/index.html | Enable browser OTEL via meta tag |
| Picea.Abies.UI.Demo/wwwroot/abies.js | Add keyboard repeat + delegation debug logs |
| Picea.Abies.UI.Demo/wwwroot/abies-otel.js | Pin OTel CDN ESM + explicit export-on-end |
| Picea.Abies.Tests/DebuggerRuntimeReplayApplicationTests.cs | New runtime replay tests (Debug-only) |
| Picea.Abies.Tests/DebuggerRingBufferTests.cs | Update entries to include PatchCount |
| Picea.Abies.Tests/DebuggerMealyMachineTests.cs | Expand debugger machine coverage (AtStart/AtEnd/etc.) |
| Picea.Abies.Templates/templates/abies-server/Program.cs | Template OTEL setup + OTLP proxy + a11y labels |
| Picea.Abies.Templates/templates/abies-server/AbiesServerApp.csproj | Add OTel packages for template |
| Picea.Abies.Templates/templates/abies-browser/wwwroot/index.html | Enable browser OTEL via meta tag |
| Picea.Abies.Templates/templates/abies-browser/wwwroot/abies.js | Add keyboard repeat + delegation debug logs |
| Picea.Abies.Templates/templates/abies-browser/wwwroot/abies-otel.js | Pin OTel CDN ESM + explicit export-on-end |
| Picea.Abies.Templates/templates/abies-browser/Program.cs | Debugger enabled-by-default with opt-out env var |
| Picea.Abies.Templates/templates/abies-browser/AbiesApp.csproj | Exclude Host sources from WASM project build |
| Picea.Abies.Templates/templates/abies-browser/AbiesApp.Host/Program.cs | New host to serve AppBundle + OTLP proxy |
| Picea.Abies.Templates/templates/abies-browser/AbiesApp.Host/AbiesApp.Host.csproj | New host project with OTEL + publish target |
| Picea.Abies.Templates/templates/abies-browser-empty/wwwroot/index.html | Enable browser OTEL via meta tag |
| Picea.Abies.Templates/templates/abies-browser-empty/wwwroot/abies.js | Add keyboard repeat + delegation debug logs |
| Picea.Abies.Templates/templates/abies-browser-empty/wwwroot/abies-otel.js | Pin OTel CDN ESM + explicit export-on-end |
| Picea.Abies.Templates/templates/abies-browser-empty/Program.cs | Debugger enabled-by-default with opt-out env var |
| Picea.Abies.Templates/templates/abies-browser-empty/AbiesApp.csproj | Exclude Host sources from WASM project build |
| Picea.Abies.Templates/templates/abies-browser-empty/AbiesApp.Host/Program.cs | New host to serve AppBundle + OTLP proxy |
| Picea.Abies.Templates/templates/abies-browser-empty/AbiesApp.Host/AbiesApp.Host.csproj | New host project with OTEL + publish target |
| Picea.Abies.Templates.Testing/TemplateBuildTests.cs | Add template default + debugger asset tests |
| Picea.Abies.Templates.Testing/ServerTemplateTests.cs | Update locators to use accessible button names |
| Picea.Abies.Templates.Testing/BrowserTemplateTests.cs | Assert symbol labels for +/- buttons |
| Picea.Abies.Templates.Testing.E2E/ServerTemplateTests.cs | Add debugger startup/toggle + replay tests |
| Picea.Abies.Templates.Testing.E2E/Infrastructure/TemplateFixture.cs | NuGet restore config to avoid stale cache |
| Picea.Abies.Templates.Testing.E2E/Infrastructure/LocalNuGetFeed.cs | Pack debug configuration for local feed |
| Picea.Abies.Templates.Testing.E2E/Infrastructure/DotNetCli.cs | Allow pack configuration override |
| Picea.Abies.Templates.Testing.E2E/Helpers/InteractivityHelpers.cs | Update decrement fallback to - |
| Picea.Abies.Templates.Testing.E2E/BrowserTemplateTests.cs | Add debugger toggle + replay tests; update decrement |
| Picea.Abies.SubscriptionsDemo/wwwroot/index.html | Enable browser OTEL via meta tag |
| Picea.Abies.SubscriptionsDemo/wwwroot/abies.js | Add keyboard repeat + delegation debug logs |
| Picea.Abies.SubscriptionsDemo/wwwroot/abies-otel.js | Pin OTel CDN ESM + explicit export-on-end |
| Picea.Abies.Server/Transport.cs | Extend DomEvent with optional trace context |
| Picea.Abies.Server.Kestrel/wwwroot/abies-otel.js | New server-mode browser OTEL module (trace context) |
| Picea.Abies.Server.Kestrel/WebSocketTransport.cs | Deserialize/propagate trace context fields |
| Picea.Abies.Server.Kestrel/Picea.Abies.Server.Kestrel.csproj | Sync Debug-only debugger.js; exclude in Release |
| Picea.Abies.Server.Kestrel/OtlpProxyEndpoint.cs | OTLP endpoint resolution + forwarding updates |
| Picea.Abies.Server.Kestrel.Tests/WebSocketTransportTests.cs | Add tests for optional/new trace fields |
| Picea.Abies.Server.Kestrel.Tests/WebSocketSessionTests.cs | Add trace propagation + debugger command tests |
| Picea.Abies.Server.Kestrel.Tests/EndpointTests.cs | Assert debugger module served from /_abies/ |
| Picea.Abies.Presentation/wwwroot/abies.js | Add keyboard repeat + delegation debug logs |
| Picea.Abies.Presentation/wwwroot/abies-otel.js | Pin OTel CDN ESM + explicit export-on-end |
| Picea.Abies.Counter.Wasm/wwwroot/index.html | Enable browser OTEL via meta tag |
| Picea.Abies.Counter.Wasm/Program.cs | Debugger enabled-by-default with opt-out env var |
| Picea.Abies.Counter.Wasm.Host/Program.cs | Map OTLP proxy endpoint |
| Picea.Abies.Counter.Testing.E2E/DebuggerBridgeRegressionTests.cs | New WASM regression test for JSON bridge |
| Picea.Abies.Counter.Testing.E2E/CounterInteractivityTests.cs | Add debugger back/step replay test |
| Picea.Abies.Counter.Server/Program.cs | Map OTLP proxy endpoint |
| Picea.Abies.Conduit.Wasm/Program.cs | Debugger enabled-by-default with opt-out env var |
| Picea.Abies.Conduit.Wasm.Host/Program.cs | Map OTLP proxy endpoint |
| Picea.Abies.Conduit.Testing.E2E/Server/FeedServerTests.cs | Add server debugger toggle E2E test |
| Picea.Abies.Conduit.Testing.E2E/HealthTests.cs | Add WASM debugger toggle E2E test |
| Picea.Abies.Conduit.ServiceDefaults/Extensions.cs | Add Conduit-specific ActivitySources |
| Picea.Abies.Conduit.Server/Program.cs | Map OTLP proxy endpoint |
| Picea.Abies.Conduit.AppHost/Picea.Abies.Conduit.AppHost.csproj | Bump Aspire SDK + PostgreSQL hosting package |
| Picea.Abies.Conduit.App/Conduit.cs | Add otel-verbosity meta tag to document |
| Picea.Abies.Browser/wwwroot/abies.js | Add keyboard repeat + delegation debug logs |
| Picea.Abies.Browser/wwwroot/abies-otel.js | Pin OTel CDN ESM + explicit export-on-end |
| Picea.Abies.Browser/build/Picea.Abies.Browser.targets | Copy debugger.js from NuGet contentFiles |
| Picea.Abies.Browser/Runtime.cs | Ensure handler wiring precedes optional debugger bootstrap |
| Picea.Abies.Browser/Picea.Abies.Browser.csproj | Pack debugger.js into Browser NuGet contentFiles |
| Picea.Abies.Browser/Interop.cs | Add runtime bridge plumbing + debugger message dispatch |
| Picea.Abies.Browser/Debugger/DebuggerRuntimeBridge.cs | Remove browser-specific runtime bridge (moved shared) |
| Picea.Abies.Browser/Debugger/DebuggerAdapter.cs | Switch to shared protocol + source-gen serialization |
| Picea.Abies.Browser.Tests/DebuggerMountPointTests.cs | Update response DTO expectations |
| Picea.Abies.Browser.Tests/DebuggerAdapterTests.cs | Update adapter contract tests for new protocol |
| .vscode/settings.json | Add chat terminal timeout auto-approve setting |
| .squad/orchestration-log/2026-03-27T12-52-49Z-jsdev-wasm-input-regression.md | New orchestration log entry |
| .squad/orchestration-log/2026-03-26T14-22-00Z-otel-end-to-end-enabled.md | New orchestration log entry |
| .squad/orchestration-log/2026-03-26T13-35-00Z-websocket-otel-propagation.md | New orchestration log entry |
| .squad/orchestration-log/2026-03-26T12-58-08Z-scribe-session-maintenance.md | New orchestration log entry |
| .squad/log/2026-03-27T12-52-49Z-jsdev-wasm-input-regression.md | New session log entry |
| .squad/decisions/inbox/jsdev-timeline-debugger-fixes.md | New decision inbox entry |
| .squad/decisions/inbox/csharpdev-timeline-debugger-fixes.md | New decision inbox entry |
| .squad/decisions/inbox/beastmode-final-debugger-blockers.md | New decision inbox entry |
| .squad/decisions.md | Merge/record decisions and repo practices |
| .squad/agents/techwriter/history.md | Update agent history |
| .squad/agents/reviewer/history.md | Update agent history |
| .squad/agents/lead/history.md | Update agent history |
| .squad/agents/jsdev/history.md | Update agent history |
| .squad/agents/csharpdev/history.md | Update agent history |
| const reqPath = decodeURIComponent((req.url || '/').split('?')[0]); | ||
| const safePath = reqPath === '/' ? '/index.html' : reqPath; | ||
| const fullPath = path.normalize(path.join(root, safePath)); | ||
|
|
||
| if (!fullPath.startsWith(root) || !fs.existsSync(fullPath) || fs.statSync(fullPath).isDirectory()) { | ||
| res.writeHead(404, { 'content-type': 'text/plain; charset=utf-8' }); |
There was a problem hiding this comment.
Same issue as the spec: path.join(root, safePath) ignores root when safePath starts with /, so the file server will 404 all requests. Adjust safePath (remove the leading / or resolve via path.resolve(root, '.' + safePath)) before joining, to ensure the live-check script can load repo assets.
| using var forwardRequest = new HttpRequestMessage(HttpMethod.Post, targetUrl); | ||
| memoryStream.Position = 0; | ||
| forwardRequest.Content = new StreamContent(memoryStream); | ||
| forwardRequest.Content.Headers.ContentType = | ||
| new System.Net.Http.Headers.MediaTypeHeaderValue(contentType!); | ||
| using var forwardContent = new ByteArrayContent(memoryStream.ToArray()); | ||
| if (!forwardContent.Headers.TryAddWithoutValidation("Content-Type", contentType)) | ||
| { | ||
| forwardContent.Headers.ContentType = System.Net.Http.Headers.MediaTypeHeaderValue.Parse(contentType); | ||
| } | ||
| forwardRequest.Content = forwardContent; |
There was a problem hiding this comment.
Forwarding the OTLP request body via new ByteArrayContent(memoryStream.ToArray()) forces an extra full-buffer allocation/copy per request (and can be large for trace payloads). Prefer streaming the existing MemoryStream (e.g., StreamContent after rewinding) and set the Content-Type header with TryAddWithoutValidation/MediaTypeHeaderValue.Parse to avoid the header validation issue without copying the payload.
| catch (HttpRequestException ex) | ||
| { | ||
| context.Response.StatusCode = StatusCodes.Status502BadGateway; | ||
| await context.Response.WriteAsync($"Failed to forward to collector: {ex.Message}"); | ||
| activity?.SetStatus(ActivityStatusCode.Error, ex.Message); | ||
| logger?.LogWarning(ex, "OTLP proxy failed to forward {SignalType} to {TargetUrl}", signalType, targetUrl); | ||
| } | ||
| catch (OperationCanceledException) when (!context.RequestAborted.IsCancellationRequested) | ||
| { | ||
| context.Response.StatusCode = StatusCodes.Status504GatewayTimeout; | ||
| await context.Response.WriteAsync("Collector did not respond within timeout."); | ||
| activity?.SetStatus(ActivityStatusCode.Error, "Timeout"); | ||
| logger?.LogWarning("OTLP proxy timeout forwarding {SignalType} to {TargetUrl}", signalType, targetUrl); | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| context.Response.StatusCode = StatusCodes.Status500InternalServerError; | ||
| await context.Response.WriteAsync($"Unexpected proxy error: {ex.GetType().Name}: {ex.Message}"); | ||
| activity?.SetStatus(ActivityStatusCode.Error, ex.Message); | ||
| logger?.LogError(ex, "OTLP proxy unexpected failure forwarding {SignalType} to {TargetUrl}", signalType, targetUrl); | ||
| } |
There was a problem hiding this comment.
The proxy currently returns exception type/name/message to the caller (both for forwarding failures and the generic catch (Exception) path). This can leak internal details to unauthenticated clients and also makes responses inconsistent across failure modes. Consider returning a generic error body (or no body) and rely on structured logging/activity status for diagnostics.
Picea.Abies.Server/Transport.cs
Outdated
| public readonly record struct DomEvent( | ||
| string CommandId, | ||
| string EventName, | ||
| string EventData, | ||
| string? TraceParent = null, | ||
| string? TraceState = null); |
There was a problem hiding this comment.
This record parameter list uses tab indentation, which is inconsistent with the surrounding formatting and is likely to get rewritten by dotnet format / .editorconfig. Reformat the parameter indentation to the repo’s standard spacing to avoid noisy diffs and formatting CI failures.
.vscode/settings.json
Outdated
| "chatgpt.openOnStartup": true, | ||
| "chat.tools.terminal.autoApprove": { | ||
| "timeout": true | ||
| } |
There was a problem hiding this comment.
Repo-level VS Code settings enabling terminal "auto approve" can change developer tooling behavior in ways that are hard to audit and may not be appropriate for all contributors. Consider removing this from the committed workspace settings (or scoping it to personal/user settings) unless the team has explicitly agreed to enforce it repo-wide.
| "chatgpt.openOnStartup": true, | |
| "chat.tools.terminal.autoApprove": { | |
| "timeout": true | |
| } | |
| "chatgpt.openOnStartup": true |
| } finally { | ||
| await browser.close(); | ||
| server.close(); | ||
| } No newline at end of file |
There was a problem hiding this comment.
server.close() is not awaited here, so the script can exit before the HTTP server has fully closed (and can leave the event loop open in some environments). Wrap server.close in a Promise and await it (similar to the listen call) for deterministic cleanup.
| const reqPath = decodeURIComponent((req.url || '/').split('?')[0]); | ||
| const safePath = reqPath === '/' ? '/index.html' : reqPath; | ||
| const fullPath = path.normalize(path.join(root, safePath)); | ||
|
|
||
| if (!fullPath.startsWith(root) || !fs.existsSync(fullPath) || fs.statSync(fullPath).isDirectory()) { |
There was a problem hiding this comment.
path.join(root, safePath) will ignore root because safePath always starts with /, causing fullPath to resolve from the filesystem root and the subsequent startsWith(root) guard to 404 every request. Strip the leading slash (or use path.resolve(root, . + safePath)) so files under the repo can actually be served, while keeping the traversal protection intact.
MCGPPeters
changed the title
📝 Description
What
Add debugger/runtime diagnostics and browser/server event tracing updates, including keyboard event payload improvements and handler-level instrumentation.
Why
Intermittent event handling needed deeper observability across client delegation and server handler resolution. This adds traceability and test coverage for debugger session behaviors.
How
🔗 Related Issues
N/A
✅ Type of Change
🧪 Testing
Test Coverage
Testing Details
dotnet build -c Debugsuccessfully✨ Changes Made
🔍 Code Review Checklist