Skip to content

Add ExecutionContext to snapshot command details#261

Draft
broken-circle wants to merge 1 commit into
swiftlang:mainfrom
broken-circle:feature/execution-context
Draft

Add ExecutionContext to snapshot command details#261
broken-circle wants to merge 1 commit into
swiftlang:mainfrom
broken-circle:feature/execution-context

Conversation

@broken-circle
Copy link
Copy Markdown
Contributor

Closes #222 by adding SubprocessError.executionContext, representing a snapshot of the executable, arguments, environment, and working directory configured at the time the error was thrown.

Motivation

SubprocessError currently surfaces what went wrong via code and underlyingError, but not what was being run. A user who logs or rethrows an error has no machine-readable record of which subprocess invocation produced that error. Reconstructing that information from surrounding application code is tedious and error-prone. Including it on SubprocessError is a natural fit.

Design

A new public ExecutionContext struct holds the four configured inputs (executable, arguments, environment, working directory). The values represent the caller's inputs, not a resolved snapshot. For example, if Environment.inherit was used, then ExecutionContext.environment reflects .inherit, not a snapshot of the parent process's environment variables.

SubprocessError gains an optional executionContext property, which is populated for every error that propagates from a run() call. It may be nil for errors observed outside of a run() call, such as errors thrown by monitorProcessTermination(for:).

Errors can originate at two layers:

  • The spawn path itself, where Configuration.run(input:output:error:_:) catches and wraps every thrown SubprocessError.
  • Inner I/O layers reachable from inside a user body closure (StandardInputWriter, AsyncBufferSequence and StringSequence, OutputProtocol).

The inner I/O layers attach the context themselves before throwing, with the most specific information they have. The outer wrap in Configuration.run(input:output:error:_:) then catches anything else propagating out and attaches the same context. SubprocessError.withExecutionContext(_:) makes attachment idempotent: if an error already carries an ExecutionContext, the method returns the error unchanged, preserving the inner-layer attachments.

I considered threading ExecutionContext through every internal error factory directly, but that would have required updating dozens of throw sites with the same context object and offered no improvement over the wrap-at-higher-level approach.

Future directions

#222 also requests including the exit code/signal and captured standard input/output on the error. Both involve scoping and design questions distinct from the snapshot (for example, whether the captured streams should be included verbatim or truncated). I plan to address these in follow-up PRs.

Testing

The full test suite passes locally on macOS, and on windows-latest and ubuntu-latest using GitHub Actions.

Changes to existing tests

Nine existing tests in IntegrationTests that asserted on SubprocessError equality have been updated to directly assert on the relevant properties, since the addition of executionContext caused the type-level comparison to fail.

ProcessMonitoringTests.testInvalidProcessIdentifier() has been extended to verify that Execution-level errors carry no executionContext, confirming the public documentation.

New tests

Four new integration tests verify that executionContext is attached and round-trips correctly.

A new ExecutionContextTests suite adds unit tests for ExecutionContext and SubprocessError.withExecutionContext(_:).

The StandardInputWriter wrap is exercised implicitly by every input method routed through it. I excluded a direct test because the trigger conditions involve pipe-buffer timing that would make it less reliable than surrounding tests.

`SubprocessError` currently surfaces what went wrong via `code` and
`underlyingError`, but not what was being run.

Add `SubprocessError.executionContext`, representing a snapshot of the
executable, arguments, environment, and working directory configured at
the time the error was thrown.
@broken-circle broken-circle force-pushed the feature/execution-context branch from 57f3b23 to 23c0fe5 Compare May 10, 2026 20:38
@jakepetroules
Copy link
Copy Markdown
Contributor

The values represent the caller's inputs, not a resolved snapshot. For example, if Environment.inherit was used, then ExecutionContext.environment reflects .inherit, not a snapshot of the parent process's environment variables.

The state used to derive the resolved values can change throughout the lifetime of a process, and so I think it's important we surface the resolved values at the time of the error.

Perhaps the executionContext could have both "unresolved" and "resolved" variants of the executable and environment -- depending on where the error was thrown from, the "resolved" versions might be unavailable and therefore nil, but we should provide them to the caller if we have them.

@broken-circle
Copy link
Copy Markdown
Contributor Author

broken-circle commented May 11, 2026

@jakepetroules For sure, resolved environment values can be added on all platforms, and resolved executables are straightforward on Unix.

Windows has some hurdles for the resolved executable when the caller passes executable(_:) and we go through the built-in executable search of CreateProcessW and CreateProcessWithLogonW. Neither returns the resolved path, and PROCESS_INFORMATION carries only handles and IDs. We could call QueryFullProcessImageNameW post-spawn to recover it, but that's an extra syscall and point of failure. An alternative would be to leave resolvedExecutable as nil in that specific case and implement the post-spawn lookup if there's demand for it. Callers that pass path(_:) or go through the executablePathOverride slow path would still get resolved values as on Unix.

Do you want to resolve the working directory too? If the provided working directory was nil, we'd resolve to the parent's CWD at spawn. That's an extra syscall per spawn, but it would be consistent with the other resolutions.

Were you thinking these features should be part of this PR, or as a follow-up? I'd go with a separate PR, but I'm good with either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Feature] Retrieve the actual command in case of failure

2 participants