Skip to content

Improve crash diagnostics: capture throwing-site stacks, all-thread dumps, and operation context so unobserved-fault crashes (e.g. #1350) are diagnosable #1351

Description

@JoshuaRowePhantom

Summary

Issue #1350 was an unobserved TaskScheduler fault (AggregateExceptionSocketException 995 from TcpClient.CompleteConnectAsync) whose captured stack contained ONLY the socket-teardown frames. Crash records today capture only the faulting exception''s teardown stack; there is no throwing-site stack, no thread snapshot, and no task/operation context. That makes it very hard to answer "which component/operation caused this?" for unobserved-task and shutdown-race crashes among many possible network/tunnel/transport sites. This issue proposes a phased set of concrete, mostly-first-party .NET diagnostics improvements so crashes like #1350 carry enough context to localize — under a hard no-user-data constraint (see next section).

Privacy / Data-Handling Requirements (hard constraint)

Crash diagnostics MUST NOT capture or persist any user data. The only things we want to track are (1) the KINDS of tasks/operations that might be running, and (2) THREAD STACKS — expressed as code identity only. Every capability below is designed to satisfy this constraint; anything that could carry user data is either excluded or gated behind an explicit, clearly-labeled, user-initiated export (never default/automatic).

ALLOWED (no user data):

  • Symbolic thread stack traces — per-frame DeclaringType.FullName + MethodName only (code identity, no argument values, no locals, no memory).
  • KINDS/categories of operations in flight — category strings + counts (e.g. "TunnelRelay.Connect" → 3).
  • Exception TYPE names (flattened AggregateException type chain).
  • Process id, managed thread ids.
  • Task-id / causality correlation (ids only, no payload).
  • Timestamps.

DISALLOWED (may contain user data):

  • Full-memory minidumps (MiniDumpWithFullMemory) and any raw memory dump that includes stack/heap memory — locals/strings can contain entity content, tokens, hostnames, or file paths.
  • Operation-scope TAGS carrying values (e.g. host=…, URLs, entity names, workspace/agent/tunnel identifiers). Operation scopes are category-only.
  • Exception.Data contents.
  • Exception MESSAGE text when it may embed user data (paths, hostnames, entity names). Capture the exception TYPE and STACK; treat message text as potentially-sensitive and omit or redact by default.
  • App-state snapshots that name workspaces/agents/tunnels.
  • Recent-log-tail inclusion in crash records.
  • System.Net.Sockets EventSource payload fields that carry remote endpoints/hostnames — capture kind + correlation only.

What we capture today

  • Handlers: features\Phantom.Workspaces\UnhandledExceptionHandler.csInstall() hooks AppDomain.CurrentDomain.UnhandledException (:18) and TaskScheduler.UnobservedTaskException (:19); InstallDispatcherHandler() hooks Avalonia Dispatcher.UIThread.UnhandledException (:24). OnUnobservedTaskException (:30-34) calls e.SetObserved() then ShowOrDiscard (dialog only, no logging here). ShowOrDiscard (:44-62) is a single-dialog guard + posts CrashDialog. Wired at Program.cs:24 and App.axaml.cs:223/233; also Agent.Cli/Program.cs:12, Agent.Gui/Program.cs:19, Web.Server/Program.cs:49.
  • Persistence: features\Phantom.Workspaces.Logging\GlobalExceptionLogging.cs is the ONLY persistence path — independently subscribes AppDomain/Unobserved (:37-38); Log (:70-83) flattens AggregateException and passes the exception object to ILogger (full .ToString() preserved by the sink). Registered at App.axaml.cs:271.
  • Sink: RollingFileLoggerProvider writes phantom-workspaces-yyyyMMdd.log (7-day retention, LoggingBootstrap.cs:16) in the config-driven LogDirectory (Services\Logging\LogDirectoryProvider.cs, default under %LOCALAPPDATA%\Phantom.Workspaces\…\logs). Plain text, no structured/JSON, no scopes honored.
  • CrashDialog: features\Phantom.Workspaces\Controls\CrashDialog.axaml.cs shows full exception.ToString() (:22); "Report" builds a GitHub issues/new URL truncated to 1400 chars (:47-68).
  • Dumps/trace: NONE in production code (grep for MiniDump|EventPipe|EventSource|EventListener|FirstChanceException|createdump returns zero prod hits). Only CI hang-dumps via scripts\run-tests.ps1:62-65 --blame-hang. No dedicated crashes\ folder.
  • KEY LIMITATION for Crash: AggregateException: A Task's exception(s) were not observed either by Waiting on the Task or accessing its Exception property. As a result, the unobserved exception was rethrown by the finalizer thread. (The I/O operation has been aborted because of #1350: an UnobservedTaskException only carries the completion/teardown frames — the abandoned task''s CREATION stack (which would name the TcpClient owner) is already gone by the time the finalizer surfaces it.

Proposed capabilities

Reframed around the owner-stated goal — (1) KINDS of tasks that might be running, (2) symbolic THREAD STACKS — under the no-user-data constraint.

Gap today Proposed capability (privacy-safe) Revealed for #1350 (no user data)
Only teardown frames; no throwing-site stack Scoped AppDomain.CurrentDomain.FirstChanceException capture of exception TYPE + throwing STACK + active operation-KIND (message redacted/omitted; reentrancy-guarded; rate-limited; type filter) Symbolic throwing stack (method identities) at the TcpClient.ConnectAsync throw site
No ambient "who started this" context Operation-KIND registryAsyncLocal-based scopes carry ONLY a category string (e.g. "TunnelRelay.Connect", "Transport.Accept", "Mongo.Query"); a concurrent Dictionary<string kind, int count> is incremented on scope enter and decremented on exit Active operation-kind = TunnelRelay.Connect with count > 0 at the fault instant
No thread snapshot Symbolic all-thread stack capture — per-frame DeclaringType.FullName + MethodName only, serialized into the crash record (e.g. via ClrMD self-inspection or equivalent stackwalk). A full memory .dmp is DISALLOWED by default; if ever wanted it must be an explicit, user-initiated, clearly-labeled opt-in export Symbolic stack of the socket-owning thread (method identities only)
No task/socket event trace In-proc EventListener ring buffer on System.Threading.Tasks.TplEventSource (task ids + causality only — no payload with user data); for System.Net.Sockets, capture ONLY event kind + correlation, explicitly EXCLUDING remote endpoint/host fields TplEventSource task-id chain to the creator of the abandoned task; a correlated socket-kind event (no host)
No dedicated crash record crashes\crash-{ts}-{pid}.txt restricted to the ALLOWED set: exception TYPE(s) (flattened AggregateException chain), symbolic all-thread stacks, active operation-kinds+counts, PID, managed TIDs, task-correlation ids, timestamps, correlation id Structured, privacy-safe, self-contained artifact suitable to attach
CrashDialog Report URL truncated at 1400 chars Surface the crash-folder path (privacy-safe artifact) instead of raw stack in URL User can inspect / attach the privacy-safe crash record

Feasibility notes

  • There is no supported public API to enumerate all live Task objects; the supported route for task causality is the TplEventSource ETW/EventPipe stream (in-proc EventListener). Stated explicitly so no one tries to enumerate tasks.
  • DOTNET_DbgEnableMiniDump/createdump env vars only fire on TERMINATING faults, not on the SetObserved() unobserved path — so an in-proc symbolic stackwalk is needed for the unobserved case.
  • All proposed techniques are first-party / P-Invoke, net10.0 Windows + Avalonia, no admin required.
  • FirstChanceException fires on EVERY throw (incl. caught/expected) — MUST be scoped/filtered/rate-limited and reentrancy-safe.
  • A full-memory dump is DISALLOWED for privacy reasons; symbolic stacks are the privacy-preserving substitute. Even a stacks-only/triage minidump can contain user strings in stack memory, so symbolic-frames-only is the safe default. If a minidump is ever mentioned in code, it must be restricted to a triage/stacks-only variant AND gated behind an explicit user-initiated action — never automatic.

Design / Fix (phased plan)

Phase 1 (cheap, first-party, privacy-safe by construction):

  1. Dedicated crash-record file under {LogDirectory}\crashes containing ONLY the ALLOWED set (exception TYPE chain, PID/TIDs, task-correlation ids, timestamps, correlation id, active operation-kinds + counts). No Exception.Data, no app-state names, no log-tail, no unredacted messages.
  2. Operation-KIND registryAsyncLocal category-only scopes + a concurrent kind→count map — surfaced in GlobalExceptionLogging.Log.
  3. Scoped FirstChanceException logging — exception TYPE + throwing STACK + active operation-kind; message text redacted/omitted; reentrancy-guarded; rate-limited; type filter.

Phase 2 (higher value, more work):
4. Symbolic all-thread stack capture (per-frame DeclaringType.FullName + MethodName only) written to the crash folder on unobserved/appdomain faults. NO memory dump.
5. In-proc TplEventSource (task ids + causality only) + System.Net.Sockets (kind + correlation only, remote endpoints excluded) EventListener ring buffer flushed to the crash folder.
6. CrashDialog shows the crash-folder path and links to it (replace URL truncation).

Files to change:

  • UnhandledExceptionHandler.cs (:16 add FirstChanceException hook; :44 ShowOrDiscard calls ICrashDiagnostics.CaptureAsync)
  • GlobalExceptionLogging.cs (:57 / :70 enrich with active operation-kinds + counts; invoke crash-record writer)
  • App.axaml.cs (:271 construct/register CrashDiagnostics; crash folder = Path.Combine(logDirectoryProvider.LogDirectory,"crashes"))
  • Controls\CrashDialog.axaml.cs (:22 / :47 show + open crash folder path)

Files to add:

  • Phantom.Workspaces.Logging\ICrashDiagnostics.cs + CrashDiagnosticsService.cs — writes the crash record and symbolic all-thread stacks (no memory dump); owns the ring buffer.
  • OperationScope.csAsyncLocal push/pop of a category string only + kind→count map; CurrentKinds() returns kind+count pairs (no values).
  • TplEventRingBuffer.csEventListener, bounded circular buffer; retains only ids/causality/kinds (no payload user-data fields).

Considered / Rejected (privacy-excluded)

The following were considered but EXCLUDED from default/automatic crash capture to satisfy the no-user-data requirement. They may only be captured via an explicit, clearly-labeled, user-initiated export — never as part of default crash capture:

  • Full-memory MiniDumpWithFullMemory auto-capture — contains stack/heap memory (locals, strings, tokens, hostnames, file paths). Replaced by symbolic-frames-only capture.
  • Operation tags with valueshost=…, URLs, entity names, workspace/agent/tunnel identifiers. Replaced by category-only kinds + counts.
  • Exception.Data contents — arbitrary user data.
  • Recent-log-tail inclusion in crash records — may contain any prior logged user data.
  • App-state snapshots naming workspaces/agents/tunnels.
  • Raw socket endpoint fields on System.Net.Sockets events — remote hostnames/IPs. Replaced by kind + correlation only.
  • Unredacted exception messages — may embed paths/hostnames/entity names. Message text is redacted/omitted; exception TYPE + STACK captured instead.

Expected Tests

Existing seams are sufficient: UnhandledExceptionHandler.ShowCrashDialogAsync is a replaceable Func (:13), _dialogActive internal (:10), GlobalExceptionLogging.ResetForTests() (:87); tests use Avalonia.Headless.XUnit; style Subject_Scenario_ExpectedOutcome, e.g. existing ShowOrDiscard_WhenNoDialogActive_SetsDialogActiveFlag, RollingFileLogger_WritesEntry_CreatesFileInResolvedLogDirectory. Introduce ICrashDiagnostics with a fake writing to a temp dir so tests avoid real P/Invoke and real dialogs. Tests lock in the privacy behavior.

Test Name Class What It Verifies
CrashDiagnostics_OnUnobservedTaskException_WritesSymbolicStacksAndOperationKinds_NoUserData CrashDiagnosticsServiceTests (new) Crash record contains symbolic frames + operation kinds/counts and NO argument values, entity/tunnel names, Exception.Data, or log content
OperationScope_CarriesCategoryOnly_DoesNotRecordArgumentValues GlobalExceptionLoggingTests Operation scopes expose only a category string; no value tags are captured; kind→count map reflects active scopes
CrashRecord_ExcludesExceptionMessageWhenPotentiallySensitive_CapturesTypeAndStack CrashDiagnosticsServiceTests (new) The record captures exception TYPE + symbolic stack and omits/redacts message text
CrashDiagnostics_DoesNotWriteFullMemoryDumpByDefault CrashDiagnosticsServiceTests (new) No full-memory .dmp is produced by automatic crash capture; the crash folder contains only the privacy-safe record
TplEventRingBuffer_ForSocketEvents_ExcludesRemoteEndpointFields CrashDiagnosticsServiceTests (new) Socket events are captured by kind/correlation only, explicitly excluding remote endpoint/host data
FirstChanceHandler_WhenSocketExceptionThrown_LogsSymbolicStackAndOperationKind_MessageRedacted UnhandledExceptionHandlerTests Scoped first-chance SocketException logs exception TYPE + symbolic throwing stack + active operation-kind; message text is redacted/omitted
OperationScope_WhenTaskFaultsUnobserved_LoggedFaultIncludesActiveOperationKinds GlobalExceptionLoggingTests The AsyncLocal category-only operation scope flows into the unobserved-fault log entry as kinds+counts (no values)
TplEventRingBuffer_OnUnobservedFault_FlushesTaskCausalityIdsOnly_NoPayloadData CrashDiagnosticsServiceTests (new) Ring buffer flushes recent TplEventSource events retaining ids/causality only — no payload fields containing user data
CrashDialog_WhenCrashRecordWritten_ShowsPathToCrashFolder CrashDialogTests The dialog surfaces the crash-folder path instead of a truncated URL

Related

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions