Hacker News new | ask | show | jobs
by forgotpwtomain 3667 days ago
I've complained a few times about what seems to me, to be a less-friendly way of handling Promise rejections in es6.

Consider the relatively common use-case - there is a service object which proxies requests, format's the call to the backend (or fetches from cache) and format's the returned response for the caller of the object. So the pattern looks something like: ServiceObject.fetchData(query).then((data) => { / * display data * / }, (err)=> { / * catch error - display no results * /})

At some-point you want to chain the promise to update another part of the ui: promise.then((data) => { /* display something else / }, (err) => { / catch error - display failed to load */ }).

The problem is you can't squash the error in the 'reject' of the previous promise now, because otherwise the error isn't propagated to the last promise in the link and instead you will hit the 'success' function. This 'reject' behavior is alright if there is something your 'success' function can do when the original request failed, but in a great majority of cases if the request failed there is nothing you can do - you put a 'reject' in the first chain of the promise resolution (potentially in the serviceObject itself) with some generic flash-message like 'request failed please try again' and call it good. As it stands you end up with a call chain where what a function higher up in the chain should return should be based on what a resolving function further down the chain is doing -- not having to do this was for me was almost entirely the plus-side of the promise-style over callbacks-style concurrency model.

I bring this up now because curiously the jQuery model of Deferred() precisely did not do this before -(see section#2 of Major Changes):

> Example: returns from rejection callbacks

if an error wasn't re-thrown in a catch, the promise-handling would stay in the 'reject' chain as long as an error had been thrown. I am quite curious as to why the current-model won, I understand some of the potential benefits but in practice I find that this behavior is worse in 90% of use-cases that I have encountered. If someone has a link to a mailing-thread / issue where this was discussed I would be quite interested.

2 comments

The way I handle situations like this is to only put the catches where I need them to serve some purpose. Generally, I don't catch errors at the start of the chain because I can't know what to do with them at that point. If I do catch them, it's only for logging or similar purposes and I still let the error propagate further.

One pattern I use is to rethrow the error:

return service.fetchData().then((data) => {}).catch((err) => {console.log(err); throw err;});

Another pattern I use is to split the promise chain so that I let my main results flow be the result that gets passed on, but I can do other things in a parallel manner internally:

let results = service.fetchData().then((data) => {}); results.catch((err) => {console.log(err);}); return results;

> return service.fetchData().then((data) => {}).catch((err) => {console.log(err); throw err;});

Yes - but this kind of code requires that you know that something else chains the promise and handles 'err' - which is my entire complaint, a higher level function shouldn't need to know whether it has children or if they do error handling. Otherwise it's back to the same kind of callback style where you have to go into another file and modify a top-level function to accommodate adding a child.

Edit:

I would rather you and the other commentators here re-read my parent post, as well as the relevant resources[0][1][2] - I seriously think I'm repeating the same thing for the 4th time here, no - I don't need an explanation of how promises work; I only was pointing out that the previous (2.0) dferred.promise model actually fits most better in most of use cases that I've experienced than the es6 one and I found that quite curious; but it seems impossible to have that discussion without being on the same page first.

[0] https://blog.jquery.com/2016/06/09/jquery-3-0-final-released...

[1] https://api.jquery.com/deferred.promise/

[2] https://developer.mozilla.org/en/docs/Web/JavaScript/Referen...

It doesn't need to care whether someone else is handling it. It should just do what it needs to do according to its contract. The caller that receives the promise as a result has its own contract that may or may not involve handling the error as well (and so on up the chain).

You generally have to have an end-of-the-chain catch as a safety precaution. If you don't have one and the promise fails you may get no feedback that an error happened at all. All methods that return promises should be able to expect the caller to handle them appropriately whether they pass or fail - it's not their responsibility to try and figure out how to handle an error in the context of the wider application.

«You generally have to have an end-of-the-chain catch as a safety precaution. If you don't have one and the promise fails you may get no feedback that an error happened at all.»

This will be increasingly less of a need as browser native promises catch up (and more reason to make sure that the promise library you are using is built on top of native promises rather than rolled from scratch). Several browser dev consoles will already show unhandled promise rejections today, and there's a growing convergence to also providing browser/system-wide event handlers for unhandled rejections as well.

http://www.2ality.com/2016/04/unhandled-rejections.html

You're returning a promise. At that point you've already assumed that something else is going to expect a promise, and a promise can either succeed or fail.
If you're not handling the error in the first promise, then throw it again.
> If you're not handling the error in the first promise, then throw it again.

You are handling the error (e.g. providing the user with a flash message telling them their was an error) - but if you don't re-throw it you will end up in the 'success' callback-chain - without a result obviously. Therefore whether you re-throw the error or squash it depends on whether another 'reject' function will be there to squash it further down the chain etc.

You should only ever be squashing an error into a success response if that's what someone would be expecting but that's rarely the case. Normally you should be allowing the error to bubble up and be decided by the caller.

I can think of a couple cases where catching an error down low with the purpose of squashing it makes sense. One would be for a service that you know may not have data and you don't care if it doesn't. For instance, trying to get geolocation data for a user in order to improve their experience but if it fails you don't need to show an error:

return geolocationService.fetch().catch((err) { /* log error /; return {}; / empty result/ });

Another case is when you might have multiple ways of handling the request where one is prioritized over the other. In this case, you can catch errors from the first attempt and try a second method. If they both error out, then the result promise will be an error as well.

return primary.fetch().catch((err) => { return secondary.fetch(); }).then((data) => { / manipulate data */});

> Therefore whether you re-throw the error or squash it depends on whether another 'reject' function will be there to squash it further down the chain etc.

Does it matter if another 'reject' function is there to squash it? I was under the impression that unhandled rejections would just vanish into the ether... no need to worry about another promise handling them. Just re-throw the error and forget about it.

Edit: To be clear, the browser may display an error in the console as a diagnostic tool, but my impression is that unhandled rejections will not result in an actual exception that halts execution.

Edit 2: Here's a fun example for the Chrome console:

    var p = Promise.reject();
    setTimeout(() => p.catch(e => {}), 1000);
It displays an error... and then a second later the error transforms into a non-error!
> Edit: To be clear, the browser may display an error in the console as a diagnostic tool, but my impression is that unhandled rejections will not result in an actual exception that halts execution.

Well if it's not handled there is simply no further promise-chain to call. If you maintain a large-enough app, you probably have some kind of tool for reporting client-side javascript errors. When you have unhandled promises it's not always clear that they are unhandled because you handled it and just happened to re-throw even though the promise has no further chain, or whether someone messed up and actually isn't handling a rejection. Thus to avoid this, adding new promise to a chain involves finding the first .catch(), adding a throw, and an extra .catch() further down the chain.