Fall back to tree pull when export exceeds cap or DO overloads (#192)#195
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR extends the export error classification to detect truncated JSON responses, Durable Object overload conditions, and payload-too-large errors, treating them as unsupported so the syncer falls back to a paginated list-tree pull instead of retrying. New test coverage validates the classification logic and confirms reconcile fallback behavior under these failure modes. ChangesExport fallback and classification
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/mountsync/syncer_test.go (1)
636-637: ⚡ Quick winAdd typed truncation errors to classification coverage.
These tests currently validate truncation mostly via error-message strings. Add typed cases (
io.ErrUnexpectedEOF,*json.SyntaxError) so classification coverage remains stable if message text changes.Proposed test hardening
func TestExportSnapshotOverloadedClassification(t *testing.T) { unsupported := []error{ + io.ErrUnexpectedEOF, + &json.SyntaxError{Offset: 12}, &HTTPError{StatusCode: 500, Code: "internal_error", Message: "Durable Object is overloaded. Requests queued for too long."}, &HTTPError{StatusCode: 503, Message: "Worker overloaded"}, errors.New("http 500 internal_error: Durable Object is overloaded. Requests queued for too long."), &HTTPError{StatusCode: 413, Code: "payload_too_large", Message: "workspace export body is 3524788058 bytes, which exceeds the export body limit of 134217728; use paginated tree/read APIs instead"}, }Also applies to: 709-715
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/mountsync/syncer_test.go` around lines 636 - 637, Update the test cases that currently assert truncation by error-message strings to include typed truncation errors so classification doesn't depend on text: replace or add cases using io.ErrUnexpectedEOF and a *json.SyntaxError (e.g., construct &json.SyntaxError{Offset:...}) as the exportErr values in the table used by the tests (the entries around the exportErr field in internal/mountsync/syncer_test.go and the similar entries at the later block near lines 709-715). Ensure the test logic that classifies truncation treats these typed errors the same way as the current string-based cases so the assertions still pass.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/mountsync/syncer_test.go`:
- Around line 636-637: Update the test cases that currently assert truncation by
error-message strings to include typed truncation errors so classification
doesn't depend on text: replace or add cases using io.ErrUnexpectedEOF and a
*json.SyntaxError (e.g., construct &json.SyntaxError{Offset:...}) as the
exportErr values in the table used by the tests (the entries around the
exportErr field in internal/mountsync/syncer_test.go and the similar entries at
the later block near lines 709-715). Ensure the test logic that classifies
truncation treats these typed errors the same way as the current string-based
cases so the assertions still pass.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3fb64c7d-0600-4d58-9846-518aad5fc969
📒 Files selected for processing (2)
internal/mountsync/syncer.gointernal/mountsync/syncer_test.go
…loads On large workspaces the single full-tree export cannot complete: - The cloud caps the export body at 128MB and responds `http 413 payload_too_large: workspace export body is N bytes ... use paginated tree/read APIs instead` (observed: a 3.5GB export body on rw_fc7b534b). - Under load the export also intermittently fails `http 500 internal_error: Durable Object is overloaded`. exportSnapshotUnsupported recognized neither, so pullRemoteFullExport retried the doomed export every cycle and bootstrap wedged (~14k files on rw_fc7b534b, never completing). Classify both 413 payload-too-large and 5xx-with-"overloaded" as unsupported so pullRemoteFull falls through to pullRemoteFullTree, whose paginated ListTree + per-file reads are individually bounded and resume from the persisted cursor — exactly what the 413 body instructs. Bare 5xx without the overload signal still retries the export, so genuinely transient server errors are unaffected. Verified live on rw_fc7b534b: bootstrap broke past the prior export wall (~14.3k) and resumed climbing via the paginated tree path. Stacks on the truncated-export fallback (#192). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
khaliqgant
force-pushed
the
codex/issue-192-do-overload
branch
from
774bd92 to
5b57b57
Compare
Relayfile Eval ReviewRun: Passed: 4 | Needs human: 0 | Reviewable: 0 | Missing output: 0 | Failed: 0 | Skipped: 0 Human Review CasesNo reviewable human-review cases captured Relayfile output. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
1 participant
Summary
Extends the truncated-export fallback (PR that landed
451ff91) to two further failure modes that wedge bootstrap on large workspaces.On
rw_fc7b534b(~29k files, ~3.5 GB export body), bootstrap was wedging because the full-tree export endpoint can't fit the workspace into one response:http 413 payload_too_large— the cloud caps the export body at 128 MB and responds:workspace export body is N bytes, which exceeds the export body limit of 134217728; use paginated tree/read APIs instead. This is the persistent, deterministic blocker.http 500 internal_error: Durable Object is overloaded— under load the export also intermittently fails this way (it's the secondary symptom of the same problem: the export forces the DO to serialize the entire workspace in one invocation).exportSnapshotUnsupportedrecognized neither, sopullRemoteFullExportretried the doomed export every cycle and bootstrap never progressed past where the DO began to fail (observed wedging at ~14k files).Classify both
413 payload_too_largeand5xx-with-"overloaded"as unsupported sopullRemoteFullfalls through topullRemoteFullTree, whose paginatedListTree+ per-file reads are individually bounded and resume from the persisted cursor — exactly what the 413 body instructs. Bare5xxwithout the overload signal still retries the export so genuinely transient server errors are unaffected.Test plan
go test ./internal/mountsync/... -count=1— full package green, including:TestReconcileFallsBackToTreeWhenExportJSONTruncated(existing, unaffected)TestReconcileFallsBackToTreeWhenExportDurableObjectOverloaded(new)TestReconcileFallsBackToTreeWhenExportPayloadTooLarge(new)TestExportSnapshotOverloadedClassification(new — positive cases for 413/overload, negative cases asserting transient 5xx still retries)rw_fc7b534b: pre-fix the daemon wedged at ~14.3k files retrying the failing export. Post-fix the daemon broke past the wall (14,317 → 15,095 → 16,095 → … → 25,013 BOOTSTRAP COMPLETE, then grew to 29,282 via incremental sync). Nocycle failed: 413lines appeared after the fix took over — the fallback swallows the error cleanly.Related
unexpected end of JSON inputon every full-export cycle (distinct from #175 H stall) #192 (the originalunexpected end of JSON inputsymptom is one of three blockers on this code path; this PR completes the export-fallback story451ff91started).🤖 Generated with Claude Code