fix(orb): guard finalizeRelayFailureRetryRow DB writes against duplicate redelivery (#8332)#8462
Conversation
…ilureRetryRow's log (JSONbored#8332)
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8462 +/- ##
==========================================
+ Coverage 79.69% 89.64% +9.95%
==========================================
Files 791 98 -693
Lines 79319 22847 -56472
Branches 23954 3912 -20042
==========================================
- Hits 63216 20482 -42734
+ Misses 13298 2187 -11111
+ Partials 2805 178 -2627
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-24 14:47:52 UTC
Review summary Nits — 4 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Summary
src/orb/relay.ts'sretryFailedRelaysdoc comment promises "Never throws." Its per-row finalize step,finalizeRelayFailureRetryRow, issued itsDELETE/UPDATEonorb_relay_failureswith no try/catch — a transient D1 write failure right after a successful forward would reject out of thePromise.allbatch, breaking that contract, and would leave the failure row un-deleted despite the event already having been forwarded (risking duplicate redelivery on the next retry tick). Wrapped both writes in a try/catch that logs an alertable structured error (delivery_id,event_name,outcome) and swallows, matching this file's existingconsole.error(JSON.stringify({...}))logging convention used elsewhere in the same file.(Reopens the substance of #8441, which was auto-closed on a stale
codecov/patchevaluation from a genuine gap in the original test — theerror instanceof Error ? error.message : String(error)fallback branch was only ever exercised with realErrorinstances. Fixed here: one regression test now throws a non-Error value to cover theString(error)branch, verified locally at 99.11% branch coverage onsrc/orb/relay.ts, the only remaining gap being pre-existing unrelated code outside this diff.)Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Closes #123) — a linked open issue is required for every contributor PR.Closes #8332
Validation
git diff --checknpm run typechecknpx vitest run test/integration/orb-relay.test.ts --coverage --coverage.include="src/orb/relay.ts"— all 85 tests pass; 99.11% branch coverage (112/113) on the changed file — the single remaining uncovered branch is pre-existing code outside this diff (enqueueConfigPushRelay, lines 385-392)npm run actionlintnpm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
src/orb/relay.tsand its integration test (no UI/MCP/worker/OpenAPI surface touched), so the UI/MCP/workers/OpenAPI-specific checks above were not run locally; they are unaffected by this diff and are still exercised by the full CI gate.Safety
UI Evidencesection below. (N/A — no visible UI change.)UI Evidence
N/A — this is a backend cron/retry-path change with no visible UI surface.
Notes
finalizeRelayFailureRetryRow's two DB writes are guarded; no other logic inretryFailedRelaysor elsewhere in the file was touched, per the issue's own scope note.