Repository navigation
fix(host-service): classify any file its storage cannot map as environmental - #8842
Conversation
…nmental git's xmmap() reports 'mmap failed: <errno>' for packfiles, pack indexes and worktree files it hashes. ETIMEDOUT/ECANCELED there come from the storage layer (an evicted cloud-synced file or a stalled network volume), the same condition already classified for the index. Refs HOST-SERVICE-X
|
No actionable comments were generated in the recent review.
|
| Layer / File(s) | Summary |
|---|---|
Match and classify mmap failures packages/host-service/src/trpc/router/git/utils/classify-git-error.ts, packages/host-service/src/trpc/router/git/utils/classify-git-error.test.ts |
The classifier matches mmap failures caused by cancellation or timeout and uses the existing PRECONDITION_FAILED / GIT_ENVIRONMENT path. Tests cover both cases and verify that Cannot allocate memory remains unclassified. |
Priority:
Estimated code review effort: 2 (Simple) | ~8 minutes
Change: Bug fix
Suggested reviewers: saddlepaddle
Merge Risk: 
Minimal · up to 49537
-
The narrow mmap classification is covered by positive and negative tests and is ready to merge.
Pre-merge checks: 
5
-
Full details: Description check
-
Explanation
The description is detailed and on-topic. It explains the problem, root cause, fix, scope, negative case, testing, and issue reference. It uses different headings from the template and omits the checklist, but the required technical information is substantially complete.
-
Finishing Touches
-
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
[Low impact] This PR appears safe to merge. SummaryClassifies Git mapping failures caused by canceled or timed-out storage reads as
Reviews (1) · Last reviewed commit: "fix(host-service): classify any file its..." · Reviewed by Greptile |
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
Problem
git.getStatusreturns a 500 when git cannotmmapa file other than the index because the storage under it cannot deliver the file. Every status refresh on such a machine reports to Sentry.Over the last 30 days, the message
fatal: mmap failed: Operation canceled(or... Operation timed out) appeared in about 250git.getStatusevents from about 30 machine/release pairs, on releases 1.29.0 through 1.38.0, all macOS. The events are spread across several stack-grouped issues (HOST-SERVICE-X, -7Q, -88, -70 and others), and those groups also collect unrelated git failures. So archiving a group would also mute real failures.Root cause
#7636 classified this condition when git reads the index (
unable to map index file: Operation timed out|canceled). It left the same errnos on other files as 500s. git maps other files through itsxmmap()wrapper (wrapper.c): packfiles and pack indexes (packfile.c), and large worktree files when it hashes them for status. On failure that wrapper callsdie_errno("mmap failed%s", mmap_os_err()).mmap_os_err()adds text only forENOMEM, so the line ends in: <strerror(errno)>.The cause of
ETIMEDOUT/ECANCELEDfrommmapis the one #7636 describes. A cloud-synced folder evicts a file and leaves a dataless placeholder. The sync client then cannot fetch the file back in time, or it cancels the fetch. Or a network volume stops answering. Superset cannot make the file readable. It stays unreadable until the storage recovers or the repository moves off it.Fix
This adds one pattern to the existing
CWD_UNREADABLE/INDEX_STORAGE_UNAVAILABLE→PRECONDITION_FAILED / GIT_ENVIRONMENTbranch. The pattern is anchored to git's wholefatal: mmap failed:line, and it accepts only the two storage errnos. The message is kept as-is. As with #7636, the client handlesPRECONDITION_FAILEDandINTERNAL_SERVER_ERRORthe same way, so only reporting changes, not what the user sees.Still a 500:
mmap failedwith any other errno, such asCannot allocate memory.Verification
Tests written first. Before the pattern change:
After:
The negative test is load-bearing. With the pattern temporarily widened to
/mmap failed/m, it fails:bunx biome checkon both files:cd packages/host-service && bun run typecheck:tsc --noEmit --emitDeclarationOnly false, exit 0.bun test src/trpc/router/git(packages/host-service):Refs HOST-SERVICE-X
Summary by CodeRabbit