MSSQL: tedious driver's RequestError should be wrapped in DBError

[jeremy-w]

For working with SQL Server, knex has switched from node-mssql to tedious, and thus so has Objection 3.

Tedious throws a RequestError to present SQL errors encountered during execution, such as:

RequestError {
  message: "insert into [programType] ([CreatedBy], [ModifiedBy], [programTypeKey]) output inserted.[programTypeKey] values (@p0, @p1, @p2) - Violation of PRIMARY KEY constraint 'PK_programType_programTypeKey'. Cannot insert duplicate key in object 'dbo.programType'. The duplicate key value is (Minesweeper).",
  code: 'EREQUEST',
  number: 2627,
  state: 1,
  class: 14,
  serverName: '49df3e21e80c',
  procName: '',
  lineNumber: 1,
  originalStack: "RequestError: insert into [programType] ([CreatedBy], [ModifiedBy], [programTypeKey]) output inserted.[programTypeKey] values (@p0, @p1, @p2) - Violation of PRIMARY KEY constraint 'PK_programType_programTypeKey'. Cannot insert duplicate key in object 'dbo.programType'. The duplicate key value is (Minesweeper).\n" +
    '    at Parser.<anonymous> (SRCROOT/node_modules/tedious/lib/connection.js:1177:27)\n' +
    '    at Parser.emit (events.js:314:20)\n' +
    '    at Readable.<anonymous> (SRCROOT/node_modules/tedious/lib/token/token-stream-parser.js:27:14)\n' +
    '    at Readable.emit (events.js:314:20)\n' +
    '    at addChunk (SRCROOT/node_modules/readable-stream/lib/_stream_readable.js:298:12)\n' +
    '    at readableAddChunk (SRCROOT/node_modules/readable-stream/lib/_stream_readable.js:280:11)\n' +
    '    at Readable.Object.<anonymous>.Readable.push (SRCROOT/node_modules/readable-stream/lib/_stream_readable.js:241:10)\n' +
    '    at SRCROOT/node_modules/readable-stream/lib/internal/streams/from.js:49:29\n' +
    '    at Generator.next (<anonymous>)\n' +
    '    at asyncGeneratorStep (SRCROOT/node_modules/readable-stream/lib/internal/streams/from.js:3:103)'
} undefined

(It throws a ConnectionError for connection-level issues. I don't know if db-errors aims to cover connection-related errors rather than statement-related ones.)

Minimally, wrapping every RequestError into a DBError would enable trapping database errors as a client of Objection straightforwardly, without having to depend on details of the underlying engine.

To vend a more specific subclass, such as The specific error flavor can be identified by the RequestError.number, which corresponds to the SQL Server messageId:

1> declare @en_US integer = 1033; declare @RequestError_number integer = 2627; select * from sys.messages where language_id = @en_US and message_id = @RequestError_number
2> go
message_id language_id severity is_event_logged text
---------- ----------- -------- --------------- ----
2627 1033 14 0 Violation of %ls constraint '%.*ls'. Cannot insert duplicate key in object '%.*ls'. The duplicate key value is %ls.

(1 rows affected)

Configuration

• [email protected]
• [email protected]
• [email protected]
• [email protected]
• SQL Server 2019 Developer Edition

Related Work

It looks like some steps were taken towards fixing this as part of the commit "Migrate tests to GitHub actions", which appears to make more changes than just switching CI. However, the mssql tests are presently failing.

Workaround

Clients of db-errors can look for RequestError directly and inspect its code and message.

[jeremy-w]

In practice, I wound up working around this by going from "check is DBError then scrutinize error message" to "just scrutinize the error message". This worked because I was very specifically looking for the named, violated constraint. This led to a series of changes like:

diff --git a/src/seeds/c015_programStatuses.ts b/src/seeds/c015_programStatuses.ts
index 1322130..728043f 100644
--- a/src/seeds/c015_programStatuses.ts
+++ b/src/seeds/c015_programStatuses.ts
@@ -44,11 +44,9 @@ export async function seedProgramStatuses(
         ModifiedBy: CREATOR,
       })
     } catch (error) {
-      const indicatesSeedingHasAlreadySucceeded =
-        error instanceof DBError &&
-        error.message.match(
-          /Violation of PRIMARY KEY constraint 'PK_programStatus_programStatusKey'/
-        )
+      const indicatesSeedingHasAlreadySucceeded = error.message.match(
+        /Violation of PRIMARY KEY constraint 'PK_programStatus_programStatusKey'/
+      )
       if (!indicatesSeedingHasAlreadySucceeded) {
         log(error)
         throwErrorIfTesting(error)

[simon-kolkmann]

I've opened #28 to fix the failing tests. Otherwise the package seems to handle mssql errors correctly already, but a new release is necessary to include the latest changes in objection.