Requiring the module changes default bahavior of process.on('unhandledRejection')

[yoavain-sundaysky]

On NodeJS 14.x unhandled rejection are treated as warnings and do not crash the process.

When requiring the module, the code from compress_binding.js is changing this behavior so that any unhandled rejection terminates the process.

Here's a code snippet that reproduces the problem:

const sleep = (ms) => new Promise(resolve => setTimeout(resolve, ms));

const unHandledRejectionHere = async () => {
	Promise.reject("This is an unhandled exception");
}

const main = async () => {
	await unHandledRejectionHere();
	
	await sleep(1000);
	console.log("After 1st call");
	
	const wawoff2 = require("wawoff2");
	console.log("After require(\"wawoff2\")");
	
	await unHandledRejectionHere();
	
	await sleep(1000);
	console.log("After 2nd call");
}

main().catch(console.error);

Expected behavior:

(node:26404) UnhandledPromiseRejectionWarning: This is an unhandled exception
(Use `node --trace-warnings ...` to show where the warning was created)
(node:26404) 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:26404) [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.
After 1st call
After require("wawoff2")
(node:26404) UnhandledPromiseRejectionWarning: This is an unhandled exception
(node:26404) 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: 2)
After 2nd call

Actual behavior:

• Process crashes during 2nd call
  Log ends with:

RuntimeError: abort(This is an unhandled exception). Build with -s ASSERTIONS=1 for more info.
    at process.abort (...\node_modules\wawoff2\build\compress_binding.js:1:10773)
    at process.emit (events.js:412:35)
    at processPromiseRejections (internal/process/promises.js:245:33)
    at processTicksAndRejections (internal/process/task_queues.js:96:32)

[puzrin]

That part is auto-generated by emscripten. I see 3 potential possibilities:

• That was fixed in emscripten and just need rebuild with new version
• Wrong/missed build options
• Bug in emscripten, required to be reported in their repo

Do you have time to take a look and localize problem? We did build process as friction-less as possible (via docker). But i don't keep emscripten details in my head, and very busy at current moment.

[puzrin]

Quick googling for "emscripted exit on unhandled rejection" give such kind of links emscripten-core/emscripten#9028. But i did not dived into details.

[yoavain-sundaysky]

I googled and ran into this:
https://github.com/emscripten-core/emscripten/pull/9061/files

It was merged, so if I understand correctly, the default behavior was changed in a way that only if adding NODEJS_CATCH_REJECTION=1, the process['on']('unhandledRejection', abort); will be added.

So maybe it is only a matter of rebuilding with a new version

[puzrin]

Could you try dev branch and let me know result https://github.com/fontello/wawoff2/tree/dev?

[yoavain-sundaysky]

The artifacts still contain process["on"]("unhandledRejection" so it's still failing.

Maybe need to explicitly set flag to 0?
NODEJS_CATCH_REJECTION=0

[puzrin]

Could you experiment yourself with options? I can't allocate time now to investigate emscrpten doc and verify everything. Build scripts are quite simple and easy to modify.

[yoavain-sundaysky]

I managed to setup and run a build once, so I'll try to play with the options a bit

[yoavain-sundaysky]

This is the change in the Makefile that worked for me (at least for the above test code)

CARGS=--bind -s NODEJS_CATCH_REJECTION=0 -s ALLOW_MEMORY_GROWTH=1 -s SINGLE_FILE=1 -O3

[yoavain-sundaysky]

Let me know if you want me to open a PR

[puzrin]

For debug, temporary drop -s SINGLE_FILE=1 and add -g2, then you will see small perttified wrappers => easy to track changes.

Probably, if we modify this part, worth to use -s ENVIRONMENT=node and recheck other options https://github.com/emscripten-core/emscripten/blob/main/src/settings.js

[yoavain-sundaysky]

1. -s NODEJS_CATCH_REJECTION=0 removes the following:

process["on"]("unhandledRejection", function(reason) {
  throw reason;
 });

2. -s NODEJS_CATCH_EXIT=0 removes this block:

 process["on"]("uncaughtException", function(ex) {
  if (!(ex instanceof ExitStatus)) {
   throw ex;
  }
 });

3. -s ENVIRONMENT=node makes some changes that assumes node environment (as oppose to browser)
   for example, this:

var ENVIRONMENT_IS_WEB = typeof window === "object";
var ENVIRONMENT_IS_WORKER = typeof importScripts === "function";
var ENVIRONMENT_IS_NODE = typeof process === "object" && typeof process.versions === "object" && typeof process.versions.node === "string";

Turns into this:

var ENVIRONMENT_IS_WEB = false;
var ENVIRONMENT_IS_WORKER = false;
var ENVIRONMENT_IS_NODE = true;

which might limit the module to only work in NodeJS and not in Browsers

I think the first 2 are safe, while the 3rd should probably tested for backward-compatibility

[yoavain-sundaysky]

Opened a PR with the change

[yoavain-sundaysky]

Thank you!
Not everyday you get this effort from OSS owners.

Hope you have time to publish a new version as well.

[puzrin]

Published 2.0.1.

Thanks for your help!

[lirantal]

@puzrin and @yoavain-sundaysky you are both amazing for fixing this.
Many thanks ❤️

[soumen1102]

Indeed @yoavain-sundaysky thanks for your contribution to make software safer than before... 👍