-2

Working in a error handling problem, where I need to catch error based on status code returned. In addition I need to get the body of the response to get the specific error message. I created a rough script, as follows:

let status = 200;
return fetch('/someurl',{ credentials: 'include' })
        .then(res => {
             if(!res.ok) {
                 status = res.status;
             }
             return res.json();
         })
        .then(json => {
            if(status < 200 || status >= 300) {
                notifyError(status, json);
            }
            treatResponse(json);
        })
        .catch(err => notifyError(err));

This code works as I expected, but it's not what I would expect in terms of code quality since I'm using a global variable to inform next then what happened in the previous then... very ugly.

I think that my code reflects the fact that I'm new to the fetch api and this promise stuff which I understood but I'm not familiar with.

So can somebody sugest something better?

Thanks

Patrick Roberts
  • 49,224
  • 10
  • 102
  • 153
Victor Dolirio
  • 143
  • 1
  • 3
  • 11
  • 1
    `res.json` is async function? Besides of this `notifyError` looks strange, it accepts `status` and `json` **or** error... Much better accept only error, but with possible `status` and `json` fields. – alexmac Sep 06 '17 at 18:09
  • @alexmac I agree with you about the strange signature of `notifyError()`, but to answer your question, yes, `res.json()` returns a promise. – Patrick Roberts Sep 06 '17 at 18:10

2 Answers2

0

You could convert it to this, by passing a promise to an array of promises using Promise.all():

return fetch('/someurl', { credentials: 'include' })
  .then(res => Promise.all([res.status, res.json()]))
  .then(([status, json]) => {
    if (status < 200 || status >= 300) {
      notifyError(status, json)
    }

    treatResponse(json)
  })
  .catch(err => notifyError(err))
Patrick Roberts
  • 49,224
  • 10
  • 102
  • 153
  • that's a neat trick – Sumi Straessle Sep 06 '17 at 17:56
  • I think you should add `return treatResponse(json)` because if it returns something, this result won't be used. – alexmac Sep 06 '17 at 18:16
  • @alexmac tell OP that, not me. I'm just duplicating the given code with the requested improvement. – Patrick Roberts Sep 06 '17 at 18:31
  • If you see a bug in OP code, you should at least mention about that, and maybe, suggest the possible solution how to fix it. – alexmac Sep 06 '17 at 18:43
  • @alexmac thing is, it's not a bug. Sometimes promises are only used to determine when something is complete, not necessarily to await a value. – Patrick Roberts Sep 06 '17 at 18:55
  • I don't talk that is a bug, I talk that it's a _possible bug_. The expression `if (a = 10)` is a bug? No, it isn't, but if developer doesn't know what it does, this can cause a lot of problems later. What about missing `return` statement, `bluebird`, for example, shows warning [in such case](http://bluebirdjs.com/docs/warning-explanations.html#warning-a-promise-was-created-in-a-handler-but-was-not-returned-from-it) – alexmac Sep 06 '17 at 19:05
-2

Simply throw error and catch that error on catch block-

return fetch('/someurl',{ credentials: 'include' })
        .then(res => {
             if(!res.ok) {
                 throw Error('something');
             }
             return res.json();
         })
        .then(json => {
            if(status < 200 || status >= 300) {
                throw Error('something othere');
            }
            treatResponse(json);
        })
        .catch(err => notifyError(err));

may be this help- read

Fazal Rasel
  • 4,446
  • 2
  • 20
  • 31