fix(server): treat removed Bitbucket permissions endpoint as unknown, not blocking (#6525)

Lars Nieuwenhuis committed 2 months ago

Commit 75472801 parent afca73d2 files · +58 −1Browse files

Unified · Split

2 files

apps/server/src/pullRequest/BitbucketPullRequestApi.test.ts +40 −0
apps/server/src/pullRequest/BitbucketPullRequestApi.ts +18 −1

apps/server/src/pullRequest/BitbucketPullRequestApi.test.ts+40−0

@@ -867,6 +867,46 @@ layer("BitbucketPullRequestApi.layer", (it) => { · 866 unmodified lines
867	}),	867	}),
868	);	868	);
869		869
		870	it.effect(
		871	"reads a removed permissions endpoint as granted rather than failing the merge on it",
		872	() =>
		873	Effect.gen(function* () {
		874	// Bitbucket retired /user/permissions/repositories under CHANGE-2770: every account now
		875	// gets HTTP 410 here, whatever it may do.
		876	mockedRequest.mockReturnValue(
		877	Effect.fail(
		878	new BitbucketApi.BitbucketResponseError({
		879	operation: "request",
		880	status: 410,
		881	responseBodyLength: 0,
		882	}),
		883	),
		884	);
		885	const api = yield* BitbucketPullRequestApi.BitbucketPullRequestApi;
		886
		887	assert.isTrue(yield* api.getRepositoryPermission({ repository: "acme/web" }));
		888	}),
		889	);
		890
		891	it.effect("still fails the permission read on a failure that is not the removed endpoint", () =>
		892	Effect.gen(function* () {
		893	mockedRequest.mockReturnValue(
		894	Effect.fail(
		895	new BitbucketApi.BitbucketResponseError({
		896	operation: "request",
		897	status: 401,
		898	responseBodyLength: 0,
		899	}),
		900	),
		901	);
		902	const api = yield* BitbucketPullRequestApi.BitbucketPullRequestApi;
		903
		904	const error = yield* Effect.flip(api.getRepositoryPermission({ repository: "acme/web" }));
		905
		906	assert.strictEqual(error._tag, "BitbucketResponseError");
		907	}),
		908	);
		909
870	it.effect("reads the workspace's people and marks whoever is already a reviewer", () =>	910	it.effect("reads the workspace's people and marks whoever is already a reviewer", () =>
871	Effect.gen(function* () {	911	Effect.gen(function* () {
872	mockedRequest	912	mockedRequest

apps/server/src/pullRequest/BitbucketPullRequestApi.ts+18−1

   | BitbucketRepositoryUnsupportedError
   | BitbucketDiffCommitError;
+/**
+ * `/user/permissions/repositories` answering CHANGE-2770's removal notice rather than a
+ * permission — Bitbucket sends this for every account now, not only ones it would have refused.
+ */
+function isRepositoryPermissionRemovedError(
+  error: BitbucketPullRequestApiError,
+): error is BitbucketApi.BitbucketResponseError {
+  return error._tag === "BitbucketResponseError" && error.status === 410;
+}
 /**
  * Bitbucket's own ceiling. Asking for more does not fail — it answers with an empty page and no
  * error at all, so this is a number to respect rather than to push against.
     // Nothing on the repository, the pull request or the workspace states what the credentials
     // may do, so this endpoint is the one request Bitbucket makes unavoidable. It is asked
     // alongside the reads the detail was already making, so it costs no round trip of its own.
+    //
+    // Bitbucket permanently removed this endpoint (CHANGE-2770): every account now gets HTTP 410
+    // in place of an answer, whatever it may do. That is the deprecated-endpoint signal, not a
+    // permission being refused, so it is read the same way an unreachable read already is
+    // elsewhere — as a permission that could not be learned, which grants rather than blocks, and
+    // leaves the actual merge or write to say why if the account may not do it. Any other failure
+    // (a bad token, a network fault, an unreadable body) still fails as it did before.
     getRepositoryPermission: (input) =>
       withRepository(input.repository, () =>
         readPage({
           )}`,
           decode: decodeRepositoryPermissionJson,
         }),
-      ),
+      ).pipe(Effect.catchIf(isRepositoryPermissionRemovedError, () => Effect.succeed(true))),
     getPullRequestDiff: (input) =>
       input.commit !== undefined && !isCommitSha(input.commit)