kwonoj / cld3-asm

WebAssembly based Javascript bindings for google Compact Language Detector v3
MIT License
56 stars 7 forks source link

Unhandled rejection shut down my service #140

Closed raphaelboukara closed 8 months ago

raphaelboukara commented 4 years ago

Hi, First of all great thanks for this amazing repos.

TL;DR

When using cld3-asm in a node service, on each unhandled rejection, my service will shut down without send any beforeExit or exit or SIGINT or SIGTERM event so I can handle those cases properly.

How to reproduce?

Open terminal:

mkdir cld3-asm-issue
cd cld3-asm-issue
nvm use 12
npm init --y
npm i express cld3-asm
touch index.js
// index.js
const cld3 = require('cld3-asm');
const app = require('express')();

process.on(
    'SIGTERM',
    () => process.stdout.write('SIGTERM\n')
);

process.on(
    'beforeExit',
    () => process.stdout.write('beforeExit\n')
);

process.on(
    'exit',
    (status) => process.stdout.write(`exit: ${status}\n`)
);

cld3.loadModule()
    .then(() => {
        app.listen(
            5432,
            () => console.log('listening...')
        );
    })
    .then(() => {
        setTimeout(() => Promise.reject(), 2000);
    })

Launch the service:

node index.js

Output:

listening...

/cld3-asm-issue/node_modules/cld3-asm/dist/cjs/lib/node/cld3.js:8
var Module=typeof Module!=="undefined"?Module:{};var moduleOverrides={};var key;for(key in Module){if(Module.hasOwnProperty(key)){moduleOverrides[key]=Module[key]}}Module["arguments"]=[];Module["thisProgram"]="./this.program";Module["quit"]=function(status,toThrow){throw toThrow};Module["preRun"]=[];Module["postRun"]=[];var ENVIRONMENT_IS_WEB=false;var ENVIRONMENT_IS_WORKER=false;var ENVIRONMENT_IS_NODE=true;if(Module["ENVIRONMENT"]){throw new Error("Module.ENVIRONMENT has been deprecated. To force the environment, use the ENVIRONMENT compile-time option (for example, -s ENVIRONMENT=web or -s ENVIRONMENT=node)")}var scriptDirectory="";function locateFile(path){if(Module["locateFile"]){return Module["locateFile"](path,scriptDirectory)}else{return scriptDirectory+path}}if(ENVIRONMENT_IS_NODE){if(!(typeof process==="object"&&typeof require==="function"))throw new Error("not compiled for this environment (did you b
abort() at Error
    at jsStackTrace (/cld3-asm-issue/node_modules/cld3-asm/dist/cjs/lib/node/cld3.js:8:11112)
    at stackTrace (/cld3-asm-issue/node_modules/cld3-asm/dist/cjs/lib/node/cld3.js:8:11283)
    at process.abort (/cld3-asm-issue/node_modules/cld3-asm/dist/cjs/lib/node/cld3.js:8:1058443)
    at process.emit (events.js:215:7)
    at processPromiseRejections (internal/process/promises.js:201:33)
    at processTicksAndRejections (internal/process/task_queues.js:94:32)

And my service died without telling anyone.

Is there a way to change this behavior? Maybe I missed some config. Is it mandatory for you to re-throw the error you catch via the unhandledRejection event?

kwonoj commented 4 years ago

I don't rethrow: it's runtime behavior in emscripten https://github.com/emscripten-core/emscripten/blob/b1a78891c216c98d90057a358731424ab609f22c/src/shell.js#L190 by default. This seems bit interesting since emscripten sets global handler to any rejection, I may try to see and change behavior but can't promise when / or if I'll do this. My main usecase was using browser env, which this didn't affect.

alonfixler commented 1 year ago

Hi @kwonoj I see that that catching the rejection is controlled via NODEJS_CATCH_REJECTION. Can it be changed to have a default of false when using this package or at least add a config that delegates the control over this to the consumers of cld3-asm?

kwonoj commented 1 year ago

Yes, I don't have strong opinions on this. Unfortunately I do not have enought time to check, if someone can come up with PR I can accept those.

alonfixler commented 1 year ago

@kwonoj Here's a PR: https://github.com/kwonoj/docker-cld3-wasm/pull/36