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

Improve type safety on error responses #61

Closed
SferaDev opened this issue May 3, 2022 · 6 comments
Closed

Improve type safety on error responses #61

SferaDev opened this issue May 3, 2022 · 6 comments

Comments

@SferaDev
Copy link
Contributor

SferaDev commented May 3, 2022

Right now, fetchers do not have a way to include error responses types. Add them and make the default fetcher to type the union of them.

Also, create types for inlined responses.

@fabien0102
Copy link
Owner

Some points after a call with @SferaDev to have more context:

The idea is to add a TError that can extends all possible errors types in the fetcher
image

Then, since the type will be passed in every operations, this will force the user to implement every error cases (and typescript will warn on every new error introduce in openapi)

@fabien0102
Copy link
Owner

Thinking a bit more about this, some ideas that I would love to integrate to improve the error type safety.

1. Adding an ErrorWrapper in the fetcher.ts

export NamespaceErrorWrapper<TError> = TError

This will permit to easily extend and tweak the error type, as example

// Adding a fallback error type
export NamespaceErrorWrapper<TError> = TError | { type: "unknown"; message: string; }

2. Generate a more accurate error type on components.ts

export MyRouteError = NamespaceErrorWrapper<
 | { status: "404"; payload: Responses.NotFoundError } // $ref in openapi
 | { status: "422"; payload: { message: string; field: string; } } // inline in openapi
 | { status: `5${string}`; payload: Responses.BasicError }
>

and injecting this error to the fetcher (as describe previously) as generic.

So it's easier to discriminate on status to deal with the error.

3. Exporting an AllErrors union in components.ts

export type AllErrors = Responses.NotFoundError | Responses.BasicError | { message: string; field: string; }

So we could use this in as extends value in the fetcher.ts

export async function namespaceFetch<
  TData,
  TError extends NamespaceErrorWrapper<{ status: string; payload: AllErrors }>
  /* ... */
>(){}

@SferaDev
Copy link
Contributor Author

SferaDev commented May 5, 2022

@fabien0102 I like your proposals.

Just make status a number please (as in Response.status).

And do we really need things like 5${string}? I think you chose string to have template literal types, but I don't see them possible.

@fabien0102
Copy link
Owner

The thing is… in openapi, you can define 5xx (https://spec.openapis.org/oas/v3.0.3#patterned-fields-0)
image

And also default, and this is not making our life easy in term of generation 😓
I still need to play a bit with TS to see how I can type this properly, so far I'm really happy with the result…

@SferaDev
Copy link
Contributor Author

SferaDev commented May 5, 2022

The thing is… in openapi, you can define 5xx (https://spec.openapis.org/oas/v3.0.3#patterned-fields-0)

Then yes it might make sense to use strings.

Although maybe we could have a union without doing any parsing.

type Status = 200 | 201 | 400 | 404 | "5xx" | "default";

And the fetcher will have to discriminate or build the logic they want per union member.

Even though I like having the status code, I don't really see the real usage other than knowing that it's likely the error will happen so I need to make sure to treat the scenario. We already get the status code from the response error in runtime.

@fabien0102
Copy link
Owner

I have the pattern to generate the correct error type 😁

0ae6efe7-d641-4704-88f5-4775769888db

So we can have numbers and discriminate properly 🎉

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

2 participants