diff --git a/src/api/routes.ts b/src/api/routes.ts index 7b8fa7907a..3c3a352fe0 100644 --- a/src/api/routes.ts +++ b/src/api/routes.ts @@ -1293,25 +1293,42 @@ export function createApp() { // Public-safe README status badge (#541). Unauthenticated and embeddable: it serves ONLY whitelisted, // repo-level metrics, and ONLY for installed repos that opted in via the `badgeEnabled` setting. Excluded // from requiresApiToken above; aggressively cached + stale-while-revalidate like the public stats route. + // #8377: loadPublicRepoBadge is NOT fail-safe (D1 reads plus a possible cold-cache GitHub manifest fetch), + // so a transient blip used to escape as Hono's bare 500 — unusually visible here, since these badges are + // embedded in third-party READMEs behind GitHub's camo proxy. Same route-level try/catch the sibling + // /quality route and the #4995 relay fix already use (the loader itself is untouched). 503, never 404: + // 404 stays reserved for the real "no public badge for this repo" case so a monitor can tell the two + // apart, and the 503 branch uses the SHORT cache so a transient failure is never cached for the long + // stale-while-revalidate window. app.get("/v1/public/repos/:owner/:repo/badge.svg", async (c) => { - const quality = await loadPublicRepoBadge(c.env, c.req.param("owner"), c.req.param("repo")); c.header("Content-Type", "image/svg+xml; charset=utf-8"); - if (!quality) { + try { + const quality = await loadPublicRepoBadge(c.env, c.req.param("owner"), c.req.param("repo")); + if (!quality) { + c.header("Cache-Control", "public, max-age=300"); + return c.body(renderUnavailableBadgeSvg(), 404); + } + c.header("Cache-Control", "public, max-age=600, stale-while-revalidate=86400"); + return c.body(renderBadgeSvg(quality)); + } catch { c.header("Cache-Control", "public, max-age=300"); - return c.body(renderUnavailableBadgeSvg(), 404); + return c.body(renderUnavailableBadgeSvg(), 503); } - c.header("Cache-Control", "public, max-age=600, stale-while-revalidate=86400"); - return c.body(renderBadgeSvg(quality)); }); app.get("/v1/public/repos/:owner/:repo/badge.json", async (c) => { - const quality = await loadPublicRepoBadge(c.env, c.req.param("owner"), c.req.param("repo")); - if (!quality) { + try { + const quality = await loadPublicRepoBadge(c.env, c.req.param("owner"), c.req.param("repo")); + if (!quality) { + c.header("Cache-Control", "public, max-age=300"); + return c.json({ schemaVersion: 1, label: PUBLIC_BADGE_LABEL, message: "unavailable", color: "#9e9e9e", cacheSeconds: 300 }, 404); + } + c.header("Cache-Control", "public, max-age=600, stale-while-revalidate=86400"); + return c.json(buildShieldsBadge(quality, 600)); + } catch { c.header("Cache-Control", "public, max-age=300"); - return c.json({ schemaVersion: 1, label: PUBLIC_BADGE_LABEL, message: "unavailable", color: "#9e9e9e", cacheSeconds: 300 }, 404); + return c.json({ schemaVersion: 1, label: PUBLIC_BADGE_LABEL, message: "unavailable", color: "#9e9e9e", cacheSeconds: 300 }, 503); } - c.header("Cache-Control", "public, max-age=600, stale-while-revalidate=86400"); - return c.json(buildShieldsBadge(quality, 600)); }); // Public per-repo review-quality metrics (#2568). Unauthenticated; aggregate counts/rates only; opt-in via diff --git a/test/integration/api.test.ts b/test/integration/api.test.ts index c8b219e90f..3df7a35f2c 100644 --- a/test/integration/api.test.ts +++ b/test/integration/api.test.ts @@ -347,6 +347,27 @@ describe("api routes", () => { const unknown = await app.request("/v1/public/repos/acme/missing/badge.json", {}, env); expect(unknown.status).toBe(404); await expect(unknown.json()).resolves.toMatchObject({ message: "unavailable" }); + + // REGRESSION (#8377): a backend failure inside loadPublicRepoBadge must render the graceful + // "unavailable" badge with 503 — not escape as Hono's bare, unstructured 500 (no app.onError exists). + // 503 (not 404) so a monitor can tell a transient backend problem apart from "this repo has no badge", + // and the short cache so a blip is never cached for the long stale-while-revalidate window. + const brokenEnv = withRepositoryReadFailure(env); + const failedSvg = await app.request("/v1/public/repos/acme/badged/badge.svg", {}, brokenEnv); + expect(failedSvg.status).toBe(503); + expect(failedSvg.headers.get("content-type")).toContain("image/svg+xml"); + expect(failedSvg.headers.get("cache-control")).toBe("public, max-age=300"); + expect(await failedSvg.text()).toContain("unavailable"); + + const failedJson = await app.request("/v1/public/repos/acme/badged/badge.json", {}, brokenEnv); + expect(failedJson.status).toBe(503); + expect(failedJson.headers.get("cache-control")).toBe("public, max-age=300"); + await expect(failedJson.json()).resolves.toMatchObject({ schemaVersion: 1, label: "loopover", message: "unavailable", cacheSeconds: 300 }); + + // The healthy path is untouched by the new guard — still 200 with the long cache. + const stillOk = await app.request("/v1/public/repos/acme/badged/badge.svg", {}, env); + expect(stillOk.status).toBe(200); + expect(stillOk.headers.get("cache-control")).toContain("stale-while-revalidate"); }); it("serves public per-repo review-quality metrics only for installed, opted-in repos (#2568)", async () => { @@ -7070,6 +7091,24 @@ function mcpHeaders(env: Env, sessionId?: string): Record { }; } +/** #8377: make the badge loader's first D1 read (getRepository) throw, so the route's own catch is what + * produces the response — the same targeted-`prepare` technique as {@link withProductUsageInsertFailure}. */ +function withRepositoryReadFailure(env: Env): Env { + const db = env.DB as unknown as { prepare(sql: string): unknown; batch(statements: unknown[]): Promise }; + return { + ...env, + DB: { + prepare(sql: string) { + if (sql.includes("repositories")) throw new Error("d1 blip reading repositories"); + return db.prepare.call(db, sql); + }, + batch(statements: unknown[]) { + return db.batch.call(db, statements); + }, + } as unknown as D1Database, + }; +} + function internalHeaders(env: Env): Record { return { authorization: `Bearer ${env.INTERNAL_JOB_TOKEN}`,