fix: do not prevent POST /app-manifests/{code}/conversions requests (#30)

BREAKING CHANGE: Mutating requests (`DELETE`, `PATCH`, `POST`, `PUT`) or no longer blocked before sending the request. The response status is handled instead and the error message is amended where applicable

Author: gr2m

package-lock.json

@@ -33,9 +33,8 @@
     "@pika/plugin-build-web": "^0.9.0",
     "@pika/plugin-ts-standard-pkg": "^0.9.0",
     "@tsconfig/node10": "^1.0.3",
-    "@types/fetch-mock": "^7.3.1",
     "@types/jest": "^26.0.0",
-    "fetch-mock": "^9.0.0",
+    "fetch-mock": "^9.10.7",
     "jest": "^25.1.0",
     "semantic-release": "^17.0.0",
     "ts-jest": "^25.1.0",

package.json

@@ -11,8 +11,6 @@ import {
 import { isRateLimitError } from "./is-rate-limit-error";
 import { isAbuseLimitError } from "./is-abuse-limit-error";
 
-const MUTATING_METHODS = ["DELETE", "PATCH", "POST", "PUT"];
-
 export async function hook(
   reason: string,
   request: RequestInterface,
@@ -24,27 +22,32 @@ export async function hook(
     parameters
   );
 
-  if (MUTATING_METHODS.includes(endpoint.method)) {
-    throw new RequestError(
-      `"${endpoint.method} ${endpoint.url}" is not permitted due to lack of authentication. Reason: ${reason}`,
-      403,
-      {
-        request: request.endpoint.parse(endpoint),
-      }
-    );
-  }
-
   return request(endpoint as EndpointOptions).catch((error) => {
     if (error.status === 404) {
       error.message = `Not found. May be due to lack of authentication. Reason: ${reason}`;
+      throw error;
     }
 
     if (isRateLimitError(error)) {
       error.message = `API rate limit exceeded. This maybe caused by the lack of authentication. Reason: ${reason}`;
+      throw error;
     }
 
     if (isAbuseLimitError(error)) {
       error.message = `You have triggered an abuse detection mechanism. This maybe caused by the lack of authentication. Reason: ${reason}`;
+      throw error;
+    }
+
+    if (error.status === 401) {
+      error.message = `Unauthorized. "${endpoint.method} ${endpoint.url}" failed most likely due to lack of authentication. Reason: ${reason}`;
+      throw error;
+    }
+
+    if (error.status >= 400 && error.status < 500) {
+      error.message = error.message.replace(
+        /\.?$/,
+        `. May be caused by lack of authentication (${reason}).`
+      );
     }
 
     throw error;

src/hook.ts

@@ -175,13 +175,15 @@ test('auth.hook(request, "GET /repos/octocat/hello-world") returns rate limit re
   }
 });
 
-test('auth.hook(request, "PATCH /repos/octocat/hello-world") fails without sending request', async () => {
+test('auth.hook(request, "PATCH /repos/octocat/hello-world") with 401 response', async () => {
   const requestMock = request.defaults({
     headers: {
       "user-agent": "test",
     },
     request: {
-      fetch: fetchMock.sandbox(),
+      fetch: fetchMock
+        .sandbox()
+        .patchOnce("path:/repos/octocat/hello-world", 401),
     },
   });
 
@@ -192,7 +194,7 @@ test('auth.hook(request, "PATCH /repos/octocat/hello-world") fails without sendi
     throw new Error("should not resolve");
   } catch (error) {
     expect(error.message).toBe(
-      '"PATCH /repos/octocat/hello-world" is not permitted due to lack of authentication. Reason: test'
+      'Unauthorized. "PATCH /repos/octocat/hello-world" failed most likely due to lack of authentication. Reason: test'
     );
   }
 });
@@ -218,3 +220,62 @@ test('auth.hook(request, "GET /repos/octocat/hello-world") does not swallow non-
     expect(error.message).toBe("unrelated");
   }
 });
+
+test('auth.hook(request, "POST /repos/octocat/hello-world/issues/123/comments") with 403 response', async () => {
+  const requestMock = request.defaults({
+    headers: {
+      "user-agent": "test",
+    },
+    request: {
+      fetch: fetchMock
+        .sandbox()
+        .postOnce("path:/repos/octocat/hello-world/issues/123/comments", {
+          status: 403,
+          body: {
+            message: "You cannot comment on locked issues",
+          },
+        }),
+    },
+  });
+
+  const { hook } = createUnauthenticatedAuth({ reason: "test" });
+
+  try {
+    await hook(
+      requestMock,
+      "POST /repos/octocat/hello-world/issues/123/comments"
+    );
+
+    throw new Error("should not resolve");
+  } catch (error) {
+    expect(error.message).toBe(
+      "You cannot comment on locked issues. May be caused by lack of authentication (test)."
+    );
+  }
+});
+
+test('500 response', async () => {
+  const requestMock = request.defaults({
+    headers: {
+      "user-agent": "test",
+    },
+    request: {
+      fetch: fetchMock
+        .sandbox()
+        .getOnce("path:/", 500),
+    },
+  });
+
+  const { hook } = createUnauthenticatedAuth({ reason: "test" });
+
+  try {
+    await hook(
+      requestMock,
+      "GET /"
+    );
+
+    throw new Error("should not resolve");
+  } catch (error) {
+    expect(error.message).toBe("");
+  }
+});

test/index.test.ts

@@ -0,0 +1,25 @@
+import { request } from "@octokit/request";
+import fetchMock, { MockMatcherFunction } from "fetch-mock";
+
+import { createUnauthenticatedAuth } from "../src/index";
+
+test('https://github.com/octokit/auth-unauthenticated.js/issues/29', async () => {
+  const requestMock = request.defaults({
+    headers: {
+      "user-agent": "test",
+    },
+    request: {
+      fetch: fetchMock.sandbox().postOnce('path:/app-manifests/123/conversions', { 
+        status: 201,
+        body: { id: 1}
+       }),
+    },
+  });
+
+  const { hook } = createUnauthenticatedAuth({ reason: "test" });
+  const { data } = await hook(requestMock, "POST /app-manifests/{code}/conversions", {
+    code: 123
+  });
+
+  expect(data).toStrictEqual({ id: 1 });
+});