diff --git a/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts b/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts index 8daf82277a083..6227d6c533297 100644 --- a/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts +++ b/packages/source-control-github/src/server/GitHubPullRequestApi.test.ts @@ -42,6 +42,12 @@ const refusedReviewer = { message: "Resource not accessible by integration", path: ["repository", "pullRequest", "reviewRequests", "nodes", 0, "requestedReviewer"], }; +// What GitHub answers for a pull request that has no `refs/pull//head`. +const unresolvedPullRef = { + type: "NOT_FOUND", + message: "Could not resolve head ref 'refs/pull/7/head'.", + path: ["repository", "pullRequest", "baseRef", "compare"], +}; const pullRequestInput = { cwd: "/w", repository: "acme/web", host: "github.com", number: 7 }; type PullRequestRead = ( input: typeof pullRequestInput, @@ -2546,6 +2552,23 @@ layer("GitHubPullRequestApi.layer", (it) => { }), ); + it.effect.each([ + { action: "merge", mutation: "mergePullRequest(" }, + { action: "update-branch", mutation: "updatePullRequestBranch(" }, + ] as const)("runs $action on a pull request GitHub has no pull ref for", ({ action, mutation }) => + Effect.gen(function* () { + route([ + "query PullRequestActionState", + { ...actionState({ baseRef: { compare: null } }), errors: [unresolvedPullRef] }, + ]); + const cli = yield* GitHubPullRequestApi.GitHubPullRequestApi; + + yield* cli.runPullRequestAction({ ...pullRequestInput, action }); + + expect(variablesOf(mutation)).toHaveLength(1); + }), + ); + it.effect("merges at once when auto-merge is asked of a pull request that is ready now", () => Effect.gen(function* () { route(["query PullRequestActionState", actionState({ mergeStateStatus: "CLEAN" })]); @@ -3457,7 +3480,12 @@ layer("GitHubPullRequestApi.layer", (it) => { }).pipe(Effect.asVoid, Effect.flip); expect(error._tag).toBe(tag ?? "GitHubApiResponseError"); if (error._tag === "GitHubApiResponseError") - expect(error).toMatchObject({ status, githubErrors: errors.map((e) => e.message) }); + expect(error).toMatchObject({ + status, + githubErrors: errors.map((e) => + "path" in e ? `${e.message} (at ${e.path.join(".")})` : e.message, + ), + }); expect(mockedExecute).toHaveBeenCalledTimes(index + 1); } }), @@ -3569,6 +3597,91 @@ layer("GitHubPullRequestApi.layer", (it) => { }), ); + const unresolvedCore = { + ...coreResponse({ baseRef: { compare: null } }), + errors: [unresolvedPullRef], + }; + it.effect.each([ + { description: "alone", answers: [unresolvedCore], requestedReviewers: [100] }, + { + description: "beside a refused reviewer", + answers: [ + { ...unresolvedCore, errors: [unresolvedPullRef, refusedReviewer] }, + unresolvedCore, + ], + requestedReviewers: [100, 0], + }, + ])( + "reads a pull request without a pull ref, its comparison unknown ($description)", + ({ answers, requestedReviewers }) => + Effect.gen(function* () { + for (const answer of answers) + mockedExecute.mockReturnValueOnce(Effect.succeed(output(encodeJson(answer)))); + const cli = yield* GitHubPullRequestApi.GitHubPullRequestApi; + const detail = yield* cli.getPullRequestDetail(pullRequestInput); + expect(detail.number).toBe(7); + expect(detail.comparison).toBeNull(); + expect(variablesOf("PullRequestCore").map((v) => v["requestedReviewers"])).toEqual( + requestedReviewers, + ); + }), + ); + + it.effect("fails a missing pull ref beside another error, in GitHub's words", () => + Effect.gen(function* () { + const refusedChecks = { + type: "FORBIDDEN", + message: "Resource not accessible by integration", + path: ["repository", "pullRequest", "commits", "nodes", 0, "commit", "statusCheckRollup"], + }; + mockedExecute.mockReturnValueOnce( + Effect.succeed( + output(encodeJson({ ...unresolvedCore, errors: [unresolvedPullRef, refusedChecks] })), + ), + ); + const cli = yield* GitHubPullRequestApi.GitHubPullRequestApi; + const error = yield* Effect.flip(cli.getPullRequestDetail(pullRequestInput)); + expect(error).toMatchObject({ + _tag: "GitHubApiResponseError", + status: 200, + githubErrors: [ + "Could not resolve head ref 'refs/pull/7/head'. (at repository.pullRequest.baseRef.compare)", + "Resource not accessible by integration (at repository.pullRequest.commits.nodes.0.commit.statusCheckRollup)", + ], + }); + }), + ); + + it.effect("still reads a pull request GitHub cannot find as missing, in GitHub's words", () => + Effect.gen(function* () { + const { data } = coreResponse(); + mockedExecute.mockReturnValueOnce( + Effect.succeed( + output( + encodeJson({ + data: { repository: { ...data.repository, pullRequest: null } }, + errors: [ + { + type: "NOT_FOUND", + message: "Could not resolve to a PullRequest with the number of 7.", + path: ["repository", "pullRequest"], + }, + ], + }), + ), + ), + ); + const cli = yield* GitHubPullRequestApi.GitHubPullRequestApi; + const error = yield* Effect.flip(cli.getPullRequestDetail(pullRequestInput)); + expect(error).toMatchObject({ + _tag: "GitHubApiNotFoundError", + githubErrors: [ + "Could not resolve to a PullRequest with the number of 7. (at repository.pullRequest)", + ], + }); + }), + ); + it.effect.each([false, true])( "continues after the core check page without reading it twice (reviewer refusal: %s)", (refused) => diff --git a/packages/source-control-github/src/server/GitHubPullRequestApi.ts b/packages/source-control-github/src/server/GitHubPullRequestApi.ts index 6db55da028411..284034e28b48d 100644 --- a/packages/source-control-github/src/server/GitHubPullRequestApi.ts +++ b/packages/source-control-github/src/server/GitHubPullRequestApi.ts @@ -54,7 +54,11 @@ import { PullRequestMutationsGraphQLGroup, type PullRequestActionSummary, } from "./pullRequestMutations.graphql.ts"; -import { PullRequestReadsGraphQLGroup } from "./pullRequestReads.graphql.ts"; +import { + PullRequestActionState, + PullRequestCore, + PullRequestReadsGraphQLGroup, +} from "./pullRequestReads.graphql.ts"; import { PullRequestReviewsGraphQLGroup } from "./pullRequestReviews.graphql.ts"; import * as SourceControlHost from "@t3tools/source-control-core/server/SourceControlHost"; import * as SourceControlRateLimit from "@t3tools/source-control-core/server/SourceControlRateLimit"; @@ -320,23 +324,54 @@ const isGraphQlClientError = (error: unknown): error is GraphQLClientError.Graph const { isNotFoundOnly } = GitHubGraphQlClient; +const isMissingPullRef = ({ path, type }: GraphQLClientError.GraphQLError): boolean => + type === "NOT_FOUND" && path?.join(".") === "repository.pullRequest.baseRef.compare"; + const isRefusedRequestedReviewersOnly = (error: unknown): boolean => isGraphQlClientError(error) && error.reason._tag === "ResponseError" && error.reason.status === 200 && - error.reason.errors.every( - ({ path, type }) => - type === "FORBIDDEN" && - path?.length === 6 && - path[0] === "repository" && - path[1] === "pullRequest" && - path[2] === "reviewRequests" && - path[3] === "nodes" && - typeof path[4] === "number" && - Number.isInteger(path[4]) && - path[4] >= 0 && - path[5] === "requestedReviewer", - ); + // A missing pull ref can come along; the retried read tolerates it. + error.reason.errors.every((graphQlError) => { + const { path, type } = graphQlError; + return ( + isMissingPullRef(graphQlError) || + (type === "FORBIDDEN" && + path?.length === 6 && + path[0] === "repository" && + path[1] === "pullRequest" && + path[2] === "reviewRequests" && + path[3] === "nodes" && + typeof path[4] === "number" && + Number.isInteger(path[4]) && + path[4] >= 0 && + path[5] === "requestedReviewer") + ); + }); + +const isMissingPullRefOnly = ( + error: E, +): error is E & + GraphQLClientError.GraphQLClientError & { readonly reason: GraphQLClientError.ResponseError } => + isGraphQlClientError(error) && + error.reason._tag === "ResponseError" && + error.reason.status === 200 && + error.reason.errors.every(isMissingPullRef); + +/** + * GitHub can lack `refs/pull//head` for a pull request it otherwise answers, and then + * fails only the comparison against it. The rest of that answer stands, its comparison unknown. + */ +const toleratingMissingPullRef = + (decode: (data: unknown) => Effect.Effect) => + (effect: Effect.Effect) => + effect.pipe( + Effect.catchIf(isMissingPullRefOnly, (error) => + decode(error.reason.data).pipe(Effect.mapError(() => error)), + ), + ); +const decodePullRequestCore = Schema.decodeUnknownEffect(PullRequestCore.result); +const decodePullRequestActionState = Schema.decodeUnknownEffect(PullRequestActionState.result); /** A large pull request can produce a multi-megabyte patch; past this it is truncated. */ const DIFF_MAX_OUTPUT_BYTES = 8 * 1024 * 1024; @@ -1455,14 +1490,16 @@ export const make = Effect.gen(function* () { const { owner, name } = parseRepositorySelector(input.repository); const operation = "getPullRequestDetail"; const read = (requestedReviewers: number) => - client.PullRequestCore({ - owner, - name, - number: input.number, - headRef: `refs/pull/${input.number}/head`, - required: asksRequired(input.host), - requestedReviewers, - }); + client + .PullRequestCore({ + owner, + name, + number: input.number, + headRef: `refs/pull/${input.number}/head`, + required: asksRequired(input.host), + requestedReviewers, + }) + .pipe(toleratingMissingPullRef(decodePullRequestCore)); return SourceControlRateLimit.Interactive.pipe( Effect.flatMap((allowReserve) => readRequestedReviewers(input, 100, read).pipe( @@ -1691,6 +1728,7 @@ export const make = Effect.gen(function* () { ? new GitHubRequestPolicy.GitHubApiNotFoundError({ host: input.host, operation: input.operation, + githubErrors: GitHubGraphQlClient.describeErrors(error), }) : readError(input.cwd, input.operation, error), ), @@ -2634,6 +2672,7 @@ export const make = Effect.gen(function* () { headRef: `refs/pull/${input.number}/head`, }) .pipe( + toleratingMissingPullRef(decodePullRequestActionState), graphql({ ...target, allowReserve: true }), Effect.map((result) => result.repository?.pullRequest ?? null), Effect.flatMap(found(target)), diff --git a/packages/source-control-github/src/server/GitHubRequestPolicy.ts b/packages/source-control-github/src/server/GitHubRequestPolicy.ts index 0d48932db327b..89bd70df4f0c9 100644 --- a/packages/source-control-github/src/server/GitHubRequestPolicy.ts +++ b/packages/source-control-github/src/server/GitHubRequestPolicy.ts @@ -69,12 +69,17 @@ export class GitHubApiNotFoundError extends Schema.TaggedError 0 + ? `${missing} GitHub said: ${this.githubErrors.join("; ")}` + : missing; } } diff --git a/packages/source-control-github/src/server/gitHubGraphQlClient.test.ts b/packages/source-control-github/src/server/gitHubGraphQlClient.test.ts index 769a8ce65f259..eaec0d33d60d9 100644 --- a/packages/source-control-github/src/server/gitHubGraphQlClient.test.ts +++ b/packages/source-control-github/src/server/gitHubGraphQlClient.test.ts @@ -280,7 +280,9 @@ describe("call", () => { const { layer } = harness(() => json({ data: { repository: null }, - errors: [{ type: "NOT_FOUND", message: "Could not resolve to a Repository" }], + errors: [ + { type: "NOT_FOUND", path: ["repository"], message: "Could not resolve to a Repository" }, + ], }), ); return Effect.gen(function* () { @@ -289,10 +291,39 @@ describe("call", () => { _tag: "GitHubApiNotFoundError", host: "github.com", operation: "stats", + githubErrors: ["Could not resolve to a Repository (at repository)"], }); }).pipe(Effect.provide(layer)); }); + it.effect( + "fails a NOT_FOUND below the pull request as GitHub's reason, not a missing one", + () => { + const { layer } = harness(() => + json({ + data: { repository: { pullRequest: stats } }, + errors: [ + { + type: "NOT_FOUND", + path: ["repository", "pullRequest", "baseRef", "compare"], + message: "Could not resolve head ref 'refs/pull/7/head'.", + }, + ], + }), + ); + return Effect.gen(function* () { + const error = yield* Effect.flip(callStats()); + expect(error).toMatchObject({ + _tag: "GitHubApiResponseError", + status: 200, + githubErrors: [ + "Could not resolve head ref 'refs/pull/7/head'. (at repository.pullRequest.baseRef.compare)", + ], + }); + }).pipe(Effect.provide(layer)); + }, + ); + it.effect( "fails NOT_FOUND alongside another error as a failure carrying GitHub's reasons", () => { diff --git a/packages/source-control-github/src/server/gitHubGraphQlClient.ts b/packages/source-control-github/src/server/gitHubGraphQlClient.ts index 484e808b0cd49..b948d5e605a49 100644 --- a/packages/source-control-github/src/server/gitHubGraphQlClient.ts +++ b/packages/source-control-github/src/server/gitHubGraphQlClient.ts @@ -43,10 +43,31 @@ function queryHash(query: string): string { return (hash >>> 0).toString(16).padStart(8, "0"); } +/** + * Whether every error is a NOT_FOUND on a lookup: a root field (with its list items), + * `repository.pullRequest` or `repository.issueOrPullRequest`. A NOT_FOUND deeper down is one + * field GitHub could not resolve. + */ export const isNotFoundOnly = (error: unknown): boolean => GraphQLClientError.isGraphQLClientError(error) && error.reason._tag === "ResponseError" && - error.reason.errors.every((graphQlError) => graphQlError["type"] === "NOT_FOUND"); + error.reason.errors.every( + ({ type, path }) => + type === "NOT_FOUND" && + (path === undefined || + path.slice(1).every((segment) => typeof segment === "number") || + (path.length === 2 && + path[0] === "repository" && + (path[1] === "pullRequest" || path[1] === "issueOrPullRequest"))), + ); + +/** GitHub's message for each error in an answer, with the field it names. */ +export const describeErrors = (error: GraphQLClientError.GraphQLClientError) => + error.reason._tag === "ResponseError" + ? error.reason.errors.map(({ message, path }) => + path === undefined || path.length === 0 ? message : `${message} (at ${path.join(".")})`, + ) + : []; /** Share REST's credentials/slots/quota/pause; leave non-rate-limit errors to each lookup. */ export class GitHubGraphQlAuth extends GraphQLMiddleware.Service< @@ -234,6 +255,7 @@ const fromClientError = ( const context = { host: normalizeHost(target.host), operation: target.operation, + githubErrors: describeErrors(error), }; const credential = Option.filter( Headers.get(reason.headers ?? Headers.empty, CREDENTIAL_SOURCE_HEADER), @@ -247,7 +269,6 @@ const fromClientError = ( : new GitHubRequestPolicy.GitHubApiResponseError({ ...context, status: reason.status ?? 200, - githubErrors: reason.errors.map((graphQlError) => graphQlError.message), githubErrorTypes: reason.errors.map((graphQlError) => typeof graphQlError["type"] === "string" ? graphQlError["type"] : null, ),