diff --git a/src/review/visual/preview-url.ts b/src/review/visual/preview-url.ts index 3f3a18c985..8acead866f 100644 --- a/src/review/visual/preview-url.ts +++ b/src/review/visual/preview-url.ts @@ -100,14 +100,14 @@ async function findAcrossPages( firstPageUrl: string, init: GithubJsonInit, selectItems: (payload: unknown) => TItem[], - probe: (items: TItem[]) => TResult | null, + probe: (items: TItem[]) => TResult | null | Promise, ): Promise { for (let page = 1; page <= PREVIEW_LIST_MAX_PAGES; page += 1) { // Callers pass a `per_page=100` first-page URL; append the 1-based page cursor for page 2+ only (page 1 is // GitHub's default, so leaving it bare keeps that request byte-identical to the pre-pagination read). const url = page === 1 ? firstPageUrl : `${firstPageUrl}&page=${page}`; const { payload, link } = await githubJsonWithLink(url, init); - const found = probe(selectItems(payload)); + const found = await probe(selectItems(payload)); if (found !== null) return found; if (!hasNextPage(link)) return null; } @@ -138,13 +138,56 @@ export async function getLatestDeploymentStatus(params: { ? `ref=${encodeURIComponent(params.ref)}` : ""; if (!selector) return { url: null, failed: false }; - let deployments: Array<{ id?: number }>; - try { - deployments = await githubJson>(`${base}/deployments?${selector}&per_page=10`, { - token: params.token, - apiVersion: params.apiVersion, - rateLimitAdmissionKey: params.rateLimitAdmissionKey, + const opts = { token: params.token, apiVersion: params.apiVersion, rateLimitAdmissionKey: params.rateLimitAdmissionKey }; + type DeploymentStatus = { state?: string; environment_url?: string }; + const selectStatuses = (payload: unknown) => (Array.isArray(payload) ? (payload as DeploymentStatus[]) : []); + const probeStatusesForUrl = (statuses: DeploymentStatus[]) => { + for (const status of statuses) { + const ok = status.state === "success" || status.state === "in_progress"; + if (ok && status.environment_url) return status.environment_url; + } + return null; + }; + const inspectDeploymentStatuses = async (deploymentId: number): Promise<{ url: string | null; latestState?: string }> => { + let latestState: string | undefined; + let capturedLatest = false; + const url = await findAcrossPages( + `${base}/deployments/${deploymentId}/statuses?per_page=10`, + opts, + selectStatuses, + (statuses) => { + if (!capturedLatest) { + latestState = statuses[0]?.state; + capturedLatest = true; + } + return probeStatusesForUrl(statuses); + }, + ).catch((error) => { + console.log(JSON.stringify({ event: "deployment_status_error", deployment: deploymentId, message: String(error).slice(0, 200) })); + return null; }); + if (url) return { url }; + return latestState !== undefined ? { url: null, latestState } : { url: null }; + }; + let sawFailure = false; + let sawPending = false; + try { + const url = await findAcrossPages<{ id?: number }, string>( + `${base}/deployments?${selector}&per_page=10`, + opts, + (payload) => (Array.isArray(payload) ? (payload as Array<{ id?: number }>) : []), + async (deployments) => { + for (const deployment of deployments) { + if (deployment.id == null) continue; + const { url: foundUrl, latestState } = await inspectDeploymentStatuses(deployment.id); + if (foundUrl) return foundUrl; + if (latestState === "failure" || latestState === "error") sawFailure = true; + else if (latestState === "in_progress" || latestState === "queued" || latestState === "pending") sawPending = true; + } + return null; + }, + ); + if (url) return { url, failed: false }; } catch (error) { // 404 → the ref genuinely has no deployments. Any other failure (403 missing scope, rate limit, 5xx) is // NOT "no preview"; report `error` so the caller keeps polling rather than showing a false terminal state. @@ -152,30 +195,6 @@ export async function getLatestDeploymentStatus(params: { console.log(JSON.stringify({ event: "deployment_lookup_error", repo: `${params.repo.owner}/${params.repo.repo}`, selector, message: String(error).slice(0, 200) })); return { url: null, failed: false, error: true }; } - const ids = deployments.map((d) => d.id).filter((id): id is number => id != null); - const statusLists = await Promise.all( - ids.map((id) => - githubJson>(`${base}/deployments/${id}/statuses?per_page=10`, { - token: params.token, - apiVersion: params.apiVersion, - rateLimitAdmissionKey: params.rateLimitAdmissionKey, - }).catch((error) => { - console.log(JSON.stringify({ event: "deployment_status_error", deployment: id, message: String(error).slice(0, 200) })); - return [] as Array<{ state?: string; environment_url?: string }>; - }), - ), - ); - let sawFailure = false; - let sawPending = false; - for (const statuses of statusLists) { - for (const status of statuses) { - const ok = status.state === "success" || status.state === "in_progress"; - if (ok && status.environment_url) return { url: status.environment_url, failed: false }; - } - const latest = statuses[0]?.state; - if (latest === "failure" || latest === "error") sawFailure = true; - else if (latest === "in_progress" || latest === "queued" || latest === "pending") sawPending = true; - } return { url: null, failed: sawFailure && !sawPending }; } diff --git a/test/unit/preview-url.test.ts b/test/unit/preview-url.test.ts index 19b91c8e8d..517d6cd642 100644 --- a/test/unit/preview-url.test.ts +++ b/test/unit/preview-url.test.ts @@ -1,6 +1,6 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { clearGitHubResponseCacheForTest, githubRateLimitAdmissionKeyForInstallation, latestGitHubRestRateLimitObservation } from "../../src/github/client"; -import { extractPreviewUrl, findPreviewUrlFromPrComments, getPreviewBuildState } from "../../src/review/visual/preview-url"; +import { extractPreviewUrl, findPreviewUrlFromPrComments, getLatestDeploymentStatus, getPreviewBuildState } from "../../src/review/visual/preview-url"; /** GitHub's `Link` header for a page that advertises a next page (the exact shape findAcrossPages walks). */ const NEXT_LINK = '; rel="next", ; rel="last"'; @@ -166,6 +166,56 @@ describe("preview-url pagination (#7450)", () => { await expect(getPreviewBuildState({ token: "t", repo: REPO, sha: "fail" })).resolves.toBe("absent"); expect(failLater).toHaveBeenCalledTimes(2); }); + + it("getLatestDeploymentStatus follows Link: rel=next on deployments and finds the preview URL on page 2 (#7805)", async () => { + const page1Deployments = Array.from({ length: 10 }, (_v, i) => ({ id: i + 1 })); + const page2Deployments = [{ id: 99 }]; + const fetchMock = vi.fn(async (input: RequestInfo | URL) => { + const url = String(input); + if (url.includes("/deployments?") && url.includes("sha=abc")) { + return isPage2(input) + ? Response.json(page2Deployments) + : Response.json(page1Deployments, { headers: { link: NEXT_LINK } }); + } + if (url.includes("/deployments/99/statuses")) { + return Response.json([{ state: "success", environment_url: "https://pr-99.app.workers.dev" }]); + } + if (url.includes("/deployments/") && url.includes("/statuses")) { + return Response.json([{ state: "failure" }]); + } + throw new Error(`unexpected fetch: ${url}`); + }); + vi.stubGlobal("fetch", fetchMock); + + await expect(getLatestDeploymentStatus({ token: "t", repo: REPO, sha: "abc" })).resolves.toEqual({ + url: "https://pr-99.app.workers.dev", + failed: false, + }); + expect(fetchMock.mock.calls.some((c) => /\/deployments\?.*page=2/.test(String(c[0])))).toBe(true); + expect(String(fetchMock.mock.calls.find((c) => String(c[0]).includes("/deployments?"))![0])).not.toContain("&page="); + }); + + it("getLatestDeploymentStatus follows Link: rel=next on deployment statuses and finds environment_url on page 2 (#7805)", async () => { + const page1Statuses = Array.from({ length: 10 }, () => ({ state: "pending" })); + const page2Statuses = [{ state: "success", environment_url: "https://deep-status.app.workers.dev" }]; + const fetchMock = vi.fn(async (input: RequestInfo | URL) => { + const url = String(input); + if (url.includes("/deployments?")) { + return Response.json([{ id: 7 }]); + } + if (url.includes("/deployments/7/statuses")) { + return isPage2(input) ? Response.json(page2Statuses) : Response.json(page1Statuses, { headers: { link: NEXT_LINK } }); + } + throw new Error(`unexpected fetch: ${url}`); + }); + vi.stubGlobal("fetch", fetchMock); + + await expect(getLatestDeploymentStatus({ token: "t", repo: REPO, sha: "deep" })).resolves.toEqual({ + url: "https://deep-status.app.workers.dev", + failed: false, + }); + expect(fetchMock).toHaveBeenCalledTimes(2); + }); }); describe("extractPreviewUrl", () => {