HiRoFa / quickjs_es_runtime

this is a wrapper library for the javascript runtime quickjs written in rust which works with typescript, modules, promises, async, await and much more
https://github.com/HiRoFa/quickjs_es_runtime
MIT License
104 stars 12 forks source link

Unexpected error logs even when the promise rejection is handled #74

Closed SreeniIO closed 1 year ago

SreeniIO commented 1 year ago
async function main() {
    throw new Error("test");
}
main().catch((e) => {});

The above code is logging the following two error logs even though the promise rejection is handled

quickjs_runtime::quickjs_utils::promises: 233: unhandled promise rejection detected 
quickjs_runtime::quickjs_utils::promises: 245: unhandled promise rejection - reason: Error: test
    at main (test.js:3)
    at <eval> (test.js:5)
andrieshiemstra commented 1 year ago

See https://github.com/bellard/quickjs/issues/112

I guess I'll need to fork quickjs and add fixes like the one proposed there...

andrieshiemstra commented 1 year ago

I'm doing some testing, and I'm not sure there is really an issue..

#[tokio::test]
    async fn test_rej_tracker(){

        simple_logging::log_to_stderr(LevelFilter::Info);

        let rt = QuickJsRuntimeBuilder::new().build();

        let script = Script::new("prom_rej.js", r#"
            new Promise((res, rej) => {
                console.log("running prom");
                setTimeout(() => {
                    rej(1);
                }, 1);
            }).catch((ex) => {
                console.log("prom catch %s", ex);
            });
        "#);

        let _ = rt.js_eval(None, script).await;

        let script2 = Script::new("prom_rej.js", r#"
            new Promise((res, rej) => {
                throw Error(2);
            }).catch((ex) => {
                console.log("prom catch 2 %s", ex);
            });
        "#);

        let _ = rt.js_eval(None, script2).await;

        sleep(Duration::from_secs(1));

    }

in your code example you invoke main() which is syntactic sugar for running something like

new Promise((resolve, reject) => {
    try {
        const res = (() => {
            throw Error("test");
        })();
        resolve(res);
    } catch(ex) {
         reject(ex);
    }
})

As per spec (see https://promisesaplus.com/ and/or https://stackoverflow.com/questions/42118900/when-is-the-body-of-a-promise-executed) the executor runs synchronously and thus rejects the Promise before you call .catch((e) => {})

so at the time of rejecting the promise with Error("test") there was no .catch called and thus the promise rejection was unhandled

when .catch is called, the Promise is allready fullfilled and the catch function is called with the rejected Exception.

unfortunately at the time of rejection there is no way to know you're going to call .catch later on except for wrapping the promise in something async and thus killing of the normal working of the executor being called sync..

e.g.:

#[tokio::test]
    async fn test_rej_tracker(){

        simple_logging::log_to_stderr(LevelFilter::Info);

        let rt = QuickJsRuntimeBuilder::new().build();

        let script = Script::new("prom_rej.js", r#"
            const executor = () => {
                console.log("running promise");
                throw Error(2);
            };
            new Promise((resolve, reject) => {
                setImmediate(() => {
                    try {
                        resolve(executor());
                    } catch(ex) {
                        reject(ex);
                    }
                });
            }).catch((ex) => {
                console.log("prom catch 2 %s", ex);
            });
        "#);

        let _ = rt.js_eval(None, script).await;

        sleep(Duration::from_secs(2));

    }
andrieshiemstra commented 1 year ago

you could wrap it pretty something like this:

function doReallyAsync(fn) {
    return new Promise((resolve, reject) => {
        setImmediate(() => {
                try {
                    resolve(fn());
                } catch(ex) {
                    reject(ex);
                }
        });
    });
}

async function main() {
    return await doReallyAsync(() => {
        console.log("promise executor running here");
        throw Error("test");
    });
}
main().catch((e) => {});
andrieshiemstra commented 1 year ago

figured out an easier way, you just need to go async by awaiting something before you cause the error..

e.g.: this does not log the unhandled promise exception because the await keyword on the first line in main causes the rest of the function to execute async

async function main() {
    await Promise.resolve(null);
    throw new Error("test");
}
main().catch((e) => {});
andrieshiemstra commented 1 year ago

oh, even works like this

async function main() {
    await;
    throw new Error("test");
}
main().catch((e) => {});
SreeniIO commented 1 year ago

Thanks. The following might be a better option as it would execute sync when there is no error and do an async wait only when there is an error.

async function main() {
    try {
        // user code here
        throw new Error("test");
    } catch (e) {
        await Promise.resolve();
        return e;
    }
}
main().catch((e) => {});
andrieshiemstra commented 1 year ago

yeah this works

async function main() {
                try {
                    // user code here
                    console.log("running");
                    throw Error("test");
                } catch (e) {
                    await 1;
                    throw e;
                }
            }

            main().catch((e) => {
                console.log("err " + e);
            });

Hope this helps you further.. closing this ticket as there is nothing more I can do