fix(adapters): a query error names its operation and table on every dialect - #1807
Conversation
…ialect handleQueryError attached its context only when the message did not already contain the operation's name. Drizzle spells its statements in lower case and, on PostgreSQL and MySQL, wraps a driver error as 'Failed query: <sql>', so an update, insert or delete there - or any table or column whose name contains the word - arrived without it. The check now asks for the context the method writes, which also keeps a re-handled error from stacking it. This is what turned main's Integration (postgres) and (mysql) legs red: the transactional update twins added in #1787 assert the context, and their table is named int_txpg_update_table.
…y-their-context-on-every-dialect
|
Warning Review limit reachedNext included review available in 15 minutes. View limit detailsLimit details: You’ve used the included review currently available. You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Review configuration: ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
github-actions
Bot
added
type: docs
PR title fails Conventional Commits checkExamples of valid titles:
|
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
@nextlyhq/adapter-drizzle
@nextlyhq/adapter-mysql
@nextlyhq/adapter-postgres
@nextlyhq/adapter-sqlite
@nextlyhq/admin
@nextlyhq/admin-css
@nextlyhq/blocks-engine
@nextlyhq/blocks-react
@nextlyhq/builder
create-nextly-app
@nextlyhq/eslint-plugin
nextly
@nextlyhq/plugin-form-builder
@nextlyhq/plugin-page-builder
@nextlyhq/plugin-sdk
@nextlyhq/plugin-seo
@nextlyhq/storage-s3
@nextlyhq/storage-uploadthing
@nextlyhq/storage-vercel-blob
@nextlyhq/ui
commit: |
1 participant
What
Repairs
main's red Integration (postgres) and Integration (mysql) legs, which my #1787 turned red, by fixing the adapter behaviour the failing assertion caught: a query error now carries its operation-and-table context on every dialect.Ledger:
task:query-errors-carry-their-context, fixingfinding:query-error-context-skipped-on-pg-mysql; researchresearch:query-error-context-check.What was true on
main(95cfddac3)adapter-{postgres,mysql}/…/transaction-update-writes-the-table.integration.test.ts› refuses a column the table does not have, naming the operation and the table. Received: aDatabaseErrorwithtable: "int_txpg_update_table"and messageFailed query: UPDATE "int_txpg_update_table" …; expected/update operation failed.*ghost/s. feat(adapters): the transactional update writes the physical table, as the insert does #1787's merge commit had these legs skipped by the newer-push rule, so the next push tomainwas their first run; the twins self-skip locally (no Docker here), so the SQLite twin was the only one I ran. That is on me.DrizzleAdapter.handleQueryError: it prefixes<op> operation failed on table '<t>':onlyif (!message.includes(operation))— a proxy for "the context is not there yet". The table's name containsupdate, so the proxy said yes.pg-core/dialect.js:delete from,update … set,insert into;mysql-corethe same) and wraps a driver error asFailed query: <sql>\nparams: …(errors.js). So on PostgreSQL and MySQL the message names the operation for every query-builder statement, and the context has effectively never been attached there.How
handleQueryErrornow asks whether the context it writes is present —message.includes("<op> operation failed on table '<t>'")— rather than whether the operation's bare name appears. Same method, same shape, attached once however often one error is handled (classifyErrorreturns an existingDatabaseErrorunchanged, so a re-handled error must not stack the prefix).Who reads the message:
git grep 'operation failed'finds only the three twin assertions; no product code parses it, and nextly maps aDatabaseErrorto its public response by kind (NextlyError.fromDatabaseError), never by driver text. So this changes diagnostic text only, on PostgreSQL and MySQL, where the context was missing.Verification
adapter-drizzle/src/__tests__/adapter.test.ts(they run everywhere, unlike the pg/mysql twins): a Drizzle-shaped lower-case statement message gets the context; a message where only a name contains the operation gets it; handling the same error twice attaches it once.c42424c95(main746a7141cmerged in): adapter-drizzle, -sqlite, -postgres, -mysql lint 0 and check-types 0 (after rebuilding adapter-drizzle's dist); unit 184 / 66 / 87 / 47; SQLite integration twin 35/35;pnpm exec fallow audit --changed-since origin/main --gate new-only(3.15.0) pass, 0 introduced.95cfddac3+ this change: 1,654 passed, 1 failed —bulk-update-override-access› writes a field whose own rule refuses the caller when elevated, whichmain's own SQLite leg fails identically at95cfddac3(the unmodified control; job 103352224752). It is the other half of feat(adapters): the transactional update writes the physical table, as the insert does #1787's fallout — the tx path now bumpsupdated_at, so a patch whose only field is stripped is no longer empty — and the wt-forms lane fixed it onmainin test(nextly): an un-elevated batch row with its only field stripped is a no-op #1806; merged in here, that file passes 4/4.main's pg/mysql legs stop at the adapter twins before reaching nextly's suite, so they stay red until this lands even with test(nextly): an un-elevated batch row with its only field stripped is a no-op #1806 in.Integration (postgres)andIntegration (mysql)by name on this head.One changeset (all 25 packages, patch).