Snapshot and resolve command details with ExecutionContext#261
Snapshot and resolve command details with ExecutionContext#261broken-circle wants to merge 1 commit into
ExecutionContext#261Conversation
57f3b23 to
23c0fe5
Compare
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. |
|
@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 Do you want to resolve the working directory too? If the provided working directory was 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. Update: Rebased this onto |
23c0fe5 to
e09077a
Compare
ExecutionContext to snapshot command detailsExecutionContext
|
This is also an API change. I added the keyword. @iCharlesHu should this also be added to the "post-1.0" milestone? |
e09077a to
231543e
Compare
|
I rebased this onto |
231543e to
1dd1333
Compare
`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, along with values resolved at spawn time.
1dd1333 to
a001b92
Compare
Partially addresses #222 by adding
SubprocessError.executionContext, representing a snapshot of the executable, arguments, environment, and working directory configured at the time the error was thrown, along with values resolved at spawn time.Design
A new public
ExecutionContextstruct contains properties representing the supplied inputs and their resolved values. Resolution happens at spawn time rather than when the error is inspected, so the resolved values reflect what the child actually received; the state from which they derive can change over the parent's lifetime. A few resolved values are purposefully absent or adjusted:resolvedExecutableisnilon Windows when the executable is given by name:CreateProcessW()performs the search internally and does not report the path it used, and Subprocess does not issue a separate post-spawn lookup to recover it. Path-based executables report a path on all platforms.resolvedEnvironmentisnilfor environments given as raw bytes (no faithfulStringrepresentation), and on Windows may include aPATHentry that Subprocess adds so the child can locate its DLLs.resolvedWorkingDirectoryis the configured directory when one is set, and otherwise the parent's working directory captured at spawn.The context is attached in
Configuration.run(). A failure during spawn is caught there with the configured inputs only, since no resolved values exist yet; any error surfacing after a successful spawn propagates back through the same function and is caught with the resolved context. Every error that leaves arun()call passes through one of these two points, soStandardInputWriter,SubprocessOutputSequence, and the collected output types need no awareness ofExecutionContext. OnlySubprocessErroris wrapped, so an error thrown from the caller's own body closure propagates unchanged.executionContextisnilonly for errors observed outside arun()call, such asExecution-level process monitoring.Testing
Nine existing tests in
IntegrationTeststhat asserted onSubprocessErrorequality have been updated to directly assert on the relevant properties, since the addition ofexecutionContextcaused the type-level comparison to fail.ProcessMonitoringTests.testInvalidProcessIdentifier()has been extended to verify thatExecution-level errors carry noexecutionContext.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'll address these in follow-up PRs.