Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw error on error and data property be defined #1687

Open
1 task
Noahkoole opened this issue Jun 1, 2024 · 3 comments
Open
1 task

Throw error on error and data property be defined #1687

Noahkoole opened this issue Jun 1, 2024 · 3 comments
Labels
enhancement New feature or request openapi-fetch Relevant to the openapi-fetch library PRs welcome PRs are welcome to solve this issue!

Comments

@Noahkoole
Copy link

Description

Currently when a request fails it fills the error property and leaves the data property undefined. It would be a very nice feature if you could throw an error on error which in turn then makes the data property always defined. This implementation matches Axios. Currently I want to transition to openapi-fetch from axios but this change breaks a lot of code as I rely on try catch statements

Proposal

A option in the config of createClient where you can toggle if you want to throw an error rather then return the error value.

Checklist

@Noahkoole Noahkoole added enhancement New feature or request PRs welcome PRs are welcome to solve this issue! openapi-fetch Relevant to the openapi-fetch library labels Jun 1, 2024
@JeanRemiDelteil
Copy link
Contributor

This means a big change in the library usage.

In our case, I just have a guard to check if there is an error, typescript is able to infer that data is not undefined.
=> The current usage is easier to use with async/await I think.

  function listOrders() {
    const { data, error } = await api.POST('/api/pets_shop/order/list', {});

    if (error) {
      // Manage error.
      console.error('Error listing orders', error);

      return [];
    }

    // Use data
    return data.data.map(deserialize);
  }

Just as a quick fix you could have a helper function that does this for you, keeping the behavior from axios.
You can also adjust this to work with promise directly,

const dataOrThrow = <TData, TResponse, TError>(apiResponse: {
  response: TResponse;
  data?: TData;
  error?: TError;
}): {
  data: TData;
  response: TResponse;
} => {
  if (apiResponse.data === undefined) {
    throw apiResponse.error ?? new Error('No data');
  }

  return {
    data: apiResponse.data,
    response: apiResponse.response,
  };
};

// Usage
try {
   const { data: _d, response } = dataOrThrow(await api.POST('/api/pets_shop/order/list', {}));
} catch (error) {
   console.error(error);
}

@jlost
Copy link

jlost commented Jun 7, 2024

To me this seems like the more conventional behavior and it's odd that it isn't the default.

@drwpow
Copy link
Contributor

drwpow commented Jun 7, 2024

It’s intentionally the design that this library doesn’t throw an error to match the fetch spec. This is intended to be lower-level and flexible. The docs have a simple example on how to throw on every error with middleware.

Though I’m strongly against changing the default behavior, I wouldn’t mind this being an option on createClient as a nice quality of life improvement over having to create middleware just for this purpose (but people likely still would reach to middleware for more advanced logging etc)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request openapi-fetch Relevant to the openapi-fetch library PRs welcome PRs are welcome to solve this issue!
Projects
None yet
Development

No branches or pull requests

4 participants