openapi-typescript icon indicating copy to clipboard operation
openapi-typescript copied to clipboard

Polymorphic status code responses: ideas wanted!

Open drwpow opened this issue 2 years ago • 6 comments

Say your endpoint returned entirely different shapes for 200, 201, 404, and 500 responses. The current version of this library would assign data to 200, and error to 500, while ignoring the 201 and 404 responses altogether.

While this is the pattern that most endpoints follow—usually an endpoint will return either 201 or 200 on success, but not both, which this library supports today—it still is a theoretical hiccup for valid schemas wanting to use this library.

Supporting this is unclear, though! How could this library provide support for multiple responses, without the use of generics?

Option A: Response object (breaking change)

const res = await get('/path/to/endpoint');
if (res[200]) { … }
else if (res[201]) { … }
else if (res[404]) { … }
else { … }

Note: TS would throw an error on invalid keys, e.g. res[0]. Worth noting this is typed as an object with numeric indices, and not just a standard array.

Option B: response discriminator (non-breaking change)

const { data, error, response } = await get('/path/to/endpoint');
if (response.status === 200) { … }      // `data` is defined, has 200 shape
else if (response.status === 201) { … } // `data` is defined, has 201 shape
else if (response.status === 404) { … } // `error` is defined, has 404 shape
else { … }                              // `error` is defined, has 500 shape

Option C: ???

If you have another proposal, please submit one below!

drwpow avatar Mar 16 '23 17:03 drwpow

I like option B. It’s not breaking (which in the current alpha phase of the package doesn’t really matter, but still…). Also, there’s probably more info contained within response than just the status code, so this would be a handy way to have the “meta” stuff separated from the actual data/error.

edit: I’ll use the library in a react-router/Remix project, so having more than just the status code available in an explicit manner would certainly be super helpful.

Just curious: Why do you not want to use generics?

HerrBertling avatar Apr 08 '23 09:04 HerrBertling

True—breaking changes don’t matter much in pre-1.0. But I think the API / usability is everything, and I do think sweating the details matter! I should have put more emphasis on “easy to use” more than “breaking changes.” I also would rather err on the side of “happy path is easy/terse; complex path is mildly annoying“ rather than “everything is equally mildly annoying.” 😄

Why do you not want to use generics?

The main README has a note on this, but that’s a path to errors. Imagine the lowest-common-denominator in a codebase: not yourself, but either someone that doesn’t know better, or someone who is trying to shortcut everything. Imagine having the wrong generic, or simply providing <any> everywhere. That’s effectively as good as not using TypeScript! And further, I believe generics are a last-resort when introspection fails. And within the realm of your OpenAPI schema, I think it’s more helpful to assume everything can be introspected. No errors, no shortcuts, and even better usability!

drwpow avatar Apr 11 '23 16:04 drwpow

So v0.1.0 does somewhat solve this in that it creates unions of “good” and “bad” types. Theoretically, now, if you have different schemas for different error codes (like different shapes entirely), then that will now be reflected when you check { error }. Likewise for { data } (though that is much less-common to have multiple different responses for the same method that have wildly-varying shapes).

I’m leaving this issue open for now, because I want to investigate this further. But I think for various reasons, we’ll be going forward with Option B mostly because that’s lighter-weight and easier to get type inference working for.

Currently the only thing missing in 0.1.0 from making Option B possible is the response.status discriminator—currently it doesn’t associate specific statuses with specific response codes yet (but is very possible to do in an upcoming release)

drwpow avatar May 01 '23 05:05 drwpow

Update on this: the response.status discriminator support still hasn’t been added, and this issue will be complete once that’s in.

drwpow avatar May 22 '23 16:05 drwpow

I am very interested in this, and just wanted to add my support for going forward with Option B!

In the meantime, we use something of the sort to get the right types based on the status code:

if (response.error) {
  let error;

  switch (response.response.status) {
    case 400:
      error = response.error as paths["/rtm/notes/create"]["post"]["responses"]["400"]["content"]["application/ld+json"];
      break;
    case 422:
      error = response.error as paths["/rtm/notes/create"]["post"]["responses"]["422"]["content"]["application/ld+json"];
      break;
    // etc...
  }

It's not nearly as neat, but it kind of "works". 🤷‍♂️

EmilePerron avatar Aug 08 '23 20:08 EmilePerron

I also support option B, I think checking status codes is a good practice anyway. I'm working with this api:

get_region: {
  responses: {
    200: {
      content: {
        "application/json": components["schemas"]["Region"];
      };
    }; 
    202: {
      content: {
        "application/json": components["schemas"]["Region"];
      };
    };
    204: {
      content: never;
    };
  };
};

~Currently, data isn't type safe for this API, typescript will let me access properties of Region even though in the 204 case, there is no response body.~ I misspoke here! It appears that now data is a union of all possible response shapes, which is at least type safe. However, depending on what the shapes in question are, it can be awkward to narrow the type.

RobinClowers avatar Apr 26 '24 21:04 RobinClowers

This issue is stale because it has been open for 90 days with no activity. If there is no activity in the next 7 days, the issue will be closed.

github-actions[bot] avatar Aug 06 '24 12:08 github-actions[bot]

This issue was closed because it has been inactive for 7 days since being marked as stale. Please open a new issue if you believe you are encountering a related problem.

github-actions[bot] avatar Aug 15 '24 02:08 github-actions[bot]