OUT-3709: drop placeholder errorMessage on stale PENDING flip#246
Conversation
flipStalePendingToFailed was stamping a state-description string onto qb_sync_logs.error_message. SyncService.intiateSync reads that column when reporting to Sentry on the final retry, so the placeholder was being reported as if it were the real exception — misleading on-call. Leave errorMessage untouched on the flip so the first retry that throws populates it with the real reason via updateFailedSyncLog. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR removes the hardcoded
Confidence Score: 5/5Safe to merge — a single-line deletion that removes a misleading placeholder; no logic paths are altered and the database column already accepts NULL. The change is a targeted one-line removal with no new code paths introduced. Flipped rows will carry NULL for errorMessage until a real retry exception writes the genuine reason, which is strictly more informative than the old placeholder. The PR author has correctly identified and scoped the residual gap (silent early-return paths) as a separate follow-up. No files require special attention. Important Files Changed
Sequence DiagramsequenceDiagram
participant Cron as Cron / syncFailedRecords
participant SLS as SyncLogService
participant DB as qb_sync_logs
participant SS as SyncService#intiateSync
participant Sentry
Cron->>SLS: flipStalePendingToFailed()
SLS->>DB: "UPDATE SET status=FAILED, category=OTHERS"
Note over DB: errorMessage = NULL (placeholder removed)
Cron->>SS: retry stale row
alt retry throws real error
SS->>DB: updateFailedSyncLog(realError)
Note over DB: errorMessage = real exception
SS->>Sentry: "captureMessage(errorMessage = real exception)"
else retry succeeds
SS->>DB: "status = SUCCESS"
end
Reviews (1): Last reviewed commit: "fix(OUT-3709): drop placeholder errorMes..." | Re-trigger Greptile |
Summary
flipStalePendingToFailedpreviously stampederrorMessage: 'Stale PENDING claim — worker did not finalise in time'ontoqb_sync_logswhen recovering stale PENDING rows.SyncService.intiateSyncreadscurrentLog.errorMessagewhen reporting to Sentry on the final retry attempt (sync.service.ts:479), so that placeholder was being surfaced as if it were the real exception — misleading on-call (e.g. ACCOUNTING-SYNC-3M).status: FAILEDandcategory: OTHERS; the first retry that actually throws will populateerrorMessagewith the real reason viaupdateFailedSyncLog.Out of scope
processInvoiceDeleted/processInvoiceVoided(!invoiceSync.customer) that never overwriteerrorMessageon retry. With this change, those rows now surfacenullin Sentry instead of the misleading placeholder — uninformative, but no longer misleading. Cleanup deferred to a follow-up ticket.Test plan
yarn test).yarn lint:check,yarn prettier:check).error_message = NULLand the next retry either fills it with a real exception or flips to SUCCESS.🤖 Generated with Claude Code