OUT-3703: drop Sentry capture for QBReconnectRequiredError in cron

[SandipBajracharya]

No description provided.

[linear-code]

OUT-3703

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
quickbooks-sync	Building		May 13, 2026 9:02am
quickbooks-sync (dev)	Ready	Preview, Comment	May 13, 2026 9:02am

[greptile-apps]

Greptile Summary

This PR removes the Sentry.captureException call from the QBReconnectRequiredError handler in the daily refresh-token cron, on the grounds that the cron has no Copilot user context to act on the signal — the CustomLogger.error entry and the reconnectRequired counter are preserved. Generic errors continue to be captured by Sentry unchanged.

• Service change: five lines deleted from the QBReconnectRequiredError branch; the error is still logged and counted, just no longer sent to Sentry.
• Test update: the coverage comment is updated to match the new behavior, but no negative assertion (expect(captureException).not.toHaveBeenCalled()) is added to enforce it programmatically.

Confidence Score: 4/5

Safe to merge — the change is a deliberate, well-reasoned reduction in Sentry noise with no functional impact on token refresh logic.

The service change itself is correct and the existing generic-error Sentry path is untouched. The only gap is that the test comment documents 'no Sentry event for reconnect errors' but the assertion body doesn't enforce it, so the contract could regress silently.

The test file would benefit from a negative Sentry assertion to lock in the documented behavior.

Important Files Changed

Filename	Overview
src/app/api/quickbooks/refresh-tokens/refresh-tokens.service.ts	Removes Sentry captureException from the QBReconnectRequiredError handler; generic errors still go to Sentry. Log statement is preserved.
test/unit/api/quickbooks/refresh-tokens/refresh-tokens.service.test.ts	Updates the coverage comment to reflect the removal of the Sentry event, but adds no negative assertion to lock in that captureException is not called for QBReconnectRequiredError.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[refreshExpiringTokens cron] --> B[getPortalsWithExpiringRefreshTokens]
    B --> C{For each portal row}
    C --> D[getRefreshedQbTokenInfo]
    D -->|success| E[summary.refreshed += 1]
    D -->|QBReconnectRequiredError| F[summary.reconnectRequired += 1]
    F --> G[CustomLogger.error — log only]
    G -->|previously| H["~~Sentry.captureException~~ REMOVED"]
    G --> I[continue to next portal]
    D -->|other error| J[summary.errored += 1]
    J --> K[CustomLogger.error]
    K --> L[Sentry.captureException — still active]
    E --> C
    I --> C
    L --> C
    C -->|done| M[return RefreshTokensSummary]

Loading

Comments Outside Diff (1)

1. test/unit/api/quickbooks/refresh-tokens/refresh-tokens.service.test.ts, line 196-204 (link)

   The comment on line 10 explicitly states "no longer raises a Sentry event", but the test body doesn't assert this. Without an explicit not.toHaveBeenCalled() check, re-adding the captureException call to the service would leave this test green while silently breaking the documented contract. The Sentry mock is already set up, so the assertion is straightforward to add. Note that beforeEach doesn't reset the Sentry mock, so it's best to check inside this test's scope.

_{Reviews (1): Last reviewed commit: "chore(OUT-3703): drop Sentry capture for..." | Re-trigger Greptile}