better-sqlite3
better-sqlite3 copied to clipboard
new Database ( incorrectPathToDB, { fileMustExist: true } ) does not throw error (while readOnly misspell does)
Unless I’m too sleep deprived and missing something obvious, I can’t find any logic to throw an error on incorrect DB file path (there’s only a partial check for directory existence "Cannot open database because the directory does not exist").
So the new class inst code proceeds to attempt to create a temp DB and look for a non-existent table inside. No error caught:
Does not work — no error caught
try {
let db = new betterSQLite3( incorrectPathToDB, {
readonly: false,
fileMustExist: true,
} );
}
catch ( error ) {
console.log( 'Disaster. All is... Wait... we didn’t catch anything?' ) // not shown in the console
console.log( error ); // nothing... silence
}
Proceeding to:
node_modules/better-sqlite3/lib/methods/wrappers.js:5
return this[cppdb].prepare(sql, this, false);
^
SqliteError: no such table: test_table
Works — error caught
try {
let db = new betterSQLite3( correctPathToDB, {
readOnly: false,
fileMustExist: true,
} );
}
catch ( error ) {
console.log( 'Disaster. All is lost. But we have the details:' )
console.log( error ); // Misspelled option "readOnly" should be "readonly"
}
using the:
if ('readOnly' in options) throw new TypeError('Misspelled option "readOnly" should be "readonly"');
Related to:
#220
This works as expected for me:
const fs = require('fs');
const betterSQLite3 = require('better-sqlite3');
const incorrectPathToDB = 'nope.db'
try {
console.log(fs.existsSync(incorrectPathToDB));
let db = new betterSQLite3( incorrectPathToDB, {
readonly: false,
fileMustExist: true,
} );
}
catch ( error ) {
console.log( 'Disaster. All is... Wait... we didn’t catch anything?' ) // not shown in the console
console.log( error ); // nothing... silence
}
> node index.js
false
Disaster. All is... Wait... we didn’t catch anything?
SqliteError: unable to open database file
at new Database (/tmp/sql/node_modules/better-sqlite3/lib/database.js:51:26)
at Object.<anonymous> (/tmp/sql/index.js:8:12)
at Module._compile (internal/modules/cjs/loader.js:1068:30)
at Object.Module._extensions..js (internal/modules/cjs/loader.js:1097:10)
at Module.load (internal/modules/cjs/loader.js:933:32)
at Function.Module._load (internal/modules/cjs/loader.js:774:14)
at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:72:12)
at internal/main/run_main_module.js:17:47
Maybe your path is an empty string and you're creating a temporary database? Turns out this is not documented that an empty string does that (it's SQLite behavior) https://github.com/JoshuaWise/better-sqlite3/blob/02c9c250bc77311b08bec574d3d40890c0b17256/lib/database.js#L33
@JoshuaWise arguably if(fileMustExist && anonymous)
should throw because they contradict and are incompatible options? Around here somewhere
https://github.com/JoshuaWise/better-sqlite3/blob/02c9c250bc77311b08bec574d3d40890c0b17256/lib/database.js#L39-L44
Also can't reproduce this. As @Prinzhorn said, probably incorrectPathToDB
is an empty string or undefined
, both of which cause SQLite3 to open a new temporary database (which means whatever table you're trying to access doesn't exist, because the database is empty).
@JoshuaWise arguably
if(fileMustExist && anonymous)
should throw because they contradict and are incompatible options? Around here somewherehttps://github.com/JoshuaWise/better-sqlite3/blob/02c9c250bc77311b08bec574d3d40890c0b17256/lib/database.js#L39-L44
Perhaps that could be a helpful error message.
@Prinzhorn & @JoshuaWise — Yes, if path is passed through an incorrectPathToDB
var which is undefined, causes the "creation of a new temporary database (which means whatever table you're trying to access doesn't exist, because the database is empty)".
Exactly. 😀
In my view, that’s not the behavior described by fileMustExist: true
.
IMO:
- at the very least — we’d want to log a
Creating new empty DB
message. -
better — with the
fileMustExist: true
, we would need to throw an error in any case exceptcorrect file found
. Using, as @Prinzhorn suggested:
fs.existsSync(str_PathToDB)
My initial concern was raised when I just hardcoded a ./testAwesome.db
as the first param, not even using a var.
@JoshuaWise ’s comment somewhere that "we don’t use any fancy path logic" inspired me to input an absolute path and the DB was correctly accessed without any complaints of inexisting tables.
OK, I drilled in a bit further down my commits… it turns out it is rather an edge case.
To very quickly test a try/catch, I add an inline or outside + 1
to a param (which might mess JS truthy/falsy, var type etc). Somehow, here, it does not throw an error.
To reproduce:
const correctPathToDB = '/Users/johndoe/Applications/GoodApp/perfect.db'
const incorrectPathToDB = correctPathToDB + 1;
console.log( `Incorrect path is: ${incorrectPathToDB}` );
console.log( `Var type is: ${typeof incorrectPathToDB}` ); // string, passing a 'Expected first argument to be a string' check
try {
console.log( fs.existsSync( incorrectPathToDB ) );
let db = new betterSQLite3( incorrectPathToDB, {
readonly: false,
fileMustExist: true,
} );
}
catch ( error ) {
console.log( 'Disaster. All is... Wait... we didn’t catch anything?' ) // not shown in the console
console.log( error ); // nothing... silence
}
This silently creates a /Users/johndoe/Applications/GoodApp/perfect.db1
and then complains about inexisting tables.
My suggestion
- add a
Creating new DB perfect.db1
message.… that would help understand the silence - add the path format requirements for the DB file location param
In my case, a relative path did not work and I assembled an absolute using Node's path
and url
.
Assuming fantastic.db
is in the root of the app’s folder:
import * as path from 'path';
import * as url from 'url';
/**
* - The import.meta object exposes context-specific metadata to a JavaScript
* module. It contains the module's URL.
* - url.fileURLToPath() converts a URL into a OS-independent absolute path
* - path.dirname() returns the directory of a path without a trailing slash
*/
const correctPathToDB = path.dirname( url.fileURLToPath(
import.meta.url ) ) + '/' + 'perfect.db';
If you were able to reproduce the problem with a non-empty path argument, it sounds like this is a bug in your operating system. The fileMustExist
option simply causes us to pass SQLITE_OPEN_READWRITE
instead of (SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE)
to SQLite's sqlite3_open_v2()
C function. SQLite then converts those flags to filesystem flags. It looks like that's the part that's failing.
Woha! Thanks for the clear-up of the logic behind the fileMustExist
option. Lemmy test from the SQLite command line tool when I get a second.
Meanwhile, would you be interested to add an if ( fs.existsSync(str_PathToDB) )
check as @Prinzhorn suggested above? That would bomb-proof the arg even further and avoid relying on SQLite’s C interface.
@JoshuaWise would love this to be solved?
i'm using better-sqlite3 with docker & its very confusing which folder does not exist.
i would love a better description of the error message than this one. having seen this error 100x today.
// Make sure the specified directory exists
if (!anonymous && !fs.existsSync(path.dirname(filename))) {
throw new TypeError('Cannot open database because the directory ' + path.dirname(filename) + 'does not exist');
}
wouldn't this be enough here?
edit: i guess i can log the url
before it gets called although the above would still be an improvement.
console.log(url)
const client = sqlite(url, { verbose: console.log })