Skip to content

Commit a168d36

Browse files
committed
test coverage improvements
1 parent a6ee66f commit a168d36

10 files changed

Lines changed: 880 additions & 3 deletions

File tree

.github/workflows/ci.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -212,6 +212,13 @@ jobs:
212212
env:
213213
GH_TOKEN: ${{ github.token }}
214214

215+
- name: Upload coverage reports to Codecov
216+
uses: codecov/codecov-action@v5
217+
if: matrix.node == 24
218+
with:
219+
token: ${{ secrets.CODECOV_TOKEN }}
220+
slug: gms1/node-sqlite3
221+
215222
build-musl:
216223
permissions:
217224
contents: write

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ local.env
2626
setup.sh
2727
/build-tmp-napi-v3
2828
/build-tmp-napi-v6
29+
/coverage
2930
/.nyc_output
3031
*.tgz
3132
package-lock.json

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ Asynchronous, non-blocking [SQLite3](https://sqlite.org/) bindings for [Node.js]
1212
[![npm version](https://badge.fury.io/js/%40homeofthings%2Fsqlite3.svg)](https://badge.fury.io/js/%40homeofthings%2Fsqlite3)
1313
![NPM Downloads](https://img.shields.io/npm/dm/%40homeofthings%2Fsqlite3)
1414
[![Build Status](https://github.com/gms1/node-sqlite3/actions/workflows/ci.yml/badge.svg?branch=main)](https://github.com/gms1/node-sqlite3/actions/workflows/ci.yml)
15+
[![codecov](https://codecov.io/gh/gms1/node-sqlite3/graph/badge.svg?token=6H0X94BL3X)](https://codecov.io/gh/gms1/node-sqlite3)
1516
[![FOSSA Status](https://app.fossa.io/api/projects/git%2Bhttps%3A%2F%2Fgithub.com%2Fmapbox%2Fnode-sqlite3.svg?type=shield)](https://app.fossa.io/projects/git%2Bhttps%3A%2F%2Fgithub.com%2Fmapbox%2Fnode-sqlite3?ref=badge_shield)
1617
![Node-API v9 Badge](https://raw.githubusercontent.com/nodejs/abi-stable-node/doc/assets/Node-API%20v9%20Badge.svg)
1718

memory-bank/issues.md

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,61 @@
11
# issues
22

3+
## Uncatchable Native Addon Exceptions (C++ → Napi::Error)
4+
5+
Several operations on the native addon throw `Napi::Error` instances from C++ code that **cannot be caught by JavaScript `try/catch` or `Promise.catch()`**. These exceptions propagate as C++ exceptions through the N-API layer and terminate the process with an abort signal.
6+
7+
### Known uncatchable scenarios
8+
9+
#### 1. `Backup.step()` after `Backup.finish()`
10+
11+
Calling `step()` on a backup handle that has already been finished throws:
12+
```
13+
terminate called after throwing an instance of 'Napi::Error'
14+
what(): SQLITE_MISUSE: Backup is already finished
15+
Aborted (core dumped)
16+
```
17+
18+
**Affected code paths**:
19+
- `lib/promise/backup.js` line 97-99: The `if (!this._backup)` guard only checks for `null`, but after `finish()` the native handle is in a "finished" state that still exists but is invalid
20+
- The callback API (`lib/sqlite3-callback.js`) has the same issue — `backup.step()` after `backup.finish()` is uncatchable
21+
22+
**Workaround in promise wrapper**: The `SqliteBackup.step()` method checks `if (!this._backup)` before calling the native `step()`, but this only catches the case where `_backup` is explicitly set to `null`. After `finish()`, the `_backup` reference is still non-null but the native handle is invalid. Setting `_backup = null` in `finish()` would prevent the crash but would change the semantics of the `idle`/`completed`/`failed` getters after finish.
23+
24+
**Test impact**: Cannot write integration tests for `step()` after `finish()` — the process aborts before Mocha can report the failure. Unit tests with mocks are used instead (see `test/promise.unit.test.js`).
25+
26+
#### 2. Statement operations after `Database.close()`
27+
28+
Calling methods on a `Statement` after its parent `Database` has been closed throws uncatchable Napi::Error:
29+
```
30+
terminate called after throwing an instance of 'Napi::Error'
31+
what(): The expression evaluated to a falsy value: assert(err)
32+
Aborted (core dumped)
33+
```
34+
35+
**Affected operations**: `Statement.map()`, `Statement.all()`, `Statement.get()`, `Statement.run()`, `Statement.each()`, `Statement.finalize()`, `Statement.reset()`, `Statement.bind()`
36+
37+
**Workaround**: Always finalize all statements before closing the database. The callback API's `Database.close()` returns `SQLITE_BUSY` error if unfinalized statements exist, but if you force-close and then use a statement, the crash is uncatchable.
38+
39+
**Test impact**: Cannot test `Statement.prototype.map` error path by closing the database first. Unit tests with stubs are used instead (see `test/callback-branches.test.js`).
40+
41+
#### 3. `verbose.test.js` assertion failure after `verbose()` called
42+
43+
When `sqlite3.verbose()` has been called globally (setting `isVerbose = true`), the test "Should not add trace info to error when verbose is not called" in `verbose.test.js` will fail because the error stack DOES contain trace info. The `resetVerbose()` function restores original methods but does NOT reset the `isVerbose` flag. The assertion `err.stack.indexOf(invalid_sql) === -1` fails, and the Napi::Error from the assertion propagates as an uncatchable C++ exception.
44+
45+
**Workaround**: The `verbose()` idempotency test was placed as the LAST test in `verbose.test.js` to avoid contaminating the "not called" test. Test ordering matters for this file.
46+
47+
### Root cause
48+
49+
These are C++ exceptions thrown via `Napi::Error` that propagate through the N-API boundary. JavaScript `try/catch` only catches JavaScript exceptions. When a C++ exception reaches the N-API boundary without being caught by a `Napi::TryCatch` block on the C++ side, it triggers `std::terminate()` which calls `abort()`.
50+
51+
### Design implications
52+
53+
1. **Promise wrappers must guard against invalid states** before calling native methods, since native errors cannot be caught and converted to rejected promises
54+
2. **Test coverage for error paths** in native code must use mocks/stubs rather than trying to trigger actual native errors
55+
3. **Global state changes** (like `verbose()`) must be carefully managed in test suites to avoid cross-test contamination
56+
57+
---
58+
359
## async hook stack corruption
460

561
within CI workflow on macOS we got an async hook stack corruption as race condition in native addon

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@
7272
"rebuild": "node-gyp rebuild",
7373
"frozen-install": "yarn install --frozen-lockfile",
7474
"lint": "eslint lib/ test/ tools/",
75-
"test": "node test/support/createdb.js && nyc mocha -R spec --timeout 480000 \"test/*.test.js\" \"test/*.test.mjs\"",
75+
"test": "node test/support/createdb.js && nyc --reporter=text --reporter=lcov --reporter=html mocha -R spec --timeout 480000 \"test/*.test.js\" \"test/*.test.mjs\"",
7676
"bear": "bear -- node-gyp rebuild"
7777
},
7878
"license": "BSD-3-Clause",

test/cache.test.js

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,12 @@ describe('cache', function() {
1111
var filename = 'test/tmp/test_cache.db';
1212
helper.deleteFile(filename);
1313
var opened1 = false, opened2 = false;
14-
var db1 = new sqlite3.cached.Database(filename, function(err) {
14+
var db1 = new sqlite3.cached.Database(filename, function (err) {
1515
if (err) throw err;
1616
opened1 = true;
1717
if (opened1 && opened2) done();
1818
});
19-
var db2 = new sqlite3.cached.Database(filename, function(err) {
19+
var db2 = new sqlite3.cached.Database(filename, function (err) {
2020
if (err) throw err;
2121
opened2 = true;
2222
if (opened1 && opened2) done();
@@ -39,4 +39,37 @@ describe('cache', function() {
3939
});
4040
});
4141
});
42+
43+
it('should not cache memory database', function (done) {
44+
var filename = ':memory:';
45+
var opened1 = false, opened2 = false;
46+
var db1 = new sqlite3.cached.Database(filename, function (err) {
47+
if (err) throw err;
48+
opened1 = true;
49+
if (opened1 && opened2) done();
50+
});
51+
var db2 = new sqlite3.cached.Database(filename, function (err) {
52+
if (err) throw err;
53+
opened2 = true;
54+
if (opened1 && opened2) done();
55+
});
56+
assert.notEqual(db1, db2);
57+
});
58+
59+
60+
it('should not cache speacial database', function (done) {
61+
var filename = '';
62+
var opened1 = false, opened2 = false;
63+
var db1 = new sqlite3.cached.Database(filename, sqlite3.OPEN_READONLY, function (err) {
64+
if (err) throw err;
65+
opened1 = true;
66+
if (opened1 && opened2) done();
67+
});
68+
var db2 = new sqlite3.cached.Database(filename, sqlite3.OPEN_READONLY, function (err) {
69+
if (err) throw err;
70+
opened2 = true;
71+
if (opened1 && opened2) done();
72+
});
73+
assert.notEqual(db1, db2);
74+
});
4275
});

test/callback-branches.test.js

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
/**
2+
* Tests for uncovered branches in sqlite3-callback.js
3+
* Covers: cached.Database with mode parameter, Statement.map error/empty paths, verbose() called twice
4+
*/
5+
6+
'use strict';
7+
8+
var sqlite3 = require('..');
9+
var assert = require('assert');
10+
var helper = require('./support/helper');
11+
12+
describe('cached.Database branches', function() {
13+
before(function() {
14+
helper.ensureExists('test/tmp');
15+
});
16+
17+
it('should return cached database with mode parameter and callback', function(done) {
18+
var filename = 'test/tmp/test_cache_mode.db';
19+
helper.deleteFile(filename);
20+
21+
// First, open and cache the database
22+
var db1 = new sqlite3.cached.Database(filename, sqlite3.OPEN_READWRITE | sqlite3.OPEN_CREATE, function(err) {
23+
if (err) throw err;
24+
25+
// Now request the same cached database with mode + callback
26+
var db2 = new sqlite3.cached.Database(filename, sqlite3.OPEN_READWRITE, function(err) {
27+
if (err) throw err;
28+
assert.strictEqual(db1, db2);
29+
db1.close(function() {
30+
delete sqlite3.cached.objects[require('path').resolve(filename)];
31+
helper.deleteFile(filename);
32+
done();
33+
});
34+
});
35+
});
36+
});
37+
38+
it('should return cached database with mode parameter when db is already open', function(done) {
39+
var filename = 'test/tmp/test_cache_mode2.db';
40+
helper.deleteFile(filename);
41+
42+
var db1 = new sqlite3.cached.Database(filename, sqlite3.OPEN_READWRITE | sqlite3.OPEN_CREATE, function(err) {
43+
if (err) throw err;
44+
// db is already open now, request cached version with mode
45+
var db2 = new sqlite3.cached.Database(filename, sqlite3.OPEN_READONLY, function(err) {
46+
if (err) throw err;
47+
assert.strictEqual(db1, db2);
48+
db1.close(function() {
49+
delete sqlite3.cached.objects[require('path').resolve(filename)];
50+
helper.deleteFile(filename);
51+
done();
52+
});
53+
});
54+
});
55+
});
56+
57+
it('should return cached database with mode parameter and no callback', function(done) {
58+
var filename = 'test/tmp/test_cache_mode_nocb.db';
59+
helper.deleteFile(filename);
60+
61+
var db1 = new sqlite3.cached.Database(filename, sqlite3.OPEN_READWRITE | sqlite3.OPEN_CREATE, function(err) {
62+
if (err) throw err;
63+
// Request cached database with mode but NO callback
64+
// This covers the false branch of `if (typeof callback === 'function')` on line 46
65+
var db2 = new sqlite3.cached.Database(filename, sqlite3.OPEN_READWRITE);
66+
assert.strictEqual(db1, db2);
67+
db1.close(function() {
68+
delete sqlite3.cached.objects[require('path').resolve(filename)];
69+
helper.deleteFile(filename);
70+
done();
71+
});
72+
});
73+
});
74+
});
75+
76+
describe('Statement#map branches', function() {
77+
it('should handle error in map callback', function(done) {
78+
var db = new sqlite3.Database(':memory:');
79+
db.serialize(function() {
80+
db.run('CREATE TABLE foo (id INT, value TEXT)');
81+
db.map('SELECT * FROM nonexistent_table', function(err, map) {
82+
assert(err); // Should receive an error
83+
db.close(done);
84+
});
85+
});
86+
});
87+
88+
it('should handle empty result in map', function(done) {
89+
var db = new sqlite3.Database(':memory:');
90+
db.serialize(function() {
91+
db.run('CREATE TABLE foo (id INT, value TEXT)');
92+
db.map('SELECT * FROM foo', function(err, map) {
93+
if (err) throw err;
94+
assert.deepEqual(map, {});
95+
db.close(done);
96+
});
97+
});
98+
});
99+
100+
it('should handle error in Statement#map inner callback via stub', function(done) {
101+
// Test the error path in Statement.prototype.map's inner callback (line 124)
102+
// by stubbing the all() method to call back with an error
103+
var db = new sqlite3.Database(':memory:');
104+
db.serialize(function() {
105+
db.run('CREATE TABLE foo (id INT, value TEXT)');
106+
var stmt = db.prepare('SELECT * FROM foo');
107+
108+
// Save original all method
109+
var originalAll = stmt.all;
110+
111+
// Stub all() to call back with an error
112+
stmt.all = function() {
113+
var args = Array.prototype.slice.call(arguments);
114+
var callback = args[args.length - 1];
115+
callback(new Error('simulated all error'));
116+
return this;
117+
};
118+
119+
stmt.map(function(err, result) {
120+
assert(err);
121+
assert(err.message === 'simulated all error');
122+
123+
// Restore and clean up
124+
stmt.all = originalAll;
125+
stmt.finalize(function() {
126+
db.close(done);
127+
});
128+
});
129+
});
130+
});
131+
});

0 commit comments

Comments
 (0)