WiseLibs / better-sqlite3

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

Support an "undefined" value for the "timeout" option #1049

Closed fabiospampinato closed 1 year ago

fabiospampinato commented 1 year ago

Repro:

import sqlite3 from 'better-sqlite3';

sqlite3 ( ':memory:', {
  timeout: undefined
});

Error:

TypeError: Expected the "timeout" option to be a positive integer
    at new Database (/Users/fabio/Code/fabiospampinato/tiny-sqlite3/node_modules/better-sqlite3/lib/database.js:40:55)
    at Database (/Users/fabio/Code/fabiospampinato/tiny-sqlite3/node_modules/better-sqlite3/lib/database.js:11:10)
    at file:///Users/fabio/Code/fabiospampinato/tiny-sqlite3/asd.js:4:1
    at ModuleJob.run (node:internal/modules/esm/module_job:193:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:530:24)
    at async loadESM (node:internal/process/esm_loader:91:5)
    at async handleMainPromise (node:internal/modules/run_main:65:12)

Throwing for undefined options makes some things very inconvenient for no reason basically, so it shouldn't happen, imo.

mceachen commented 1 year ago

@JoshuaWise has tests that specifically assert against undefined, so this wasn’t an API mistake.

fabiospampinato commented 1 year ago

It could be intentional, but isn't this being intentional the mistake then? Like what's the benefit of this check?