WiseLibs / better-sqlite3

The fastest and simplest library for SQLite3 in Node.js.
MIT License
5.48k stars 396 forks source link

SqliteError message is not enumerable, test frameworks tend to ignore it #633

Closed andykais closed 2 years ago

andykais commented 3 years ago

I created an issue in the ava test repo (https://github.com/avajs/ava/issues/2755) that explains how sqlite error messages are not shown. It looks like the cause of it is that the errors are not enumerable (see here https://github.com/JoshuaWise/better-sqlite3/blob/master/lib/sqlite-error.js#L13)

Errors end up looking like this:

  Error thrown in test:

  SqliteError {}

  › Database.prepare (node_modules/.pnpm/better-sqlite3@7.4.0/node_modules/better-sqlite3/lib/methods/wrappers.js:5:21)
  › test.js:7:6

Is there a reason error messages need to be not enumerable?

Prinzhorn commented 3 years ago

I don't think enumerability per se is the problem, see also https://github.com/JoshuaWise/better-sqlite3/issues/575#issuecomment-810831445

(new Error('test')).propertyIsEnumerable('message')
false
(new (class CustomError extends Error{})('test')).propertyIsEnumerable('message')
false

How does ava serialize the Error for printing? It looks like neither toString nor util.inspect

JoshuaWise commented 2 years ago

I've changed the code property to be enumerable, as of better-sqlite3 version 7.5.0.

Originally, I made it non-enumerable to match the message property that every error has. However, after reflecting on the Node.js and JavaScript ecosystem, I realized that custom properties of error objects are pretty much always enumerable, to aid in discoverability (i.e., so they appear when using console.log).