From 4929899af434e72b4563947eb25e1e30a6c13532 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ju=CC=88rg=20Lehni?= Date: Fri, 16 Oct 2026 00:20:29 +0200 Subject: [PATCH] Wrap the errors of knex's better-sqlite3 client as database errors --- doc/release-notes/changelog.md | 1 + .../CheckViolationError/parser.js | 11 +- .../ForeignKeyViolationError/parser.js | 11 +- .../NotNullViolationError/parser.js | 6 +- .../UniqueViolationError/parser.js | 6 +- .../constraintMessage.js | 14 +++ .../ConstraintViolationError/parser.js | 3 +- lib/dbErrors/parsers/sqlite/DBError/parser.js | 3 +- tests/unit/dbErrors/sqlite.js | 116 ++++++++++++++++++ 9 files changed, 147 insertions(+), 24 deletions(-) create mode 100644 lib/dbErrors/parsers/sqlite/DBError/ConstraintViolationError/constraintMessage.js create mode 100644 tests/unit/dbErrors/sqlite.js diff --git a/doc/release-notes/changelog.md b/doc/release-notes/changelog.md index d08a0fa8c..00d1c2134 100644 --- a/doc/release-notes/changelog.md +++ b/doc/release-notes/changelog.md @@ -120,6 +120,7 @@ - `insertGraph()` and `upsertGraph()` stay under the number of values that a query can bind on SQL Server, PostgreSQL and SQLite when they insert, patch and remove many rows, and fetch many roots or related models - `upsertGraphAndFetch()` loads the relations of a `withGraphFetched()` on its query with their modifiers, named modifiers, aliases and options into the fetched models, like `insertGraphAndFetch()`, instead of ignoring the modifiers and aliases [#839](https://github.com/Vincit/objection.js/issues/839) - The error of `upsertGraph()` queries with other query builder calls names `$query()`, which adds such a call too [#1168](https://github.com/Vincit/objection.js/issues/1168) +- Errors of knex's `better-sqlite3` client are wrapped like those of `sqlite3`, e.g. a unique violation as a `UniqueViolationError` with its table and columns, where they reached the app as the driver's `SqliteError`, also in 3.x ### Types diff --git a/lib/dbErrors/parsers/sqlite/DBError/ConstraintViolationError/CheckViolationError/parser.js b/lib/dbErrors/parsers/sqlite/DBError/ConstraintViolationError/CheckViolationError/parser.js index b6ca9898b..1c2ae5726 100644 --- a/lib/dbErrors/parsers/sqlite/DBError/ConstraintViolationError/CheckViolationError/parser.js +++ b/lib/dbErrors/parsers/sqlite/DBError/ConstraintViolationError/CheckViolationError/parser.js @@ -1,19 +1,12 @@ import { CheckViolationError } from '../../../../../errors/CheckViolationError.js'; - -const REGEX = /SQLITE_CONSTRAINT: CHECK constraint failed/; +import { getConstraintMessage } from '../constraintMessage.js'; export const checkViolationErrorParser = { error: CheckViolationError, parse: (err) => { - const match = REGEX.exec(err.message); - - if (!match) { - return null; - } - // No way to extract anything reliably. - return {}; + return getConstraintMessage(err, 'CHECK') ? {} : null; }, subclassParsers: [], diff --git a/lib/dbErrors/parsers/sqlite/DBError/ConstraintViolationError/ForeignKeyViolationError/parser.js b/lib/dbErrors/parsers/sqlite/DBError/ConstraintViolationError/ForeignKeyViolationError/parser.js index 67d0ba7fa..2f435783f 100644 --- a/lib/dbErrors/parsers/sqlite/DBError/ConstraintViolationError/ForeignKeyViolationError/parser.js +++ b/lib/dbErrors/parsers/sqlite/DBError/ConstraintViolationError/ForeignKeyViolationError/parser.js @@ -1,19 +1,12 @@ import { ForeignKeyViolationError } from '../../../../../errors/ForeignKeyViolationError.js'; - -const REGEX = /SQLITE_CONSTRAINT: FOREIGN KEY constraint failed/; +import { getConstraintMessage } from '../constraintMessage.js'; export const foreignKeyViolationErrorParser = { error: ForeignKeyViolationError, parse: (err) => { - const match = REGEX.exec(err.message); - - if (!match) { - return null; - } - // No way to extract anything reliably. - return {}; + return getConstraintMessage(err, 'FOREIGN KEY') ? {} : null; }, subclassParsers: [], diff --git a/lib/dbErrors/parsers/sqlite/DBError/ConstraintViolationError/NotNullViolationError/parser.js b/lib/dbErrors/parsers/sqlite/DBError/ConstraintViolationError/NotNullViolationError/parser.js index 6a7e2cb0b..84952a6d5 100644 --- a/lib/dbErrors/parsers/sqlite/DBError/ConstraintViolationError/NotNullViolationError/parser.js +++ b/lib/dbErrors/parsers/sqlite/DBError/ConstraintViolationError/NotNullViolationError/parser.js @@ -1,12 +1,14 @@ import { NotNullViolationError } from '../../../../../errors/NotNullViolationError.js'; +import { getConstraintMessage } from '../constraintMessage.js'; -const NOT_COLUMN_REGEX = /SQLITE_CONSTRAINT: NOT NULL constraint failed: (.+)$/; +const NOT_COLUMN_REGEX = /^NOT NULL constraint failed: (.+)$/s; export const notNullViolationErrorParser = { error: NotNullViolationError, parse: (err) => { - const colMatch = NOT_COLUMN_REGEX.exec(err.message); + const message = getConstraintMessage(err, 'NOT NULL'); + const colMatch = message && NOT_COLUMN_REGEX.exec(message); if (!colMatch) { return null; diff --git a/lib/dbErrors/parsers/sqlite/DBError/ConstraintViolationError/UniqueViolationError/parser.js b/lib/dbErrors/parsers/sqlite/DBError/ConstraintViolationError/UniqueViolationError/parser.js index 6612119dd..699b3ee01 100644 --- a/lib/dbErrors/parsers/sqlite/DBError/ConstraintViolationError/UniqueViolationError/parser.js +++ b/lib/dbErrors/parsers/sqlite/DBError/ConstraintViolationError/UniqueViolationError/parser.js @@ -1,12 +1,14 @@ import { UniqueViolationError } from '../../../../../errors/UniqueViolationError.js'; +import { getConstraintMessage } from '../constraintMessage.js'; -const UNIQUE_COLUMNS_REGEX = /SQLITE_CONSTRAINT: UNIQUE constraint failed: (.+)$/; +const UNIQUE_COLUMNS_REGEX = /^UNIQUE constraint failed: (.+)$/s; export const uniqueViolationErrorParser = { error: UniqueViolationError, parse: (err) => { - const colsMatch = UNIQUE_COLUMNS_REGEX.exec(err.message); + const message = getConstraintMessage(err, 'UNIQUE'); + const colsMatch = message && UNIQUE_COLUMNS_REGEX.exec(message); if (!colsMatch) { return null; diff --git a/lib/dbErrors/parsers/sqlite/DBError/ConstraintViolationError/constraintMessage.js b/lib/dbErrors/parsers/sqlite/DBError/ConstraintViolationError/constraintMessage.js new file mode 100644 index 000000000..e33106335 --- /dev/null +++ b/lib/dbErrors/parsers/sqlite/DBError/ConstraintViolationError/constraintMessage.js @@ -0,0 +1,14 @@ +// Return the text of the SQLite error `err` from ` constraint failed` on, +// e.g. 'UNIQUE constraint failed: t.a', if it's a constraint error of that kind, +// or else `null`. `better-sqlite3` names the kind in its extended code, e.g. +// `SQLITE_CONSTRAINT_UNIQUE`, `sqlite3` only after `SQLITE_CONSTRAINT: `. knex +// puts the SQL with its values in front, so the last match is SQLite's own. +export function getConstraintMessage(err, kind) { + const text = `${kind} constraint failed`; + const isKind = + err.code === `SQLITE_CONSTRAINT_${kind.replace(' ', '')}` || + (err.code === 'SQLITE_CONSTRAINT' && + /.*SQLITE_CONSTRAINT: (.*)$/s.exec(err.message)?.[1].startsWith(text)); + + return isKind ? err.message.slice(err.message.lastIndexOf(text)) : null; +} diff --git a/lib/dbErrors/parsers/sqlite/DBError/ConstraintViolationError/parser.js b/lib/dbErrors/parsers/sqlite/DBError/ConstraintViolationError/parser.js index fba42142e..c0cce31cc 100644 --- a/lib/dbErrors/parsers/sqlite/DBError/ConstraintViolationError/parser.js +++ b/lib/dbErrors/parsers/sqlite/DBError/ConstraintViolationError/parser.js @@ -8,7 +8,8 @@ export const constraintViolationErrorParser = { error: ConstraintViolationError, parse: (err) => { - if (err.code === 'SQLITE_CONSTRAINT') { + // `better-sqlite3` uses the extended codes, e.g. `SQLITE_CONSTRAINT_UNIQUE`: + if (/^SQLITE_CONSTRAINT(_|$)/.test(err.code)) { return {}; } else { return null; diff --git a/lib/dbErrors/parsers/sqlite/DBError/parser.js b/lib/dbErrors/parsers/sqlite/DBError/parser.js index 8e5a109d5..88dac2485 100644 --- a/lib/dbErrors/parsers/sqlite/DBError/parser.js +++ b/lib/dbErrors/parsers/sqlite/DBError/parser.js @@ -5,10 +5,11 @@ export const dbErrorParser = { error: DBError, parse: (err) => { + // `sqlite3` gives its errors an `errno`, `better-sqlite3` names them: if ( typeof err.code === 'string' && err.code.startsWith('SQLITE_') && - typeof err.errno === 'number' + (typeof err.errno === 'number' || err.name === 'SqliteError') ) { return { nativeError: err, diff --git a/tests/unit/dbErrors/sqlite.js b/tests/unit/dbErrors/sqlite.js new file mode 100644 index 000000000..b59273eee --- /dev/null +++ b/tests/unit/dbErrors/sqlite.js @@ -0,0 +1,116 @@ +import { describe, it, expect } from 'vitest'; +import { + wrapError, + DBError, + UniqueViolationError, + NotNullViolationError, + CheckViolationError, + ForeignKeyViolationError, +} from '../../../lib/dbErrors/index.js'; + +// The errors of the two SQLite drivers of knex, as they throw them, after the +// SQL that knex puts in front: `sqlite3` with an `errno` and the code in the +// message, `better-sqlite3` named `SqliteError`, with the extended codes and +// without the code in the message. +function createSqlite3Error(code, errno, message, sql) { + return Object.assign(new Error(`${sql} - ${code}: ${message}`), { code, errno }); +} + +function createBetterSqlite3Error(code, message, sql) { + const err = Object.assign(new Error(`${sql} - ${message}`), { code }); + err.name = 'SqliteError'; + return err; +} + +describe('SQLite database errors', () => { + for (const [driver, createError] of [ + [ + 'sqlite3', + (code, extendedCode, message) => + createSqlite3Error(code, code === 'SQLITE_ERROR' ? 1 : 19, message), + ], + [ + 'better-sqlite3', + (code, extendedCode, message, sql = 'insert into t') => + createBetterSqlite3Error(extendedCode, message, sql), + ], + ]) { + describe(driver, () => { + it('should wrap unique violations', () => { + const err = wrapError( + createError( + 'SQLITE_CONSTRAINT', + 'SQLITE_CONSTRAINT_UNIQUE', + 'UNIQUE constraint failed: t.a, t.b', + ), + ); + expect(err).toBeInstanceOf(UniqueViolationError); + expect(err).toMatchObject({ table: 't', columns: ['a', 'b'], client: 'sqlite' }); + }); + + it('should wrap not null violations', () => { + const err = wrapError( + createError( + 'SQLITE_CONSTRAINT', + 'SQLITE_CONSTRAINT_NOTNULL', + 'NOT NULL constraint failed: t.b', + ), + ); + expect(err).toBeInstanceOf(NotNullViolationError); + expect(err).toMatchObject({ table: 't', column: 'b' }); + }); + + it('should wrap check violations', () => { + const err = wrapError( + createError( + 'SQLITE_CONSTRAINT', + 'SQLITE_CONSTRAINT_CHECK', + 'CHECK constraint failed: c > 0', + ), + ); + expect(err).toBeInstanceOf(CheckViolationError); + }); + + it('should wrap foreign key violations', () => { + const err = wrapError( + createError( + 'SQLITE_CONSTRAINT', + 'SQLITE_CONSTRAINT_FOREIGNKEY', + 'FOREIGN KEY constraint failed', + ), + ); + expect(err).toBeInstanceOf(ForeignKeyViolationError); + }); + + it('should read the kind from the error, not from the values in the SQL', () => { + const err = wrapError( + createError( + 'SQLITE_CONSTRAINT', + 'SQLITE_CONSTRAINT_CHECK', + 'CHECK constraint failed: c > 0', + "insert into t (a) values ('SQLITE_CONSTRAINT: UNIQUE constraint failed: x.y')", + ), + ); + expect(err).toBeInstanceOf(CheckViolationError); + }); + + it('should read the columns from the error, not from the values in the SQL', () => { + const err = wrapError( + createError( + 'SQLITE_CONSTRAINT', + 'SQLITE_CONSTRAINT_UNIQUE', + 'UNIQUE constraint failed: t.a', + "insert into t (a) values ('UNIQUE constraint failed: x.y')", + ), + ); + expect(err).toMatchObject({ table: 't', columns: ['a'] }); + }); + + it('should wrap other errors', () => { + const err = wrapError(createError('SQLITE_ERROR', 'SQLITE_ERROR', 'no such table: nope')); + expect(err).toBeInstanceOf(DBError); + expect(err).not.toBeInstanceOf(UniqueViolationError); + }); + }); + } +});