Comments (8)
At the moment, it does (or should?) at least notify you, when it passes the exception to Promise.onUnhandledException, which, by default, logs the error. Is it doing at least that?
Yes, that's working. It hits onUnhandledException
first, and then the next callback gets called with undefined
.
from promise.
That's a bug, thanks!
Thinking through it, how about making the promise forever-pending? I'm concerned about introducing rejections. However, I'm also concerned that using a forever-pending promise would introduce a memory leak (of these pending promises). A rejection might be nicer since, at least in Node, it would cause a message when inevitably not handled, and that message would "spiritually" correspond to there having been an unhandled exception.
The original plan was, after adding Result
helpers, to figure out how people use them, and, based on experience, add some kind of catching function that would map caught exceptions to Error(`Exn(e))
. You would then use that helper whenever your code can be reasonably expected to raise
. Not using it would be a programming error, so maybe it would be fine either reject or to return a forever-pending promise.
As I understood it, the Prometo library was heading toward Error(`Exn(e))
(cc @yawaramin).
I don't think the community ever really settled on a preference for this, and now improved rejecting promises are being proposed for ReScript (note the proposal changed during the thread).
There would still be a difference between rejecting in reason-promise in case of exceptions like in this issue, and what is being proposed in rescript-promise — rejecting only with exceptions is like Lwt.t('a)
, which is roughly Promise.rejectable('a, exn)
, whereas rescript-promise is rejectable with any type, whether exceptions or any other values.
from promise.
Hi, yeah, Prometo guarantees that it never rejects. If there's an exception when running the promise, it resolves the promise with Error(`Prometo_error(<Js.Promise.error value>))
. But it exposes the standard map
, flatMap
, etc. API for this wrapped type so you typically don't need to deal with the extra layer manually. That's the 'raison d'etre' of Prometo, basically.
EDIT: for the question of wrapping promise+result, this thread is apropos: https://forum.rescript-lang.org/t/promise-of-result-why-or-why-not/979
from promise.
Thinking through it, how about making the promise forever-pending?
I think it's it's important that it reject or resolve with an error in some way. It can be critical that you handle the failure of a promise. Ideally you could guarantee some sort of Error
result, but that of course requires the polymorphic error variant if you also want typed errors. The alternative, is to both handle the error and catch, but there are no guarantees you'll remember. :) I think bs-fetch
has some sort of exception conversion function as well?
We only recently started using `Exn(exn)
widely but so far like it. It seems like the only way to get everything we need, but comes with a few downsides as well.
from promise.
I think it's it's important that it reject or resolve with an error in some way. It can be critical that you handle the failure of a promise.
Definitely. At the moment, it does (or should?) at least notify you, when it passes the exception to Promise.onUnhandledException
, which, by default, logs the error. Is it doing at least that?
The issues are:
-
How should exception handling combine with promises?
The suggestion so far is to basically add a function
handle: ('a => 'b) => ('a => result('b, [> `Exn(exn)]))
so you can easily promote exceptions into results. I don't want to bake that default behavior in everywhere, because that would force every promise to get typed as potentially-throwing (-raising?), which forces handlers everywhere on users. I think there is some benefit to e.g. a user being able to handle errors early, and then, with the type system, being able to guarantee that the remaining promise is free of errors, or else to encode those errors explicitly in the type.We could make all promises potentially-raising, actually, and then add some kind of
noRaise
helper that will "decay" them back to promises typed as non-raising, by internally extractingError(`Exn(_))
and logging it, or otherwise handling it.I should also by now take a really close look at what Async does, since it combines non-failing promises with monitors for an exception-handling mechanism that I've always heard praised.
-
Where exceptions correspond to programming oversights and are not wrapped in
handle
or otherwise handled, it's not a problem to simply log them. The issue is what to do with chained promises afterwards. Definitely, their callbacks should not be executed.- If we reject the promises, that can still trigger a false
Promise.Js.catch
later on, for a user that is usingPromise.Js
to get rejectable promises. This seems especially dangerous if someone is using typed rejections (e.g.Promise.Js.t(int, string)
) and we suddenly inject a rejection withexn
— or any other type, really, since we can't know what type(s) the user is expecting in a rejection. - If we leave the promise forever-pending, I think we risk a small memory leak in some cases.
Dealing with this part is not something you'll have to remember to do — the unhandled exception has already been logged, and the purpose of this second mechanism is just to avoid executing the follow-on callbacks. The only thing to remember will be to handle the exceptions, which
onUnhandledException
should remind the user of. So hopefully not this... :PThe alternative, is to both handle the error and catch
- If we reject the promises, that can still trigger a false
from promise.
The above commit (f139ac3) makes reason-promise return forever-pending promises upon unhandled exceptions. I added tests, which failed, and the code fixes them.
Returning forever-pending promises can lead to memory leaks in some cases, in code that generates many of these doomed promises and retains references to them. This should be relatively rare, since most code that generates promises in a loop also tends to wait on its previous promises to resolve first, at worst in groups of N. In any case, even if it does occur, it will always be accompanied by a huge amount of spam of stack traces from onUnhandledException
. So it wouldn't be a silent memory leak, but a pretty loud notice of a likely programming error.
The other major option, to reject promises, seems like a bad idea to me, since it will introduce rejections where they are not expected, and undermine assumptions about control flow with reason-promise. It seems like it will eventually cause the same issue as here, except in some catch
on the JS side that should never have been triggered because of an earlier chain of reason-promises that should not have rejected.
I am still looking for good error-handling strategies on the higher levels of reason-promise.
I'll take care of a few other issues and release this in reason-promise 1.2.0.
from promise.
And, thanks again for the report! This was (and until the release, remains) a very serious bug.
from promise.
The release became reason-promise 1.1.3 and is now available in npm.
from promise.
Related Issues (20)
- BuckleScript 8.0.0 breaks listToArray / Promise.all HOT 5
- Reduce compiled size again HOT 4
- Format with refmt? HOT 2
- About the deprecation of the infix operators HOT 2
- Promise.allOkArray never resolves if input is an empty array HOT 1
- Tests: remainsPending appears not to be working
- Converting from Js.Promise HOT 3
- Convert tests to OCaml syntax
- Add a js_of_ocaml variant
- Try an internal module with double underscores to hide "rejectable" from error messages
- Provide type abbreviation for promises that reject with Js.Promise.error
- Convert README snippets to ReScript syntax
- Convert to ReScript syntax once there is enough adoption HOT 10
- tap on a rejected promise triggers unhandled promise rejection HOT 3
- Native Example HOT 4
- Example for reason syntax with letop
- Promise.pending/Promise.exec/Promise.resolved
- Align interfaces between native and js versions
- Melange compatibility HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from promise.