Proper mindset for use of promises?

Question

I've only recently looked at promises (JS not being my forte) and I'm not sure what the proper way to do this is. Promises are supposed to prevent right-drifting code but when I end up with somewhat complex logic I end up nested far too deep anyway, so I'm convinced I'm doing it wrong.

If I'm returning both successes and failures as json values, and I want to handle malformed json as well, I immediately think to do something like this:

fetch('json').then(function (result) {
    return result.json();
}).catch(function (result) {
    console.error("Json parse failed!");
    console.error(result.text);
}).then(function (wat) {
    // if (!result.ok) { throw...
}).catch(function (wat) {
    // Catch http error codes and log the json.errormessage
});

Of course, this won't work. This is stereotypical synchronous code. But it's the first thing that comes to mind. Problems I can see:

How do I get both the response and the json output?
How do I get separate control flow for errors and successes?
How do I catch a json parse error on both types of response?

My best attempt involves nesting to the point where I might as well be using callbacks, and it doesn't work in the end because I still haven't solved any of the above problems:

fetch('json').then(function (response) {
    if (!response.ok) {
        throw response;
    }
}).then(
    function (response) {
        response.json().then(function (data) {
            console.log(data);
        });
    },
    function (response) {
        response.json().then(function (data) {
            console.error(data.errormessage);
        });
    }
).catch(function () {
    console.error("Json parse failed!");
    // Where's my response????
});

What's the "Right" way to do this? (Or at least less wrong)

Most of your "nesting" is just weird formatting. Why did you indent the function expressions when you're passing two of them to `then` but not when you pass only one? — Bergi, Sep 23 '17 at 10:48
Yes, complex logic usually involves nesting ([with](https://stackoverflow.com/a/26077868/1048572) [promises](https://stackoverflow.com/a/42015382/1048572) or without), and that's fine. Just make sure to [always `return` the inner promises](https://stackoverflow.com/a/25756564/1048572) to keep a singular result value. — Bergi, Sep 23 '17 at 10:52

alexmac · Accepted Answer · 2017-09-23T11:04:55.007

2

If you want to call response.json() anyway (for successful and failed response) and want to use the response together will the response data. Use Promise.all:

fetch('json')
  .then(response => Promise.all([response, response.json()]))
  .then(([response, data]) => {
    if (!response.ok) {
      console.error(data.errormessage);
    } else {
      console.log(data);
    }
  })
  .catch(err => {
    if (/* if http error */) {
      console.error('Http error');
    } else if (/* if json parse error */) 
      console.error('Json parse failed');
    } else {
      console.error('Unknown error: ' + err);
    }
  });

edited Sep 23 '17 at 11:04

answered Sep 23 '17 at 10:43

alexmac

19,087
7
58
69

That works. What happens if I want to do more fetches with different logic behind them in both sides of the if? Is there a way to chain it or do I just start nesting? – J V Sep 23 '17 at 10:50
If you need to pass the result of the previous call to the next `then`, use `Promise.all` again (with the previous result(s) and the new fetch call). If don't need the previous result, just call fetch. – alexmac Sep 23 '17 at 10:59
There's a subtle bug here: If fetch rejects the promise, this will incorrectly log "Json parse failed!". – glennsl Sep 23 '17 at 11:00
You need to check `err` object (`type` or `name`) in `catch` callback to identify the error and print proper message. – alexmac Sep 23 '17 at 11:01
@alexmac You should edit your code to fix that. As it is now, it's a bug. – glennsl Sep 23 '17 at 11:03

score 1 · Answer 2 · answered Sep 23 '17 at 10:28

1

You shouldn't use exceptions for control flow in Promises any more than you should when not using Promises. That's why fetch itself doesn't just reject the promise for status codes other than 200.

Here's one suggestion, but the answer will necessarily depend on your specific needs.

fetch('json').then(function (response) {
    if (!response.ok) {
        response.json().then(function (data) {
            console.error(data.errorMessage);
        });
        return ...;
    }

    return response.json().catch(function () {
        console.error("Json parse failed!");
        return ...;
    });
}).catch(function (e) {
    console.error(e);
    return ...;
});

answered Sep 23 '17 at 10:28

glennsl

28,186
12
57
75

Doesn't this just equate to callbacks? If I want to send another request based on the json contents I have to do so inside the `then` and the nesting gets worse no? – J V Sep 23 '17 at 10:43
Notice you'll have to `return` the `response.json().then(…)` promise, returning something else afterwards won't work. – Bergi Sep 23 '17 at 10:53
@JV If you start branching you'll get nesting no matter what. Promises will reduce nesting for sequential asynchronous calls, but can't do anything if your logic isn't sequential. – glennsl Sep 23 '17 at 10:55
@Bergi It works fine if you want to return something else, like a default. Returning the nested promise will not work, since that doesn't return anything either (though of course it could, if you needed it to). – glennsl Sep 23 '17 at 10:57
1

Yeah, usually you will want to use the error message as a result value, or as a rejection to be handled further down the chain. In your case, if you just meant to `console.log` it, that might be fine, but then you should also `.catch` errors from that sub-chain (thrown if `.json()` fails to parse the response). – Bergi Sep 23 '17 at 11:01

Proper mindset for use of promises?

2 Answers2