Skip to content

Mount: remove broad HandleRequest exception catch - #2050

Merged
tyrielv merged 1 commit into
microsoft:masterfrom
tyrielv:tyrielv/narrow-handlerequest-catch
Jul 15, 2026
Merged

Mount: remove broad HandleRequest exception catch#2050
tyrielv merged 1 commit into
microsoft:masterfrom
tyrielv:tyrielv/narrow-handlerequest-catch

Conversation

@tyrielv

@tyrielv tyrielv commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Follow-up to PR #2021 (merged). Changes the outer try-catch in HandleRequest from swallowing exceptions to catch-log-rethrow, so fail-fast via Environment.Exit is 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:

  • PostIndexChanged — projection update mid-flight → stale/wrong projection
  • DehydrateFolders — folder moves + reset → half-dehydrated state
  • ReleaseLock — lock bookkeeping corruption

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 generic LogErrorAndExit in NamedPipeServer does not capture), then rethrows so the exception propagates to OnNewConnectionEnvironment.Exit as before.

What stays unchanged

The inner try-catch in HandleDownloadObjectRequest remains — it covers the specific crash scenario #2021 addressed. The download path is safe to catch and suppress:

  • Loose object writes are atomic (temp file → rename)
  • missingTreeTracker is advisory (re-populated on next request)
  • No critical mount state (projection, lock, placeholder DB) is mutated
  • Returns proper DownloadFailed response instead of broken pipe

Testing

  • Unit tests: 876 passed, 0 failed
  • The inner DownloadObject catch continues to prevent the flaky rebase test crash

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
tyrielv force-pushed the tyrielv/narrow-handlerequest-catch branch from a709f17 to e1dc57a Compare July 9, 2026 21:02
@tyrielv
tyrielv marked this pull request as ready for review July 9, 2026 21:07
@tyrielv
tyrielv enabled auto-merge July 9, 2026 21:07
@tyrielv
tyrielv merged commit cf7a17a into microsoft:master Jul 15, 2026
35 checks passed
@tyrielv tyrielv mentioned this pull request Jul 15, 2026
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.

2 participants