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

[Fetch] Throws Response Error on http failure #1

Open
jonchurch opened this issue May 20, 2023 · 2 comments
Open

[Fetch] Throws Response Error on http failure #1

jonchurch opened this issue May 20, 2023 · 2 comments

Comments

@jonchurch
Copy link
Owner

jonchurch commented May 20, 2023

Handling Errors in @spacejunk/airlock

This issue is for tracking a few concerns related to error handling in the @spacejunk/airlock package while using Fetch as the http client:

  1. Handling of error status codes: Currently, the typescript-fetch generator throws a ResponseError for status codes above 299, which is a departure from the pure Fetch behavior. This choice is not intentional but is imposed by the generator's source, which still needs to be located.
  2. Typed error objects: Ideally, we want the error objects to be typed. However, those types are not currently available in the OpenAPI SpaceTraders spec.
  3. Accessing the error response: Since the package uses Fetch, the error response can be accessed by calling response.json(). The generator's runtime handles response.json() for successful requests, so it's unexpected for users to have this magic removed for error responses.

Here's sample TypeScript code to illustrate the error handling:

import { Configuration, FleetApi, ResponseError } from '../dist';

export const config = new Configuration({
  accessToken: 'NO_TOKEN',
});

// Fails if there is NO_TOKEN, but also fails with a real token because the shipSymbol is nonsense
const badRequest = async () => {
  const fleet = new FleetApi(config);
  const res = await fleet.getShipCooldown('asdfasdfasa');
  if (res) {
    return res.data;
  }
  return undefined;
};

badRequest()
  .then((result) => console.log('Result from fulfilled promise:', result))
  .catch(async (err: ResponseError) => {
    const { response } = err;
    console.log('Full response', response);
    console.log('JSON body:', await response.json());
    console.log('status:', response.status);
  });
@jonchurch
Copy link
Owner Author

Next Steps and TODOs

  1. Locate the typescript-fetch generator's source: Investigate the generator's source code to understand the error handling mechanism and identify potential customizations to align with the pure Fetch behavior.

  2. Update the OpenAPI SpaceTraders spec: To include the necessary error types, extend the OpenAPI spec with relevant error definitions, providing better support for typed error objects.

  3. Improve error response handling: Modify the generator's runtime to handle error responses more intuitively. This may involve ensuring that response.json() is called for both successful and unsuccessful responses.

  4. Update documentation: Once the error handling mechanism is improved, update the package documentation to provide clear guidance on how to handle errors, with examples showcasing the updated behavior.

  5. Test the updated error handling behavior: Execute comprehensive tests to ensure that the improved error handling meets the desired behavior and does not introduce any unexpected side effects.

@jonchurch
Copy link
Owner Author

Part of this is thinking about if it's just worth switching to Axios. It's trivial to swap out the http client for the generator.

This project is a toy and an exercise in Not Invented Here, as is the whole spacetraders game for me. So using native Fetch in Node 18 and writing code to enrich it's experience without installing deps is fun. But, the openapi generator has already made some choices for us in terms of enriching fetch, and those were NIH. So let's not cling too dearly to the idea of writing a bunch of custom stuff unless it provides value.

Axios handles a lot of this stuff very well, but I'm trying to find a balance between having fun working with new tools (Node Native Fetch) and creating a useful library for myself and others.

All that said, the Axios client still can't type error returns, we need to fix that upstream at the Spacetraders OpenAPI Spec

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant