Why doesn't fetch reject if 400 ≤ status < 600?

[matthew-andrews]

It would be much more useful if it did, or if there were an option for it to.

[domenic]

There's a difference between an "error" and an "exception". Promise rejections parallel exceptions. But there's nothing exceptional about 4xx or 5xx errors on the web.

[matthew-andrews]

What definition of error, exception and exceptional are you using? As far as I knew the ECMA script spec refers to “Error Exceptions” implying a certain degree of interchangeability to me. I've never come across exception's adjective form (exceptional) in a computer science context. Not sure what you mean by that.

My understanding (please correct me if I am wrong) was that an Error became an exception when it is “thrown” — and so in this case whether a 4xx or 5xx is exceptional on the web is decided by how they are dealt with, not by, say, how commonly they occur…

Do you have a reference for “Promise rejects parallel exceptions”? If you “throw an Error” (e.g. below) that is caught by a Promise.

fetch('./my-lovely-json-endpoint')
  .then(function(response) {
    if (response.status >= 400 && response.status < 600) {
      throw new Error("Bad response from server");
    }
    return response.json();
  })
  .then(function(data) {
    console.log("got some data", data);
  });

Very happy to be persuaded otherwise — and I can see how ignoring the status keeps fetch clean and simple — but all the fetch use cases I can think of would require this sort of 400 ≤ status < 600 check. I can't imagine fetch ever being a feasible alternative to an AJAX wrapper if you have to write this level of boilerplate to make a request for some json…

[dgraham]

@matthew-andrews Here's the wrapper we're using on github.com as we migrate from jQuery to window.fetch. At least the required wrapper is much smaller than the full jQuery AJAX stack.

[annevk]

Yeah, we could have a status filter of sorts I suppose. Question is how generic that should be.

[matthew-andrews]

thinking about this more I really think that reject on 400+ statuses should have been the default behaviour, but it seems like that is a non starter. Offering it as an option, especially if that needs to be generic probably would be not a lot simpler than checking the status and throwing on >=400 manually.

@dgraham We've started using this to solve the problems we were having:-
https://github.com/matthew-andrews/fetchres

[annevk]

It's unclear to me why you think that makes sense. The body of a 400 or a 505 still carries semantic information that an application can use. And in fact <img> will happily display a 404 if that is an image.

[annevk]

Note that I did not say the option needs to be generic so I'm reopening this for now. We could have rejectHTTPErrors or some such.

[matthew-andrews]

superagent has quite an interesting alternative solution to this problem — it provides a res.ok that can be more easily checked against:-

 request
   .post('/api/pet')
   .send({ name: 'Manny', species: 'cat' })
   .set('X-API-Key', 'foobar')
   .set('Accept', 'application/json')
   .end(function(res){
     if (res.ok) {
       alert('yay got ' + JSON.stringify(res.body));
     } else {
       alert('Oh no! error ' + res.text);
     }
   });

Among other properties:-
https://github.com/visionmedia/superagent/blob/9e922cd34d70ead5ef4557c634a46153538134d5/lib/client.js#L385-409

[annevk]

That seems rather cool, although it does not have the nice property of rejecting. I could also imagine something like

fetch("...", {require:"2xx"})

[matthew-andrews]

i like both of those approaches. require is probably already overloaded as a term but i can't think of a better suggestion.

[matthew-andrews]

Been thinking about this a lot and I quite the TJ inspired solution:-

fetch('//mattandre.ws/path.json')
  .then(function(res) {
    if (res.ok) return res.json();
    return fallbackContent;
  });

I think it neatly addresses @domenic's concerns about client/server errors being unexceptional whilst adding just enough sugar to be convenient for developers to make handling these sorts of errors in a sensible way and it's backwards compatible*.

The nice property of rejecting is also limiting in a way because you might like instead of rejecting to do something else, say provide fallback content, instead.

What do you think?

* And can be easily polyfilled:-

(function() {
  var _fetch = fetch;
  fetch = function() {
    return _fetch.apply(this, arguments)
      .then(function(response) {
        response.ok = response.status < 400;
        return response;
      });
  };
}());

[annevk]

That polyfill seems incorrect. E.g. 399 should not match .ok, right? Or a 301 without a Location header?

[matthew-andrews]

Ah probably. I just wrote that more to give one idea of how to hook ok in. The fact that the actual status check needs to be more complex just further shows the need developers have for a little more help from the fetch spec.

[wanderview]

Gecko bug to implement this: https://bugzilla.mozilla.org/show_bug.cgi?id=1126483

[tobireif]

I also ran into the same issue - 404s didn't cause errors, thus I wasn't able to catch() the issue and handle it. JakeChampion/fetch#155

"there's nothing exceptional about 4xx [...] errors on the web."

Fetching the file failed - that's a failure, no matter whether it's called exception or error, IMHO.

I venture to guess that there will be many more reports of people being surprised that fetch doesn't throw when there's a 404. And what's worse, there probably will be many 404s going unhandled, because many will expect fetch to throw on 404.

fetch (at least in FF https://bug1126483.bugzilla.mozilla.org/attachment.cgi?id=8555602 ) reports "response.ok" for status "204 No Content". For my current app that's not what I want, so I'll simply throw an error on any non-200. (When the status is 200 the content could still be zilch, but if it's certain that there's no content, I shouldn't resolve the promise I'm returning.)

function fetchStatusHandler(response) {
  if (response.status === 200) {
    return response;
  } else {
    throw new Error(response.statusText);
  }
}

and (in my case inside a larger Promise block):

fetch(url)
  .then(
    fetchStatusHandler
  ).then(function(response) {
    return response.blob();
  }).then(function(blob) {
    resolve(blob.size);
  }).catch(function(error) {
    reject(error);
  });

For the spec:

In order to prevent the user (who might expect fetch to throw on 404) from accidentally ignoring 404s etc, perhaps raise an error if there's no status code handler supplied by the user. (eg a required arg {statusHandler: func} .)

I also like the solution proposed earlier by Anne, eg with a different name:

fetch("...", {throwUnlessStatusIs: "20x"})

It should be a required arg so that the user knowingly accepts or rejects eg 404s.

[vaibsharma]

I prefer to do like this

return new Promise((resolve, reject) => {
        fetch(url, {
            method: POST,
            headers,
            body: JSON.stringify(body),
        })
            .then((res) => {
                const response = res;
                const responsePromise = res.json();
                responsePromise
                    .then((data) => {
                        if (response.status >= 400) {
                            reject(data);
                        } else {
                            resolve(data);
                        }
                    })
                    .catch((error) => {
                        reject(error);
                    });
            });
    });

[jakearchibald]

FWIW that can be rewritten as:

return fetch(url, {
  method: 'POST',
  headers,
  body: JSON.stringify(body),
}).then((response) =>
  response.json().then((data) => {
    if (!response.ok) {
      throw Error(data.err || 'HTTP error');
    }
    return data;
  }),
);

Also, you should only reject error objects (as I've done in the example above).

[annevk]

s/responsePromise.status >= 400/!response.ok/

[jakearchibald]

Ta, edited

[vaibsharma]

Thanks @jakearchibald

[hugo-dlb]

A good workaround is to do a wrapper around fetch like an HttpClient class:

export default class HttpClient {

    static post(url: string, body: any): Promise<any> {
        return new Promise(async (resolve, reject) => {
            try {
                let result = await fetch(`${process.env.REACT_APP_API_BASE_URL}/${url}`, {
                    method: 'POST',
                    headers: {
                        'Content-Type': 'application/json'
                    },
                    body: JSON.stringify(body)
                });
                if (result.status >= 400) {
                    reject(await result.json());
                } else {
                    resolve(await result.json());
                }
            } catch (err) {
                reject(err);
            }
        });
    }
}

Then it can be used like this...

HttpClient.post('/auth/login', {
        email,
        password
    }).then(res => {
        // Success
    }).catch(err => {
        // Error
    });

[marcelo-ribeiro]

// Typescript
export const fetcher = async <T = any>(
  input: RequestInfo,
  init?: RequestInit
): Promise<T> => {
  const response = await fetch(input, init);
  if (!response.ok) throw new Error(response.statusText);
  return response.json();
};

// Javascript
export const fetcher = async(input, init = {}) => {
  const response = await fetch(input, init);
  if (!response.ok) throw new Error(response.statusText);
  return response.json();
};

Usage example:

type UserPost = {
  userId: number;
  id: number;
  title: string;
  body: string;
};

(async () => {
  try {
    const data = await fetcher<UserPost>(
      "https://jsonplaceholder.typicode.com/posts/1"
    );
    const { userId, id, title, body } = data;
    console.log({ userId, id, title, body });
  } catch (error) {
    console.log(error);
  }
})();