Repository navigation
Fix GVFS.Mount exiting 0 on failure, hiding mount errors - #2131
Draft
Tyrie Vella (tyrielv) wants to merge 1 commit into
Draft
Tyrie Vella (tyrielv) wants to merge 1 commit into
Tyrie Vella (tyrielv) wants to merge 1 commit into
Conversation
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem and Context
GVFS.Mount.execould 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 inGVFS.Mount's entry point:Mainwasvoid, so the result ofSystem.CommandLine'sInvoke()call (the real outcome of the mount attempt) was discarded. The CLI action registered on the root command also only calledverb.Execute()without returning its result.Separately, a transient exception thrown while reading cache-server configuration from git config (
CacheServerResolver.GetCacheServerFromConfig) could escapeInProcessMountVerb.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.Mainnow returnsintand propagatesInvoke()'s result as the real process exit code, callingEnvironment.Exitonly when it is non-zero (needed to force background threads to exit).InProcessMountVerb.BuildRootCommand()'s action now returns the realReturnCodeinstead of beingvoid: it catchesMountAbortedExceptionand returns itsReturnCode, sinceInvoke()never rethrows exceptions that escape the action — it always returns anint.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 aMountAbortedExceptionis traced and reported instead of escaping silently with an empty log.MountVerb.Execute()no longer reports a child exit code ofSuccess(0) whenTryMounthas already reported failure; it keeps theGenericErrordefault in that case instead, since a child process reporting success whileTryMountsays otherwise is itself abnormal.GVFS.Hooks'TryMountWithRetrynow prints the exit code, stdout, and stderr of each failed quiet retry to stderr, instead of discarding them silently.GVFS.Mount's root command returnsGenericError(not 0 or a generic 1) for an invalid enlistment, and aWaitUntilMountedtest case covering a mount process that exits with code 0.