Skip to content

OUT-3742: tighten QBO Fault handling + bind SQL projections to row schemas - #251

Merged
SandipBajracharya merged 5 commits into
masterfrom
OUT-3742
May 15, 2026
Merged

OUT-3742: tighten QBO Fault handling + bind SQL projections to row schemas#251
SandipBajracharya merged 5 commits into
masterfrom
OUT-3742

Conversation

@SandipBajracharya

@SandipBajracharya SandipBajracharya commented May 15, 2026

Copy link
Copy Markdown
Collaborator

Problem

  • assertNotQBFault only read code on the non-array branch, so APIError.status was always 400 for real QBO faults.
  • 8 hand-written QBO SELECT projections drifted from their row schemas — typos/required-key changes only surfaced at runtime.

Shipped

  • Tightened QBFaultSchema; simplified assertNotQBFault to read errors[0]?.code. Clamp non-HTTP statuses in withErrorHandler.
  • 4 typed constants (QB_INVOICE_COLUMNS, QB_ITEM_COLUMNS, QB_CUSTOMER_COLUMNS, QB_ACCOUNT_COLUMNS) bind each SELECT to its row schema via satisfies. Regression test guards required-key supersets.
  • Drive-bys: handleInvoiceDeleted throws on non-voided delete; getMessageAndCodeFromError guards an empty errors[].

Code sites

  • src/type/dto/intuitAPI.dto.ts
  • src/utils/intuitAPI.ts
  • src/app/api/core/utils/withErrorHandler.ts
  • src/app/api/quickbooks/invoice/invoice.service.ts
  • src/utils/error.ts
  • test/unit/utils/intuitAPI.{responses,queryColumns}.test.ts

Behavior change

  • qb_sync_logs.failed_record_category for live 6000/6190 faults now resolves to ACCOUNT (was QB_API_ERROR) because getCategory reads the real status. HTTP response still 400.

@linear-code

linear-code Bot commented May 15, 2026

Copy link
Copy Markdown

OUT-3742

@vercel

vercel Bot commented May 15, 2026

Copy link
Copy Markdown

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

Project Deployment Actions Updated (UTC)
quickbooks-sync Building Building May 15, 2026 0:33am
quickbooks-sync (dev) Ready Ready Preview, Comment May 15, 2026 0:33am

Request Review

@SandipBajracharya SandipBajracharya changed the title fix(OUT-3742): assertNotQBFault extracts code from Fault.Error[0] OUT-3742: assertNotQBFault extracts code from Fault.Error[0] May 15, 2026
@greptile-apps

greptile-apps Bot commented May 15, 2026

Copy link
Copy Markdown

Greptile Summary

This PR fixes two related bugs: assertNotQBFault was always producing status 400 for QBO faults because it only read code on the non-array branch, and 8 SQL SELECT projections had drifted from their row schemas. The changes introduce a typed QBFaultErrorSchema with safe code coercion, four QB_*_COLUMNS constants bound to their schemas via satisfies, and a status clamp in withErrorHandler for non-HTTP QBO codes.

  • Fault handling: QBFaultSchema.Fault.Error is now a strictly typed array; assertNotQBFault reads errors[0]?.code; withErrorHandler clamps QBO fault codes (e.g. 6240) to 400 before returning the HTTP response.
  • SQL projection binding: Four QB_*_COLUMNS constants use as const satisfies ReadonlyArray<keyof RowType> so a typo or stale field name fails at compile time; a new regression test checks each constant stays a superset of its schema's required keys.
  • Drive-bys: handleInvoiceDeleted now throws on non-voided deletes; getMessageAndCodeFromError guards against an empty errors[].

Confidence Score: 4/5

Safe to merge with one fix: QBFaultSchema strict array check silently drops faults whose Error field is null or a non-array object.

The core fault-handling path now silently swallows a QBO Fault response whenever Fault.Error is null or arrives as a non-array shape — safeParse returns false and assertNotQBFault returns without throwing. The old schema (z.unknown().optional()) always succeeded and always threw. Replacing .optional().default([]) with .catch([]) closes the gap.

src/type/dto/intuitAPI.dto.ts — the QBFaultSchema.Fault.Error definition.

Important Files Changed

Filename Overview
src/type/dto/intuitAPI.dto.ts Introduces QBFaultErrorSchema with safe code coercion and tightens QBFaultSchema to expect a typed Error array; the strict array validation (.optional() without .catch) can silently swallow faults when Fault.Error is null or a non-array shape.
src/utils/intuitAPI.ts Simplifies assertNotQBFault to read errors[0]?.code, adds four typed QB_*_COLUMNS constants bound via satisfies to their row schemas, and expands several SELECT projections to use these constants.
src/app/api/core/utils/withErrorHandler.ts Clamps non-HTTP status codes (e.g., QBO 6240) to BAD_REQUEST before passing to NextResponse.json, preserving the original code in the errors payload.
src/utils/error.ts Guards getMessageAndCodeFromError against empty errors[] by using optional chaining with a fallback, and removes the unsafe IntuitErrorType cast in favour of QBFaultErrorSchemaType.
src/app/api/quickbooks/invoice/invoice.service.ts Changes handleInvoiceDeleted to throw instead of silently returning when a non-voided invoice delete is requested.
test/unit/utils/intuitAPI.queryColumns.test.ts New regression test asserting each QB_*_COLUMNS constant is a superset of its schema's required keys and contains no keys absent from the schema.

Sequence Diagram

sequenceDiagram
    participant QBO as QBO API
    participant IA as IntuitAPI
    participant WEH as withErrorHandler
    participant Log as qb_sync_logs

    QBO->>IA: "Fault { Error: [{code: 6000, ...}] }"
    IA->>IA: "assertNotQBFault()<br/>QBFaultSchema.safeParse()<br/>errors[0].code to 6000"
    IA->>WEH: throw APIError(6000, msg, errors)
    WEH->>WEH: "getMessageAndCodeFromError()<br/>code = 6000, source = intuit"
    WEH->>WEH: "getCategory(code=6000) to ACCOUNT"
    WEH->>WEH: "httpStatusOut = clamp(6000) to 400"
    WEH->>Log: "FAILED, category=ACCOUNT"
    WEH-->>QBO: "HTTP 400 { error, errors }"
Loading

Reviews (4): Last reviewed commit: "chore(OUT-3742): switch Fault.code to tr..." | Re-trigger Greptile

Comment thread src/type/dto/intuitAPI.dto.ts
@SandipBajracharya

Copy link
Copy Markdown
Collaborator Author

@greptileai

@SandipBajracharya SandipBajracharya changed the title OUT-3742: assertNotQBFault extracts code from Fault.Error[0] fix(OUT-3742): tighten QBO Fault handling + bind SQL projections to row schemas May 15, 2026
@SandipBajracharya

Copy link
Copy Markdown
Collaborator Author

@greptileai

@SandipBajracharya SandipBajracharya changed the title fix(OUT-3742): tighten QBO Fault handling + bind SQL projections to row schemas OUT-3742: tighten QBO Fault handling + bind SQL projections to row schemas May 15, 2026
@SandipBajracharya

Copy link
Copy Markdown
Collaborator Author

@greptileai

Comment thread src/type/dto/intuitAPI.dto.ts
SandipBajracharya and others added 5 commits May 15, 2026 18:16
QBO returns Fault.Error as a typed array (`[{ code, Message, Detail, element }]`).
The old code only read `code` from the non-array branch, so APIError.status always
fell back to 400 and the real fault code (e.g. 6240) survived only on errors[].
Tightens QBFaultSchema, simplifies assertNotQBFault to use errors[0]?.code with
a finite-check fallback, and clamps non-HTTP statuses in withErrorHandler before
NextResponse.json. Also guards getMessageAndCodeFromError against an empty
errors[] and surfaces a clearer failure when a non-voided invoice delete is
requested.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Replaces 8 hand-written SELECT projections in src/utils/intuitAPI.ts with 4
typed constants (QB_INVOICE_COLUMNS / QB_ITEM_COLUMNS / QB_CUSTOMER_COLUMNS /
QB_ACCOUNT_COLUMNS), each `as const satisfies ReadonlyArray<keyof <RowType>>`.
A typo or stale field name fails tsc; a regression test asserts every
schema-required key stays in the constant so a drop can't ZodError at runtime
on response parse.

Out of scope: resolveUniqueCustomerName (DisplayName-only queries against
Vendor/Employee that don't have shared row schemas) and _getCompanyInfo
(SELECT * with a single-field schema — no drift risk).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`z.coerce.number()` runs `Number(value)` then validates the result; Zod's
number validator rejects NaN. So a Fault payload with a non-numeric `code`
(e.g. "abc") would fail QBFaultSchema.safeParse and assertNotQBFault would
silently return — the fault is dropped and the caller never throws. The
prior comment claimed assertNotQBFault treats NaN as "no usable code", but
the NaN never reached it.

Wrap the code field with .catch(NaN) so a non-numeric value yields NaN
instead of a parse failure. assertNotQBFault's existing Number.isFinite
check handles NaN and falls back to BAD_REQUEST.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Old comment claimed the schema "coerces" and assertNotQBFault treats NaN
as no-code. Pre-.catch(NaN), Zod's number validator rejected NaN before it
ever reached assertNotQBFault. Reflect what's actually true now: missing
code short-circuits via .optional(); non-numeric code yields NaN via
.catch(NaN); either way Number.isFinite fails and BAD_REQUEST wins.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…est titles

- Schema: replace .coerce.number().catch(NaN).optional() with a transform
  that returns number-or-undefined. Output type is `number | undefined`,
  no NaN, no parse failure on malformed codes.
- assertNotQBFault: simplify the code check now that the schema can't
  yield NaN.
- Trim verbose comments across this branch's diff (clamp comment, schema
  comments, isQBODuplicateDocNumberError JSDoc, test-file comments) and
  refresh the stale handleInvoiceDeleted comment.
- Rewrite test it() titles in intuitAPI.responses.test.ts and
  intuitAPI.queryColumns.test.ts as plain English instead of programmer
  notes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@SandipBajracharya
SandipBajracharya changed the base branch from OUT-3710 to master May 15, 2026 12:31
@SandipBajracharya
SandipBajracharya merged commit a24fc5d into master May 15, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants