Skip to content

Fix GVFS.Mount exiting 0 on failure, hiding mount errors - #2131

Draft
Tyrie Vella (tyrielv) wants to merge 1 commit into
microsoft:masterfrom
tyrielv:tyrielv/mount-exit-code
Draft

Tyrie Vella (tyrielv) wants to merge 1 commit into
microsoft:masterfrom
tyrielv:tyrielv/mount-exit-code

Conversation

@tyrielv

Copy link
Copy Markdown
Contributor

Problem and Context

GVFS.Mount.exe could exit with code 0 even when a mount attempt failed, which hides the failure from anything that depends on its exit code. The root cause is in GVFS.Mount's entry point: Main was void, so the result of System.CommandLine's Invoke() call (the real outcome of the mount attempt) was discarded. The CLI action registered on the root command also only called verb.Execute() without returning its result.

Separately, a transient exception thrown while reading cache-server configuration from git config (CacheServerResolver.GetCacheServerFromConfig) could escape InProcessMountVerb.Execute() entirely uncaught. Because this happens after the mount log file is created but before anything is written to it, the result was an empty log with no explanation, on top of the process still exiting 0.

Together these mask real mount failures from any caller that checks gvfs mount's exit code, including the post-command hook that retries worktree mounts after certain git operations — a failed mount looks identical to a successful one, so a caller has no signal to retry or report an error.

Changes

  • GVFS.Mount.Program.Main now returns int and propagates Invoke()'s result as the real process exit code, calling Environment.Exit only when it is non-zero (needed to force background threads to exit).
  • InProcessMountVerb.BuildRootCommand()'s action now returns the real ReturnCode instead of being void: it catches MountAbortedException and returns its ReturnCode, since Invoke() never rethrows exceptions that escape the action — it always returns an int.
  • InProcessMountVerb.Execute() now wraps the remainder of the mount attempt (including the cache-server config read) in a catch-all, so any exception that is not already a MountAbortedException is traced and reported instead of escaping silently with an empty log.
  • MountVerb.Execute() no longer reports a child exit code of Success (0) when TryMount has already reported failure; it keeps the GenericError default in that case instead, since a child process reporting success while TryMount says otherwise is itself abnormal.
  • GVFS.Hooks' TryMountWithRetry now prints the exit code, stdout, and stderr of each failed quiet retry to stderr, instead of discarding them silently.
  • Added a regression test asserting that GVFS.Mount's root command returns GenericError (not 0 or a generic 1) for an invalid enlistment, and a WaitUntilMounted test case covering a mount process that exits with code 0.

GVFS.Mount's Main method was void, so System.CommandLine's Invoke()
result (the real outcome of the mount attempt) was discarded and the
process always exited 0, even when mounting failed. The CLI action
also only ever called verb.Execute() without returning its result, and
a transient exception from CacheServerResolver.GetCacheServerFromConfig
could escape Execute() entirely uncaught, leaving behind an empty log
file with no trace of what happened.

This caused callers that depend on gvfs mount's exit code (including
the post-command hook that retries worktree mounts) to treat a failed
mount as a success, masking the real error.

- GVFS.Mount.Program.Main now returns int and propagates Invoke()'s
  result as the real process exit code, calling Environment.Exit only
  when it is non-zero.
- InProcessMountVerb.BuildRootCommand()'s action now returns the real
  ReturnCode: MountAbortedException is caught and its ReturnCode
  returned, since Invoke() never rethrows exceptions that escape the
  action.
- InProcessMountVerb.Execute() now wraps the remainder of the mount
  attempt (including the cache-server config read) in a catch-all, so
  any exception that is not already a MountAbortedException is traced
  and reported instead of escaping silently.
- MountVerb.Execute() no longer reports a child exit code of 0
  (Success) when TryMount has already reported failure; it keeps the
  GenericError default in that case instead.
- GVFS.Hooks' TryMountWithRetry now prints the exit code, stdout, and
  stderr of each failed quiet retry to stderr, instead of discarding
  them silently.
- Added a regression test asserting GVFS.Mount's root command returns
  GenericError (not 0 or a generic 1) for an invalid enlistment, and a
  WaitUntilMounted test case for a mount process that exits with code 0.

Assisted-by: Claude Sonnet 5
Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
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.

1 participant