Ky: .json() method does not execute for failing API response

[jacob-fueled]

Perhaps this is intended, but I see nothing in the README to indicate that the .json() helper method _shouldn't_ execute.

ky
  .delete('/does-not-exist')
  .json()
  .then(() => console.log('deleted'))
  .catch(({ errors }) => console.error(errors))

With the above, assuming the response body, I expect to see [{ message: 'Cannot delete a nonexistent resource' }] in the console. Instead, I get an exception about destructuring (because the argument passed to catch is a Ky.HTTPError instance).

The workaround is

ky
  .delete('/does-not-exist')
  .json()
  .then(() => console.log('deleted'))
  .catch({ response }) => response.json())
  .then(console.error)

But that seems redundant with the .json() in the chain already.

[sholladay]

We can't just throw the parsed response body because in the case of an error, there may not even be a response to parse. Also, it would be weird if the error was a simple JSON object. It really has to be an Error instance to avoid surprising people and to make it easy to identify, e.g. error instanceof ky.TimeoutError.

That said, I would be okay with attaching a property to the error with the parsed object, if available. Something like error.payload, to avoid confusing it with the unparsed body on error.response.

For reference, I do something similar in my custom Ky instance in my apps:

export const apiFetch = ky.extend({
    credentials : 'same-origin',
    mode        : 'same-origin',
    prefixUrl   : '/api/',
    timeout     : 30000,
    hooks       : {
        afterResponse : [
            async (response) => {
                if (response.status === 401) {
                    location.assign('/login?next=' + encodeURIComponent(location.pathname));
                }
                else if (!response.ok) {
                    response.payload = await response.json();
                    return response;
                }
            }
        ]
    }
});

[jacob-fueled]

That makes sense to me! Would it be possible to do that conditionally based on whether req.json() was called?

In your custom Ky instance, won't the await response.json() throw/explode if there is no response body?

[sholladay]

In your custom Ky instance, won't the await response.json() throw/explode if there is no response body?

Yes, you are correct about that. However, I use hapi as my web server framework, which always sends back JSON for error responses (4xx and 5xx status codes), even ones that are caused by mistake. The check for !response.ok ensures that await response.json() is only run for error responses. And we know that we have a response there because afterResponse only runs when there is a response (i.e. it doesn't run for ky.TimeoutErrors).

Would it be possible to do that conditionally based on whether req.json() was called?

Yeah, we would only try to parse anything if you explicitly opted into it. We could potentially do something similar for .formData(), too, and maybe other body methods.

We would need to decide how to handle missing bodies as well as parsing errors here. Sadly, not everyone is using hapi, so what should happen if you use Ky with .json() but the server sends back an error with no body or a body that isn't valid JSON? Silently ignore it or throw an error? Unfortunately, I think throwing an error might be a painful breaking change for our existing users, since it affects which type of error your .catch() would receive, basically obscuring what the original error response was. For example, you might receive a parse error instead of a ky.HTTPError, which is decidedly less useful. On the other hand, I routinely get frustrated at systems that silently ignore errors, so that isn't great, either, but it's probably the lesser evil here.