← Back to PRs

#22993: fix(delivery): guard JSON.parse in failDelivery to prevent silent infinite retries

by adhitShet open 2026-02-21 22:17 View on GitHub →
size: XS trusted-contributor
## Summary `failDelivery` reads a queue entry's JSON file and parses it without a try-catch. If the file is corrupt (partial write, disk error, filesystem corruption), `JSON.parse` throws a `SyntaxError`. Every caller silently swallows this error (`.catch(() => {})` or bare `catch {}`), so `retryCount` is never incremented and the delivery stays in the queue indefinitely — retried on every gateway restart without ever reaching `MAX_RETRIES`. ## Bug ```ts // src/infra/outbound/delivery-queue.ts, line 131 const entry: QueuedDelivery = JSON.parse(raw); // unguarded ``` All three callers suppress errors: - `deliver.ts:266` — `.catch(() => {})` - `deliver.ts:277` — `.catch(() => {})` - `delivery-queue.ts:294` — `try { … } catch { /* Best-effort */ }` Result: corrupt entry ➜ SyntaxError ➜ silently swallowed ➜ retryCount never bumped ➜ entry never reaches MAX_RETRIES ➜ infinite retry loop. ## Fix Wrap `JSON.parse` in try-catch. On parse failure, move the corrupt entry to the `failed/` subdirectory — the same destination used when `retryCount >= MAX_RETRIES`. ```ts let entry: QueuedDelivery; try { entry = JSON.parse(raw) as QueuedDelivery; } catch { // Corrupt queue file — move to failed/ to prevent infinite retries. await moveToFailed(id, stateDir); return; } ``` ## Consistency `loadPendingDeliveries` in the same file (lines 164–173) already wraps its `JSON.parse` in try-catch to skip malformed entries. This fix applies the same defensive pattern to the update path. ## Test plan - [x] `npx oxfmt --check src/infra/outbound/delivery-queue.ts` — pass - [x] `npx oxlint src/infra/outbound/delivery-queue.ts` — 0 warnings, 0 errors - [x] `npx vitest run src/infra/outbound/outbound.test.ts --config vitest.unit.config.ts` — 55 tests pass 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- greptile_comment --> <h3>Greptile Summary</h3> Fixes a silent infinite retry loop caused by corrupt queue files. The fix adds defensive error handling around `JSON.parse` in `failDelivery`, moving corrupt entries to `failed/` instead of allowing them to retry indefinitely. This matches the existing defensive pattern in `loadPendingDeliveries` (lines 171-180). - Added try-catch around `JSON.parse` in `failDelivery` to handle corrupt queue files - Corrupt entries now move to `failed/` subdirectory, preventing infinite retries - Fix applies the same defensive pattern already used in `loadPendingDeliveries` <h3>Confidence Score: 5/5</h3> - This PR is safe to merge with minimal risk - The fix addresses a real bug with a simple, defensive solution that follows existing patterns in the codebase. The change is minimal (7 lines), well-tested (existing test suite passes), and applies the same error-handling pattern already used in `loadPendingDeliveries`. The only minor improvement would be adding a try-catch around `moveToFailed` for extra defensiveness, but this doesn't impact correctness. - No files require special attention <sub>Last reviewed commit: 4c244f3</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment -->

Most Similar PRs