Mount: remove broad HandleRequest exception catch - #2050
Merged
tyrielv merged 1 commit intoJul 15, 2026
Conversation
Change the outer try-catch in HandleRequest from swallowing exceptions to catch-log-rethrow. The original catch (PR microsoft#2021) suppressed all exceptions except OutOfMemoryException, which could mask corruption in state-mutating handlers like PostIndexChanged, DehydrateFolders, and ReleaseLock — leaving the mount serving in an inconsistent state. The catch now logs the message header and exception (context that OnNewConnection's generic catch in NamedPipeServer does not capture), then rethrows so fail-fast via Environment.Exit still occurs for unhandled errors in state-mutating handlers. The inner try-catch in HandleDownloadObjectRequest remains unchanged — it correctly catches and returns DownloadFailed for transient network/disk errors in the download path, which is safe because no critical mount state is mutated. Assisted-by: Claude Opus 4.6 Signed-off-by: Tyrie Vella <tyrielv@gmail.com>
tyrielv
force-pushed
the
tyrielv/narrow-handlerequest-catch
branch
from
July 9, 2026 21:02
a709f17 to
e1dc57a
Compare
tyrielv
marked this pull request as ready for review
July 9, 2026 21:07
tyrielv
enabled auto-merge
July 9, 2026 21:07
Keith Klein (KeithIsSleeping)
approved these changes
Jul 15, 2026
Merged
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.
Summary
Follow-up to PR #2021 (merged). Changes the outer try-catch in
HandleRequestfrom swallowing exceptions to catch-log-rethrow, so fail-fast viaEnvironment.Exitis preserved while adding diagnostic context.Problem
The original broad catch suppressed all exceptions (except
OutOfMemoryException) from every pipe request handler. This could silently mask corruption in state-mutating handlers:Instead of crashing (which surfaces the problem immediately), the mount would continue serving in an inconsistent state.
Fix
The catch now logs the message header and exception details (context that
OnNewConnection's genericLogErrorAndExitinNamedPipeServerdoes not capture), then rethrows so the exception propagates toOnNewConnection→Environment.Exitas before.What stays unchanged
The inner try-catch in
HandleDownloadObjectRequestremains — it covers the specific crash scenario #2021 addressed. The download path is safe to catch and suppress:missingTreeTrackeris advisory (re-populated on next request)DownloadFailedresponse instead of broken pipeTesting