-
Notifications
You must be signed in to change notification settings - Fork 89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fromPromise
incompatible with node v15
#268
Comments
Probably the new API will need to be something like the following: const resultAsync = ResultAsync.fromPromise(
functionThatReturnsPromiseThatMightThrow,
arguments,
mapPromiseRejection,
) Which allows fromPromise(fn, args, mapper) => {
const handled = fn(args).catch(mapper)
} Edit: Another approach might be to enforce that the function argument is a "wrapper" / thunk: fromPromise(
() => functionThatReturnsPromiseThatMightThrow(arguments),
mapPromiseRejection
) |
Curious to get your thoughts @paduc 🙂 |
Wow I didn't know such a huge change was on the horizon. I've read a bit about the new behaviour but it seems to me that you can still handle rejected promises higher in the stack, can't you ? Have you tried it in Node 15 ? |
Yeah I think @paduc is right. The promise in that example would still be considered handled because This example shows it: const {ResultAsync} = require('.');
async function f () {
return Promise.reject(new Error('test'));
}
function main () {
return ResultAsync.fromPromise(f(), ()=>0);
}
main().then(console.log);
setTimeout(()=>{
console.log('done');
}, 3000); This doesn't crash the process on node v15. If you do not handle it, say, you replace |
Hmm, interesting. I hadn't gotten around to trying this out myself, but I figured that the act of not binding Thanks for confirming @davazp! |
It looks like as of node 15, unhandled promise rejections will terminate the Node process by default.
Associated commit to Node: nodejs/node#33021
What does this mean for
neverthrow
?It means that the current
fromPromise
API will need to change:The text was updated successfully, but these errors were encountered: