Joji
Joji

Reputation: 5613

JavaScript: Is there a way to detect if a response contains error messages in json format when it is not ok

I am writing a simple wrapper around fetch.

async function apiCall(
  endpoint: string,
  {
    data,
    headers: customHeaders,
    ...customConfig
  }: { data?: Object; headers?: Object } = {}
) {
  const config = {
    method: data ? 'POST' : 'GET',
    body: data ? JSON.stringify(data) : undefined,
    headers: {
      'content-type': data ? 'application/json' : undefined,
      ...customHeaders,
    },
    ...customConfig,
  }

  return fetch(endpoint, config as any).then(async (response) => {
    if (response.ok) {
      const json = await response.json() // 🤔
      return json
    } else {
    // 👇 🚨 what if `response` contains error messages in json format?
      return Promise.reject(new Error('Unknown Error'))
    }
  })
}

it works fine. The problem is with this snippet

 return fetch(endpoint, config as any).then(async (response) => {
    if (response.ok) {
      const json = await response.json()
      return json
    } else {
    // 👇 🚨 what if `response` contains error messages in json format?
      return Promise.reject(new Error('Unknown Error'))
    }
  })

if response is not ok, it rejects with a generic Error. This is because by default, window.fetch will only reject a promise if the actual network request failed. But the issue is that, even if response is not ok, it might still be able to have error messages in json format. This depends on the backend implementation details but sometimes you are able to get the error messages in the response body by response.json(). Now this use case is not covered in the wrapper I built.

So I wonder how I am going to be able to account for that? I guess you can do something like

fetch(endpoint, config as any).then(async (response) => {
      if (response.ok) {
        const json = await response.json()
        return json
      } else {
          try {
            const json = await response.json()
            return Promise.reject(json)
          } catch {
            return Promise.reject(new Error('Unknown Error'))
          }
      }
    })

but I wonder if there is some more elegant way to do that?

Lastly, I am very aware of libraries like Axios. I built this partly to satisfy my intellectual curiosity.

Btw, a lightly unrelated question but I wonder if these two are equivalent

 if (response.ok) {
        const json = await response.json()
        return json
      }
 if (response.ok) {
        return response.json()
      }

Someone flagged my question as a duplicate of this question. In fact they are not the same. I did not make the same assumption as that question did about the API call returning JSON data both on success and on failure. My question is about exactly how we should do in cases where we cannot make such an assumption.

Upvotes: 1

Views: 3253

Answers (2)

Evert
Evert

Reputation: 99495

I'd simplify Kaiido even further, fix some bugs, and add a custom error handler.

class ApiCallError extends Error {

  response: Response;
  body: any;
  httpStatus: number;

  constructor(response, body) {

    super('HTTP error: ' + response.status);
    this.httpStatus = response.status;
    this.response = response;
    this.body = body;

  }  

}
async function apiCall(endpoint: string, options?: any) {

  const config = {
    // do your magic here
  }

  const response = await fetch(endpoint, config);

  if (!response.ok) {
    throw new ApiCallError(response, await response.json());
  }
  return response.json();
}

Changes:

  1. You don't need to catch errors if you're just going to throw again.
  2. You don't need .then() if you support await, and this will make your code a lot simpler.
  3. There's really no point in Promise.resolve and Promise.reject, you can just return or throw.
  4. You should not return plain errors, you should throw them (or make sure they are wrapped in a rejecting promise)
  5. Even though in javascript you can 'throw anything' including strings and arbitrary objects, it's better to throw something that is or extends Error, because this will give you a stack trace.
  6. Making all the error information available on the custom error class provides everything a user of apiCall could possibly need.

Upvotes: 2

Kaiido
Kaiido

Reputation: 136598

That the response is not ok doesn't prevent you from consuming its body as JSON so your last snippet is indeed how it should be handled.

Now you ask for something "more elegant", well it may not be more elegant but a less redundant way to write the same would be:

fetch(endpoint, config as any).then(async (response) => {
  if (response.ok) {
    return response.json(); // there is no need to await the return value
  }
  else {
    const err_message = await response.json() // either we have a message
      .catch( () => new Error( "Unknown Error" ) ); // or we make one
    return Promise.reject( err_message );
  }
})

And regarding the last question, yes both are equivalent, the await version doing one more round-trip through the microtask-queue and making your code a bit longer, but for all that matters we could say they are the same.

Upvotes: 5

Related Questions