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

Union type for error disappears in typescript-fetch v0.10 #1723

Open
1 task
zaru opened this issue Jun 24, 2024 · 9 comments
Open
1 task

Union type for error disappears in typescript-fetch v0.10 #1723

zaru opened this issue Jun 24, 2024 · 9 comments
Labels
openapi-fetch Relevant to the openapi-fetch library question Further information is requested

Comments

@zaru
Copy link

zaru commented Jun 24, 2024

Description

The union type for the error object returned by fetch has changed in typescript-fetch v0.10 and later versions.

Reproduction

I updated typescript-fetch to v0.10.1.

After the update, the type inference for the error object returned by fetch changed as follows:

Before v0.9.7

  • const error: { message: string } | Record<string, never> | undefined

v0.10.0 and Later

  • const error: { message: string } | undefined

OpenAPI YAML

I change the error response based on the type of error. For example, in the case of a 404 error, only a message is returned. For a 422 error, error messages are returned for each field.

# Partial YAML
      responses:
        '404':
          description: NotFound
          content:
            application/json:
              schema:
                type: object
                properties:
                  message:
                    type: string
                required:
                  - message
        '422':
          description: Bad parameters
          content:
            application/json:
              schema:
                title: ''
                type: object
                properties:
                  errors:
                    type: object
                    properties: {}
                required:
                  - errors

Expected result

It would be great if the error type could be the same as in previous versions:

const error: { message: string } | Record<string, never> | undefined

Checklist

@zaru zaru added bug Something isn't working openapi-fetch Relevant to the openapi-fetch library labels Jun 24, 2024
@drwpow drwpow added question Further information is requested and removed bug Something isn't working labels Jun 24, 2024
@drwpow
Copy link
Contributor

drwpow commented Jun 24, 2024

This was an intentional change in 0.10.0 that stemmed from some changes in how the error union was formed.

It’s now an approach for errors where it’s “take the first you find” because many schemas have a single defined error type, and some other responses being null or an impossible union. That seemed to be a more common pattern than different response codes returning a completely different error shape. i.e. many error responses are unintentional/accidental,

data was kept as a union because typically those are more intentional by comparison, and the union is something handled.

Unfortunately there’s not really a way to make both behaviors work—either keeping the error union results in many schemas being unusable because of irreconcilable unions. Or keeping this “take the first match” behavior results in some error types being left out. But this change felt safer because the worst case scenario is “represent one shape correctly” rather than “misrepresent all shapes incorrectly.”

Removed the bug label, but happy to gather feedback from people, or possible solutions that make both scenarios easier for people. At its root it’s just multiple conflicting approaches to schema design, neither being “right” or “wrong.”

@dusty
Copy link

dusty commented Jun 25, 2024

Would this change effect non-errors? I tried upgrading today and noticed a similar problem. We have an API endpoint that takes a PATCH.

On updates it returns an empty 202.

However, if a 'complete' property is sent as true, then it returns a 200 with a body of a particular shape.

I found that the data property of this response was set to never. Even if I were to check the response status to differentiate between 200 and 202, it was still never.

eg: this didn't work

if (response.status === 200) {
  console.log(response.data.something)  // ts error on something
} 

@markedwards
Copy link

markedwards commented Jul 2, 2024

This change seems to pretty badly break typing in my case. This is a typical flow:

  const client = createClient<paths>({ baseUrl: "..." });
  const { data, error } = await client.GET("/path");
  if (error) {
   // handle error
  }
  // handle data, which is now guaranteed to be the 200 case

As of v0.10, error above is now always undefined in the typing. However, that's not true, at runtime it still behaves as in v0.9.7. So the typing is apparently just wrong now.

Another note, in v0.10, if there is an error then response is typed as never, which is not right. At runtime, response is a normal object and I can use response.status in the error handling, for instance.

At this point, I cannot upgrade, frankly, because the typing is totally wonky. Am I doing it wrong?

@topaxi
Copy link

topaxi commented Jul 3, 2024

Came here for the same reason as @markedwards, error is now always undefined, especially as the first error being matched is always a 5XX, which usually do not have a useful body, while 4XX have like validation errors or similar structures.

There should be a way to have a proper union of returned responses for either, data and error.

Might be worth returning a status directly on the FetchResponse type to do so.

@markedwards
Copy link

Is it possible to get some confirmation that this issue indeed needs to be fixed, or if this is the intended behavior moving forward?

@christoph-fricke
Copy link
Contributor

@drwpow Do you have examples of impossible unions? Apart from "hello" | string collapsing to just string, I cannot think of real-world examples. It looks like the changes in v0.10 originated from unions with empty responses (#1609), which caused troubles.

I am super curious about this because I managed to create unions of all possible success and error responses bodies in OpenAPI-MSW without known edge cases (source code).

Further, I added a response helper, which allows constructing responses according the API-Specs, which feels much nicer than dealing with the union from a DX point of view.

When creating an extensive example with multiple error and success codes and content-types, I also noticed problems of loosing inference/discrimination on the discriminated data-error union in OpenAPI-Fetch, which others have already described as well. This leads to an unpleasant DX for some OpenAPI specs.

I think the best solution that solves everyones problems is some sort of response helper, which can avoid dealing with unions. Just like OpenAPI-MSW's helper but for handling responses instead of constructing them. My rough idea is something like pattern matching, where status code and content-type are mapped to response handlers.

const client = createClient<paths>();
const response = await client.GET("/some/path");
return handleResponse(response, {
  // provided data would be typed according to the 200 application/json body.
  // I can also imagine just providing a response and the consumer calls `res.json()`,
  // as long as it can be type-safe.
  "200/json": (data) => return data,
  "404/empty": () => throw new Error("Not found.")
  "400/text": (text) => throw new Error("Invalid: " + text)
})

I think the code example would not have proper type inference but it provides a rough idea of what I mean with a response helper for handling responses. It would also allow us to delay parsing the response body, so consumers that are not interested in the response result don't have to parse the body in the first place.

Are you interested in collaborating on this problem and finding a solution for it? @luchsamapparat and I are really interested in solving this.

@markedwards
Copy link

@drwpow any guidance here? openapi-fetch is totally useless to us as of 0.10. The typing is completely broken. Is there any interest in addressing this, or should I start investigating alternatives?

Thanks.

@drwpow
Copy link
Contributor

drwpow commented Jul 25, 2024

@christoph-fricke I love that approach, and I would gladly welcome solving it from that angle! Thanks for putting so much time and thought into it.

Current lift events have prevented me from spending time on this library myself past few weeks, but I’d gladly approve and ship PRs that tackle the solution in the manner outlined

@Twiggeh
Copy link

Twiggeh commented Jul 29, 2024

Hey 👋

I have a proposition type for keeping error types without them intersecting and overriding each other, similar to how @christoph-fricke wanted to have a helper function.

ErrorReturn needs to be fed paths, the path literal, and the method literal and it will spit out a descriminated union without swallowing any overlapping responses !

image

image

No swallow
image
image

If this is an approach that is alright for you I can make a PR 😄

If I missed something please let me know !

type ErrorReturn<
  KP extends keyof paths,
  M extends 'put' | 'get' | 'delete' | 'post',
  Response extends {
    put?: any;
    post?: any;
    delete?: any;
    get?: any;
  } = paths[KP],
  G extends Expand<{
    [Code in keyof Response[M]['responses']]: Response[M]['responses'][Code]['content'];
  }> = Expand<{
    [Code in keyof Response[M]['responses']]: Response[M]['responses'][Code]['content'];
  }>,
> = G;

type ErrorShape = Record<number | string, { [resType: string]: any }>;

type AddMissingKeys<T extends ErrorShape> = Prettify<
  { [K in Exclude<ErrorStatusTuple[number], keyof T>]: {} } & T
>;

// infer prevents "swallowing"
export type Expand<
  T extends ErrorShape,
  G = AddMissingKeys<T> extends {
    'default': infer XX;
    499: infer WW;
    498: infer VV;
    497: infer UU;
    451: infer TT;
    450: infer SS;
    444: infer RR;
    431: infer QQ;
    430: infer PP;
    429: infer OO;
    428: infer NN;
    427: infer MM;
    426: infer LL;
    425: infer KK;
    424: infer JJ;
    423: infer II;
    422: infer HH;
    421: infer GG;
    420: infer FF;
    418: infer EE;
    417: infer DD;
    416: infer CC;
    415: infer BB;
    414: infer AA;
    413: infer Z;
    412: infer Y;
    411: infer X;
    410: infer W;
    409: infer V;
    408: infer U;
    407: infer T;
    406: infer S;
    405: infer R;
    404: infer Q;
    403: infer p;
    402: infer O;
    401: infer N;
    400: infer M;
    511: infer L;
    510: infer K;
    508: infer J;
    507: infer H;
    506: infer G;
    505: infer F;
    504: infer E;
    503: infer D;
    502: infer C;
    '4xx': infer B;
    '5xx': infer A;
  }
    ?
        | {
            [P in keyof C]: Prettify<
              C[P] & { code: ErrorKey<Stringulator<P>, '502'> }
            >;
          }[keyof C]
        | {
            [P in keyof D]: Prettify<
              D[P] & { code: ErrorKey<Stringulator<P>, '503'> }
            >;
          }[keyof D]
        | {
            [P in keyof E]: Prettify<
              E[P] & { code: ErrorKey<Stringulator<P>, '504'> }
            >;
          }[keyof E]
        | {
            [P in keyof F]: Prettify<
              F[P] & { code: ErrorKey<Stringulator<P>, '505'> }
            >;
          }[keyof F]
        | {
            [P in keyof G]: Prettify<
              G[P] & { code: ErrorKey<Stringulator<P>, '506'> }
            >;
          }[keyof G]
        | {
            [P in keyof H]: Prettify<
              H[P] & { code: ErrorKey<Stringulator<P>, '507'> }
            >;
          }[keyof H]
        | {
            [P in keyof J]: Prettify<
              J[P] & { code: ErrorKey<Stringulator<P>, '508'> }
            >;
          }[keyof J]
        | {
            [P in keyof K]: Prettify<
              K[P] & { code: ErrorKey<Stringulator<P>, '510'> }
            >;
          }[keyof K]
        | {
            [P in keyof L]: Prettify<
              L[P] & { code: ErrorKey<Stringulator<P>, '511'> }
            >;
          }[keyof L]
        | {
            [P in keyof M]: Prettify<
              M[P] & { code: ErrorKey<Stringulator<P>, '400'> }
            >;
          }[keyof M]
        | {
            [P in keyof N]: Prettify<
              N[P] & { code: ErrorKey<Stringulator<P>, '401'> }
            >;
          }[keyof N]
        | {
            [P in keyof O]: Prettify<
              O[P] & { code: ErrorKey<Stringulator<P>, '402'> }
            >;
          }[keyof O]
        | {
            [P in keyof p]: Prettify<
              p[P] & { code: ErrorKey<Stringulator<P>, '403'> }
            >;
          }[keyof p]
        | {
            [P in keyof Q]: Prettify<
              Q[P] & { code: ErrorKey<Stringulator<P>, '404'> }
            >;
          }[keyof Q]
        | {
            [P in keyof R]: Prettify<
              R[P] & { code: ErrorKey<Stringulator<P>, '405'> }
            >;
          }[keyof R]
        | {
            [P in keyof S]: Prettify<
              S[P] & { code: ErrorKey<Stringulator<P>, '406'> }
            >;
          }[keyof S]
        | {
            [P in keyof T]: Prettify<
              T[P] & { code: ErrorKey<Stringulator<P>, '407'> }
            >;
          }[keyof T]
        | {
            [P in keyof U]: Prettify<
              U[P] & { code: ErrorKey<Stringulator<P>, '408'> }
            >;
          }[keyof U]
        | {
            [P in keyof V]: Prettify<
              V[P] & { code: ErrorKey<Stringulator<P>, '409'> }
            >;
          }[keyof V]
        | {
            [P in keyof W]: Prettify<
              W[P] & { code: ErrorKey<Stringulator<P>, '410'> }
            >;
          }[keyof W]
        | {
            [P in keyof X]: Prettify<
              X[P] & { code: ErrorKey<Stringulator<P>, '411'> }
            >;
          }[keyof X]
        | {
            [P in keyof Y]: Prettify<
              Y[P] & { code: ErrorKey<Stringulator<P>, '412'> }
            >;
          }[keyof Y]
        | {
            [P in keyof Z]: Prettify<
              Z[P] & { code: ErrorKey<Stringulator<P>, '413'> }
            >;
          }[keyof Z]
        | {
            [P in keyof AA]: Prettify<
              AA[P] & { code: ErrorKey<Stringulator<P>, '414'> }
            >;
          }[keyof AA]
        | {
            [P in keyof BB]: Prettify<
              BB[P] & { code: ErrorKey<Stringulator<P>, '415'> }
            >;
          }[keyof BB]
        | {
            [P in keyof CC]: Prettify<
              CC[P] & { code: ErrorKey<Stringulator<P>, '416'> }
            >;
          }[keyof CC]
        | {
            [P in keyof DD]: Prettify<
              DD[P] & { code: ErrorKey<Stringulator<P>, '417'> }
            >;
          }[keyof DD]
        | {
            [P in keyof EE]: Prettify<
              EE[P] & { code: ErrorKey<Stringulator<P>, '418'> }
            >;
          }[keyof EE]
        | {
            [P in keyof FF]: Prettify<
              FF[P] & { code: ErrorKey<Stringulator<P>, '420'> }
            >;
          }[keyof FF]
        | {
            [P in keyof GG]: Prettify<
              GG[P] & { code: ErrorKey<Stringulator<P>, '421'> }
            >;
          }[keyof GG]
        | {
            [P in keyof HH]: Prettify<
              HH[P] & { code: ErrorKey<Stringulator<P>, '422'> }
            >;
          }[keyof HH]
        | {
            [P in keyof II]: Prettify<
              II[P] & { code: ErrorKey<Stringulator<P>, '423'> }
            >;
          }[keyof II]
        | {
            [P in keyof JJ]: Prettify<
              JJ[P] & { code: ErrorKey<Stringulator<P>, '424'> }
            >;
          }[keyof JJ]
        | {
            [P in keyof KK]: Prettify<
              KK[P] & { code: ErrorKey<Stringulator<P>, '425'> }
            >;
          }[keyof KK]
        | {
            [P in keyof LL]: Prettify<
              LL[P] & { code: ErrorKey<Stringulator<P>, '426'> }
            >;
          }[keyof LL]
        | {
            [P in keyof MM]: Prettify<
              MM[P] & { code: ErrorKey<Stringulator<P>, '427'> }
            >;
          }[keyof MM]
        | {
            [P in keyof NN]: Prettify<
              NN[P] & { code: ErrorKey<Stringulator<P>, '428'> }
            >;
          }[keyof NN]
        | {
            [P in keyof OO]: Prettify<
              OO[P] & { code: ErrorKey<Stringulator<P>, '429'> }
            >;
          }[keyof OO]
        | {
            [P in keyof PP]: Prettify<
              PP[P] & { code: ErrorKey<Stringulator<P>, '430'> }
            >;
          }[keyof PP]
        | {
            [P in keyof QQ]: Prettify<
              QQ[P] & { code: ErrorKey<Stringulator<P>, '431'> }
            >;
          }[keyof QQ]
        | {
            [P in keyof RR]: Prettify<
              RR[P] & { code: ErrorKey<Stringulator<P>, '444'> }
            >;
          }[keyof RR]
        | {
            [P in keyof SS]: Prettify<
              SS[P] & { code: ErrorKey<Stringulator<P>, '450'> }
            >;
          }[keyof SS]
        | {
            [P in keyof TT]: Prettify<
              TT[P] & { code: ErrorKey<Stringulator<P>, '451'> }
            >;
          }[keyof TT]
        | {
            [P in keyof UU]: Prettify<
              UU[P] & { code: ErrorKey<Stringulator<P>, '497'> }
            >;
          }[keyof UU]
        | {
            [P in keyof VV]: Prettify<
              VV[P] & { code: ErrorKey<Stringulator<P>, '498'> }
            >;
          }[keyof VV]
        | {
            [P in keyof WW]: Prettify<
              WW[P] & { code: ErrorKey<Stringulator<P>, '499'> }
            >;
          }[keyof WW]
        | {
            [P in keyof B]: Prettify<
              B[P] & { code: ErrorKey<Stringulator<P>, '4xx'> }
            >;
          }[keyof B]
        | {
            [P in keyof A]: Prettify<
              A[P] & { code: ErrorKey<Stringulator<P>, '5xx'> }
            >;
          }[keyof A]
        | {
            [P in keyof XX]: Prettify<
              XX[P] & { code: ErrorKey<Stringulator<P>, 'default'> }
            >;
          }[keyof XX]
    :
      never,
> = G;

type Prettify<T> = {
  [K in keyof T]: T[K];
} & {};

type Stringulator<T extends string | number | symbol> = T extends number
  ? `${T}`
  : T extends string
    ? T
    : 'was a symbol';

type ErrorKey<
  ContentType extends string,
  RawCode extends string | number | symbol,
  Code extends string = Stringulator<RawCode>,
> = ContentType extends `${infer _}/${infer Type}` ? `${Code}/${Type}` : never;

type ErrorStatusTuple = [
  499,
  498,
  497,
  451,
  450,
  444,
  431,
  430,
  429,
  428,
  427,
  426,
  425,
  424,
  423,
  422,
  421,
  420,
  418,
  417,
  416,
  415,
  414,
  413,
  412,
  411,
  410,
  409,
  408,
  407,
  406,
  405,
  404,
  403,
  402,
  401,
  400,
  511,
  510,
  508,
  507,
  506,
  505,
  504,
  503,
  502,
  '4xx',
  '5xx',
  'default',
];

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
openapi-fetch Relevant to the openapi-fetch library question Further information is requested
Projects
None yet
Development

No branches or pull requests

7 participants