Loading sql.js in Node makes it terminate the process when unrelated promise rejections are unhandled

[garybernhardt]

To reproduce: Run this script. It initializes sql.js, waits one second, then creates a rejected promise. The promise is outside of any code related to sql.js. The only relationship is that it's running in the same process.

const initSqlJs = require("sql.js")

initSqlJs().then(() => {
})

// Wait long enough that sql.js is initialized. But this doesn't directly
// `then` on the initSqlJs promise. That makes it clear that rejected promises
// totally outside of the initSqlJs promise still cause aborts.
setTimeout(() => {
  Promise.reject("failure")
  setTimeout(() => { console.log("process is still alive") }, 1000)
}, 1000)

Expected result: A warning about the rejected promise should be printed by Node, followed by the "process is still alive" message. This is what happens if you comment out the sql.js-related lines:

$ node test.js
(node:44462) UnhandledPromiseRejectionWarning: failure
(node:44462) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:44462) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
process is still alive

Actual result: sql.js sees the unhandled rejection in a promise that should be outside of sql.js' control. It aborts the entire Node process:

$ node test.js
failure
failure
/Users/grb/proj/ep/node_modules/sql.js/dist/sql-wasm.js:107
function D(a){if(e.onAbort)e.onAbort(a);Ha(a);E(a);Va=!0;throw new WebAssembly.RuntimeError("abort("+a+"). Build with -s ASSERTIONS=1 for more info.");}function pb(a){var b=qb;return String.prototype.startsWith?b.startsWith(a):0===b.indexOf(a)}function rb(){return pb("data:application/octet-stream;base64,")}var qb="sql-wasm.wasm";if(!rb()){var sb=qb;qb=e.locateFile?e.locateFile(sb,z):z+sb}
                                                         ^

RuntimeError: abort(failure). Build with -s ASSERTIONS=1 for more info.
    at process.D (/Users/grb/proj/ep/node_modules/sql.js/dist/sql-wasm.js:107:64)
    at process.emit (events.js:315:20)
    at processPromiseRejections (internal/process/promises.js:209:33)
    at processTicksAndRejections (internal/process/task_queues.js:98:32)

Discussion: Our application runs sql.js both in the browser and in Node. In the browser, we use it for interactive lessons on SQL. In Node, we use it for automated testing of the lessons.

Some of our code examples in other courses (not the SQL course) intentionally "leak" rejected promises. That test process also loads sql.js during boot, even though it's not in use at the time when rejections are leaked. Merely loading sql.js causes it to intercept the unhandled rejection and abort the Node process.

(The situation seems to be slightly weirder than that in practice. Sometimes unhandled rejections go by without any problems. I haven't been able to detect a pattern. But fortunately the repro code above seems perfectly reliable.)

Versions: We're currently running 1.1.0, but I also upgraded to 1.3.0 (current) and it happened there as well.

[lovasoa]

This looks like a problem with emscripten. Is this specific to sql.js ?

[garybernhardt]

I lack the expertise to say but I'll write a tweet and I bet someone will know.

[garybernhardt]

Looks like it's not specific to sql.js, but it can be configured with a build option. An emscripten issue pointed me to NODEJS_CATCH_REJECTION. From the linked line, it seems clear that building with -s NODEJS_CATCH_REJECTION=0 will stop this behavior. Would you be open to building sql.js with that option?

[lovasoa]

Stated like that, yes, of course. Is there a reason why NODEJS_CATCH_REJECTION=1 is the default?

[gabrielmaldi]

Is there a reason why NODEJS_CATCH_REJECTION=1 is the default?

emscripten-core/emscripten#9061

We can't disable it by default because due to the current node behavior, some errors would not be reported (the node return code would be 0).

// Catch unhandled rejections in node. Without this, node may print the error,
// and that this behavior will change in future node, wait a few seconds, and
// then exit with 0 (which hides the error if you don't read the log). With
// this, we catch any unhandled rejection and throw an actual error, which will
// make the process exit immediately with a non-0 return code.
var NODEJS_CATCH_REJECTION = 1;

[garybernhardt]

OK, so it seems like it's papering over an issue in emscripten's (or some emscripten-compiled projects') test suites or other command line tools. For a library like sql.js, I think it makes sense to turn this off to avoid changing the entire process' unhandled rejection behavior. Node exiting with 0 status on rejection is a normal (if sometimes inconvenient) part of Node:

$ node -e 'Promise.reject()'
(node:54594) UnhandledPromiseRejectionWarning: undefined
(node:54594) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:54594) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.
macbot:ep(master) $ echo $?
0

but we can always change that behavior with Node arguments when needed:

$ node --unhandled-rejections=strict -e 'Promise.reject()'
internal/process/promises.js:194
        triggerUncaughtException(err, true /* fromPromise */);
        ^

[UnhandledPromiseRejection: This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason "undefined".] {
  code: 'ERR_UNHANDLED_REJECTION'
}
macbot:ep(master) $ echo $?
1

[lovasoa]

Ok! I agree. Let's change our makefile to include NODEJS_CATCH_REJECTION=0. @garybernhardt, do you want to make the pull request ?

[garybernhardt]

Done! #423