[#55] Fix cron backfill silently swallowing DB errors#56
Conversation
All upsert calls now destructure { error } and throw on failure,
so DB errors are caught by the existing try/catch and counted in
the errors response field. Cursor update also checks for errors.
Removed 3 unused imports (plotChainedEvent, storylineCreatedEvent,
donationEvent).
Fixes #55
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
APPROVE
Verified all 4 acceptance criteria:
-
Upsert error checks — All 4 upsert calls (storyline, genesis plot, plot, donation) now destructure
{ error }and throw on failure. Thrown errors bubble up to the existingtry/catchin the main loop, correctly incrementing theerrorscounter. -
Error counter accuracy — DB failures now throw → caught by the same
catchblock that handles decode/IPFS failures →errorsresponse field reflects both types. Good. -
Cursor update — Checked with
console.error(not throw) — correct choice since cursor failure shouldn't abort the response after successful processing. -
Unused imports removed —
plotChainedEvent,storylineCreatedEvent,donationEventcleaned up.
Minor note (non-blocking): If processStorylineCreated succeeds on the storyline upsert but fails on the genesis plot upsert, the storylinesInserted counter won't increment even though the storyline row was written. This is a minor reporting inaccuracy in an edge case — not worth blocking.
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: REQUEST CHANGES
Summary
The PR fixes error counting for failed upserts and the CI checks are green, but it still does not satisfy the issue requirement to log per-event failures.
Findings
- [medium] Per-event database/decode failures are still swallowed without any log output.
- File:
src/app/api/cron/backfill/route.ts:156 - Suggestion: Capture the thrown error in the loop (
catch (error)) and log enough context to diagnose the failed log before incrementingerrors. Right now the newthrow new Error(...)messages from the upserts are discarded immediately by the bare catch, so operators only get a count with no actionable detail.
- File:
Decision
Requesting changes because issue #55 explicitly requires failures to be logged/count failures, and the current loop only counts them.
Addresses T2a review: bare catch now captures the error and logs tx hash + log index for operator diagnosis instead of silently incrementing the counter. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
project7-interns
left a comment
There was a problem hiding this comment.
APPROVE (re-review)
Bare catch now logs tx hash + logIndex + error message — good for debugging failed backfill runs. All previous approvals still hold. Ship it.
project7-interns
left a comment
There was a problem hiding this comment.
Verdict: APPROVE
Summary
The follow-up commit fixes the remaining review gap by logging per-event failures with transaction context before incrementing the error counter. The PR now satisfies issue #55 and the lint/typecheck workflow passed.
Findings
- None.
Decision
Approving because the backfill now both counts and logs database/decode failures, checks cursor update errors, removes the unused imports, and passes CI.
Summary
upsert()calls now destructure{ error }and throw on failure, so DB errors are caught by the existingtry/catchand reflected in theerrorsresponse fieldplotChainedEvent,storylineCreatedEvent,donationEvent)Fixes #55
Test plan
npm run lintpassesnpm run typecheckpasses🤖 Generated with Claude Code