Endpoints with no responses can only be '302', according to TypeScript

[MarcCelani-at]

Checklist

• I checked all open bugs and none of them matched my problem.

Environment

• https://api.github.com
• GitHub Enterprise Server:
• GitHub Enterprise Cloud
• Node.js
• Browsers:
• Others:

Versions
19.0.4

What happened?
Got an TS error

Minimal test case to reproduce the problem

const response = await octokit.orgs.checkMembershipForUser({
    org: organization,
    username,
});
// @ts-expect-error This looks like a bug in octokit.
if (response.status !== 204) {
    // Error handling
}

What did you expect to happen?
204 should be a valid response status

What the problem might be
Bad types?

[gr2m]

Just sharing my debugging as I've to run

I was able to reproduce the problem with a TypeScript playground

The octokit.orgs.checkMembershipForUser() method is sending a GET /orgs/{org}/members/{username} request as per source

(by the way, you should use octokit.rest.orgs.checkMembershipForUser() instead, for future compat)

The REST API docs state that there can be either a 204 or a 302 response

@octokit/rest 19.0.4 does use the latest version of @octokit/openapi-types (13.0.1) on fresh installs. @MarcCelani-at Can you confirm you have that version (npm ls @octokit/openapi-types)

@octokit/openapi-types do include a 302 response (playground)

When looking at @octokit/types, then Endpoints["GET /orgs/{org}/members/{username}"]["response"] returns a type for a single object with status: 302. So that's where I guess the problem lies (playground)

Here is a good starting point for further investigation: https://github.com/octokit/types.ts/blob/359fe757707bab4e9d1911108205154049c5385c/src/generated/Endpoints.ts#L61

[wolfy1339]

I can confirm that this is still an issue with the latest version of @octokit/types

I will be looking into this and #334

[wolfy1339]

After a brief look, for the GET /orgs/{org}/members/{username} endpoint,

type ExtractOctokitResponse<R> = "responses" extends keyof R // condition is true
  ? SuccessResponseDataType<R["responses"]> extends never // condition is true
    ? RedirectResponseDataType<R["responses"]> extends never // condition is false
      ? EmptyResponseDataType<R["responses"]>
      : RedirectResponseDataType<R["responses"]> // <== returned value from condition
    : SuccessResponseDataType<R["responses"]>
  : unknown;

It seems that the current implementation cannot handle cases where there is no body for many status codes like in this case.

The value returned by SuccessResponseDataType<R["responses"]> extends never while the RedirectResponseDataType<R["responses"]> and EmptyResponseDataType<R["responses"]> do not

So we need to somehow make a type that can go through all the status codes defined for an endpoint and return an OxtokitResponse with all the status codes when there is no body

[wolfy1339]

I made some modifications and managed to get the issue fixed, though I am unsure if it will have unintended consequences

diff --git a/src/generated/Endpoints.ts b/src/generated/Endpoints.ts
index c0d68f4..358e6eb 100644
--- a/src/generated/Endpoints.ts
+++ b/src/generated/Endpoints.ts
@@ -60,7 +60,7 @@ type MethodsMap = {
 };
 type SuccessStatuses = 200 | 201 | 202 | 204;
 type RedirectStatuses = 301 | 302;
-type EmptyResponseStatuses = 201 | 204;
+type EmptyResponseStatuses = 201 | 204 | 302;
 type KnownJsonResponseTypes =
   | "application/json"
   | "application/scim+json"
@@ -88,9 +88,9 @@ type DataType<T> = {
 }[KnownJsonResponseTypes & keyof T];
 type ExtractOctokitResponse<R> = "responses" extends keyof R
   ? SuccessResponseDataType<R["responses"]> extends never
-    ? RedirectResponseDataType<R["responses"]> extends never
-      ? EmptyResponseDataType<R["responses"]>
-      : RedirectResponseDataType<R["responses"]>
+    ? EmptyResponseDataType<R["responses"]> extends never
+      ? RedirectResponseDataType<R["responses"]>
+      : EmptyResponseDataType<R["responses"]>
     : SuccessResponseDataType<R["responses"]>
   : unknown;

The tests in the @octokit/types repository all pass. Applying the fix locally into node_modules and running the tests for @octokit/rest seems to work