ethanent / phin

Node HTTP client
MIT License
576 stars 33 forks source link

Don't parse JSON if response is a server error #61

Open jdforsythe opened 3 years ago

jdforsythe commented 3 years ago

Related to #48

When a server responds with a status code of 5xx, the body should not be parsed as JSON.

import * as p from 'phin';

const makeRequest() {
  const url = 'https://google.com';

  const opts: p.IJSONResponseOptions = {
    url,
    method: 'GET',
    parse: 'json',
    headers: {
      accept: 'application/json',
    },
  };

  const result = await p(opts);

  return result.body;

If the server returns a 500 Internal Server Error, the JSON parse still happens and the error is Unexpected token I in JSON at position 0. It appears the only way around this is to not have phin parse the JSON and to do it manually after checking the statusCode.

I think it makes more sense for phin to not parse the JSON unless the statusCode is a 2xx.

XuluWarrior commented 3 years ago

I recall another request library (axios?) throwing a (deliberate) exception on non-success statuses. I'm not saying that's the best way to deal with them but it is an option.

XuluWarrior commented 3 years ago

I created #75 as an alternative fix

ethanent commented 3 years ago

Comments on #75 as an alternative would be appreciated. You can see my comment on that PR. I think it's a practical solution.