0

I'm trying to improve the use of promises in some code that retrieves REST resources. I have a number of REST calls that make the perform the same sequence of actions:

  1. Fetch the config resource from the server if it hasn't been obtained previously
  2. Dispatch a flux action indicating the start of the request
  3. Send a request for the actual resource
  4. Parse the JSON in the response
  5. Dispatch a flux action indicating success with the parsed data.

The code I currently use to do this is below.

getThingsFromServer() {
    return getConfigIfNeeded().then(() => {
        dispatchStartOfRequestAction();
        return window.fetch(`${store.baseURL}/resource`)
                     .then((response) => {
                         if(response.ok) {
                             return response.json();
                         } else {
                             return Promise.reject(new Error(`${response.status} ${response.statusText}`));
                         }
                     }, (error) => {
                         return Promise.reject(new Error(`Network error: ${error.message}`));
                     })
                     .then((data) => {
                         dispatchSuccessAction(data);
                     }, (error) => {
                         return Promise.reject(new Error(`JSON parse error: ${error.message}`));
                     })
                     .catch((error) => {
                         dispatchFailureAction(error)
                     });
    });
}

There are a number of error conditions I would like to be able to handle individually, after which I want to dispatch a failure action (which is done in the catch()).

At the moment, if one of the individual then() error handlers gets called, every subsequent then() error handler is also called before finally calling the catch(). I want just one individual handler and the catch to be called.

I could dispense with handling each error individually and use a single catch at the end of it all but various sources both support and vilify the practice of handling all these different errors in the same way at the end of a promise chain. Is there a "right" answer to this beyond personal opinion?

Lukeus_Maximus
  • 443
  • 1
  • 3
  • 10
  • 1
    this question might be more suited to https://codereview.stackexchange.com, seems off topic for SO – Quince Aug 15 '17 at 14:48
  • Do you also want to do anything with errors from `getConfigIfNeeded`? Or does it never reject? – Bergi Aug 15 '17 at 15:38

1 Answers1

1

if one of the individual then() error handlers gets called, every subsequent then() error handler is also called

Yes, if you throw (or return a rejected promise) from then handler, the promise will get rejected and subsequent error handlers will get called. There's really no way around this. To differentiate the errors, you'll have to nest (see also here).

In your case, you'll want to use

dispatchStartOfRequestAction();
return fetch(`${store.baseURL}/resource`)
.then(response => {
    if (response.ok) {
        return response.json()
        .catch(error => {
            throw new Error(`JSON parse error: ${error.message}`);
        });
    } else {
        throw new Error(`${response.status} ${response.statusText}`);
    }
}, error => {
    throw new Error(`Network error: ${error.message}`);
})
.then(dispatchSuccessAction, dispatchFailureAction);
Bergi
  • 630,263
  • 148
  • 957
  • 1,375
  • I like the solution regarding the "max two levels" of promises you linked to, I think that's the closest I'm going to get to clear code. I am now also suddenly aware of my need for a dedicated error handler... – Lukeus_Maximus Aug 16 '17 at 10:19