fix: silent Plaid disconnects, Quiltt currency stamping, unverified checkout, and single-currency totals - #7
Merged
Conversation
The Plaid SDK is axios-based, so a rejected request surfaces as an AxiosError whose message is only ever "Request failed with status code 400" — the actual error_code lives in the response body. The sync workflow only inspected the message, so isDisconnectError() never matched and every broken item was recorded with a NULL error_code. Three things followed from that: the connection was never parked (the poller only skips known disconnect codes), no reconnect email was ever sent, and the connection kept reporting status=active. One production item logged 4,556 identical 400s over ten weeks, retried every 30 minutes, with no notification after the first day. - extractPlaidError() reads error_code/error_message off the response - notifyDisconnect fires only on true disconnect codes, so newly captured transient codes (INSTITUTION_DOWN, rate limits) don't spam - the poller shares PLAID_DISCONNECT_ERROR_CODES, so a captured ITEM_LOGIN_REQUIRED actually stops the retry loop Also fixes reconnects appearing to do nothing. Plaid pulls from the bank asynchronously after Link, so the sync dispatched immediately afterwards returns an empty page and marks itself success ~0.7s later. syncTransactions has a wait-and-retry loop for this, but it is gated on cursor === null and a reauth preserves the cursor. The workflow now takes an awaitUpstreamPull flag, set on the Link path, that re-checks on a backoff instead of declaring success on an empty first page. Tests cover a thrown Plaid error, which nothing exercised before.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Quiltt transactions fell back to "USD" whenever the API omitted currencyCode, so a CAD account's transactions were filed under USD while the account row correctly reported CAD. Plaid and MX both store null rather than guessing. Resolve the currency as transaction -> parent account -> null. That never invents a currency, and unlike plain parity it keeps a CAD account's transactions labelled CAD instead of discarding what we already know. The existing currency test mocked getAllTransactions to [], so the transaction path was never exercised; cover it explicitly.
The success toast fired purely on the ?checkout=success query param, which
the browser controls and which says nothing about whether the plan was
applied. The plan was only ever written by the Stripe webhook, so the
redirect raced it: a user could pay, be told "Subscription activated!",
click add-account, and immediately hit the connection-limit modal because
the server still had them on free. Locally the webhook never arrives at all
unless `stripe listen` is running, so checkout could never take effect.
The success URL now carries {CHECKOUT_SESSION_ID} and the client hands it
to a new /api/billing/confirm-checkout, which retrieves the session and
applies the plan only when payment_status is paid and client_reference_id
matches the caller — so a session id cannot be replayed against another
account. Writes are plain updates, so the webhook remains an idempotent
backstop and stays the source of truth for renewals and cancellations.
The toast now reports what is actually known: success only once the server
confirms (or the plan is already non-free, covering the webhook winning the
race), otherwise "Payment received — activating your subscription…".
The home header bucketed accounts by currency, summed each bucket, sorted descending, and rendered only the first. A portfolio holding both CAD and USD therefore displayed the CAD total alone, and the USD side — including a five-figure chequing balance — never appeared anywhere on the page. There is no FX data in the app, so collapsing the buckets into a single figure would mean inventing a rate. Every non-zero currency now gets its own line instead, largest first. Zero buckets are dropped: a Wise connection carries a sub-account per currency and most sit at 0, which would otherwise bury the real totals behind a wall of GBP 0.00 / THB 0.00. An all-zero portfolio keeps one line so the header is never blank. Each line renders at the same size and weight. Styling the non-leading currencies as muted subtext read as a secondary stat about the big number rather than a separate total of separate money. Multi-currency totals also carry their ISO code (CAD 182,180.48), since a bare $715.32 sitting under a CA$ figure says nothing about which currency it belongs to; a lone total keeps the symbol form it has always had. The header and carousel move into AccountsOverview so this can be exercised in Storybook — the route itself cannot be, as it depends on the auth session, link context, and API fetches.
Winxton
changed the title
The awaitUpstreamPull retry loop exited on `added + modified + removed > 0`. A transaction delta is not a readiness signal, so two different states looked identical: an Item that finished pulling with nothing to report, and one that never finished at all. That produced a false success. When Plaid stayed NOT_READY through every retry the loop simply fell out the bottom into markComplete, so the job was recorded as `success` with 0 records and the UI showed a green "sync completed" on a pull that never happened. It also produced the inverse: a sync with legitimately no changes burned all 18.5 minutes of backoff while the job sat `pending`, spinning the client sync banner the whole time. syncTransactions already had transactions_update_status in hand and only logged it; it is now returned and used to decide the terminal state. Not ready when retries are exhausted is recorded as UPSTREAM_NOT_READY, which is deliberately not a disconnect code, so the poller keeps retrying and no reconnect email fires. Readiness needs two different signals. For an Item that has never pulled, transactions_update_status reaching HISTORICAL_UPDATE_COMPLETE is correct. For a reauth it is not: that status is an Item-level lifecycle milestone and already reads HISTORICAL_UPDATE_COMPLETE from the original link, before any fresh pull, so gating on it would make reauth exit instantly. Reauth instead compares last_successful_update against a baseline captured before waiting. The wait also no longer keys off `cursor === null`. Plaid returns an empty next_cursor both when an Item is not ready and when it has no /transactions/sync-eligible accounts, and both persist as "" — 5 of 30 production connections are in that state, all brokerages. Waiting is now requested explicitly by the Link path, so the scheduled poll (which runs connections serially) never blocks on an upstream pull. Finally, syncError is gated to genuine disconnect codes. It drives the "Reconnect" button and the reconnect task, and without this an unfinished pull would tell the user to redo a Link flow that cannot help. This also fixes a pre-existing case where INSTITUTION_DOWN and rate limits did the same.
The module exported three things — PLAID_DISCONNECT_ERROR_CODES,
RECONNECT_ERROR_CODES, and isReconnectErrorCode — and each of its three
consumers reached for a different one. The Plaid workflow then layered two
more local predicates on top, so one yes/no question ("must the user
re-link?") was answered by three predicates over two lists, and picking the
wrong list was silent.
The codes are per-provider vocabulary, so the PLAID_ prefix was doing the
namespacing the module should have been doing. Group them per provider,
keep both lists private, and export only the predicate. The lists cannot be
mixed up by a caller because callers no longer see them.
Also drops isDisconnectError(), the substring match over the error message.
extractPlaidError() only omits errorCode when the failure never produced a
Plaid error body at all — a network fault or a bug here — and such a message
will not contain "ITEM_LOCKED". Worse, it mapped any of the six codes to
ITEM_LOGIN_REQUIRED specifically, so a matched PASSWORD_RESET_REQUIRED would
have been recorded under the wrong code. It is the same message-parsing that
never worked before; keeping it as a fallback only preserved a wrong mapping
for a case that cannot arise.
An uncoded failure now stays uncoded, which is what the poller wants: retry
it, rather than park the connection behind a reconnect prompt the user
cannot act on.
One list drove two unrelated decisions — whether to raise a "Reconnect" prompt, and whether the poller should give up — and the two disagree for STALE_DATA. That code means Plaid's own updates have been failing for over a day, which re-linking often fixes and which we already email the user about. Keeping it out of the list dropped the button while the email kept telling them to press it; putting it in would have parked the connection forever, even though the institution can recover on its own and we would never find out. Split into needsUserAction() for the prompt, task, and email, and shouldStopPolling() for the poller's skip list. They differ by exactly one code, and that difference is the point. Adds PENDING_EXPIRATION and PENDING_DISCONNECT, which Plaid documents as update-mode triggers alongside ITEM_LOGIN_REQUIRED. Both fire *before* the Item breaks — PENDING_EXPIRATION about a week before consent lapses — so catching them is what lets someone repair a connection ahead of losing data rather than after. One live connection has consent expiring in October and would have gone dark unnoticed. Transient codes stay out of both lists. Plaid classifies INSTITUTION_NOT_RESPONDING and INSTITUTION_DOWN as temporary outages, and re-linking cannot fix a bank that is not answering, so prompting would send the user on a pointless errand. Whether a "temporary" outage that persists for months should eventually escalate is a separate problem, and needs duration tracking this does not have.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Four independent bugs, all surfaced while debugging a reported balance mismatch (total balance read CA$109,944.96 while the account list showed CA$108,997.67 in Wise and $77,455.46 in CIBC).
1. Plaid errors recorded with a NULL error code —
24ce647The Plaid SDK is axios-based, so a rejected request's
messageis only ever"Request failed with status code 400"; the realerror_codesits in the response body, which the sync workflow never read.isDisconnectError()therefore never matched, so every broken item was written with a NULL code — never parked, never emailed, still reportingstatus=active. One production item logged 4,556 identical 400s over ten weeks, retried every 30 minutes.extractPlaidError()now reads the code off the response body, and the poller sharesPLAID_DISCONNECT_ERROR_CODESso a capturedITEM_LOGIN_REQUIREDactually stops the retry loop.notifyDisconnectfires only on true disconnect codes, so newly-captured transient codes (INSTITUTION_DOWN, rate limits) don't spam.Also fixes reconnects appearing to do nothing: Plaid pulls from the bank asynchronously after Link, and the wait-and-retry loop for that was gated on
cursor === null, which a reauth preserves. The Link path now passesawaitUpstreamPull.2. Quiltt data stamped USD —
c2b5620Quiltt account rows hardcoded
isoCurrencyCode: "USD"and never even requestedcurrencyCodefrom the GraphQL API, so a CIBC chequing account reporting CAD was filed as USD. Transactions had the same fallback. Plaid and MX both storenullrather than guessing.Accounts now read Quiltt's
currencyCode; transactions resolve transaction → parent account → null. Nothing invents a currency, and a CAD account's transactions stay labelled CAD.3. Checkout success taken on trust —
e82e708The success toast fired on the browser-controlled
?checkout=successparam, which says nothing about whether the plan was applied — and raced the webhook that actually writes it. A user could pay, be told "Subscription activated!", then immediately hit the connection-limit modal. Locally the webhook never arrives at all withoutstripe listen, so checkout could never take effect.The client now confirms through
/api/billing/confirm-checkout, which applies the plan only whenpayment_statusis paid andclient_reference_idmatches the caller, so a session id can't be replayed against another account. The webhook stays an idempotent backstop and remains the source of truth for renewals and cancellations.4. Total balance showed only the largest currency —
e20af46The home header bucketed accounts by currency, summed each bucket, sorted, and rendered only the first — so a portfolio holding CAD and USD displayed the CAD line alone and the USD side never appeared anywhere. Combined with #2, that is the reported mismatch: CIBC's CAD balance was filed under USD, and the USD bucket was the one being dropped.
There is no FX data in the app, so collapsing the buckets into one figure would mean inventing a rate. Every non-zero currency now gets its own equal-weight line, largest first, with its ISO code (
CAD 182,180.48) so no line reads as a subtotal of another; a lone currency keeps its symbol form. Zero buckets are dropped — a Wise connection carries a sub-account per currency and most sit at 0.Header and carousel move into
AccountsOverviewso this is exercisable in Storybook (5 stories); the route itself can't be, as it depends on auth session, link context, and API fetches.Testing
server: 105 tests pass, including new coverage for a thrown Plaid error, the Quiltt account and transaction currency paths, and checkout confirmation — none of those paths were exercised before, which is why these shipped.client:svelte-check0 errors,buildandbuild-storybookboth pass. The multi-currency logic was additionally verified against the real production account rows: it reproduces the reported CA$109,944.96 exactly before the fix, and yields the correct per-currency lines after.Notes for the reviewer
SYNC_UPDATES_AVAILABLEwebhook is the real long-term fix for Add Resend email service #1 — there is no webhook support at all today. Deliberately out of scope.🤖 Generated with Claude Code