neon-bindings / neon

Rust bindings for writing safe and fast native Node.js modules.
https://www.neon-bindings.com/
Apache License 2.0
8k stars 283 forks source link

Avoid leaving a dangling future when handling a rejection in JsPromise::to_future #1008

Closed kjvalencik closed 9 months ago

kjvalencik commented 10 months ago

Previously, Neon was creating a distinct promise chain for handling resolved promises in JsPromise::to_future. This is problematic because it causes an unhandledRejection, even though the user may be handling the error.

In other words, it was the equivalent of this JavaScript:

const p = Promise.reject(new Error("oh, no!"));

p.then(() => console.log("Everything is good!"));
p.catch(() => console.log("Handled the error"));
Handled the error
/private/tmp/try-catch/foo.js:1
const p = Promise.reject(new Error("oh, no!"));
                         ^

Error: oh, no!

Instead, we use the two argument version of .then(onResolved, onRejected) which accepts both handlers at once, acting sort of like a match result. It is equivalent to the following JavaScript:

let settled = false;

Promise
    .reject(new Error("oh, no!"))
    .then(() => {
        if (!settled) {
            console.log("Everything is good!")
        }
    }, () => {
        settled = true;

        console.log("Handled the error")
    });
kjvalencik commented 9 months ago

.then can take a rejection handler as a second argument. This is a somewhat simpler option.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/then#onrejected

Ideally, we also wouldn't need to create new JsFunction wrapping Rust on each to_future, but the only alternative I can think of is .bind(..).