diff --git a/scripts/capture-console.mjs b/scripts/capture-console.mjs index cc4af30..7b67e75 100644 --- a/scripts/capture-console.mjs +++ b/scripts/capture-console.mjs @@ -34,7 +34,7 @@ import path from "node:path"; import process from "node:process"; import { chromium } from "playwright"; import sharp from "sharp"; -import { isBlankFrame } from "../src/lib/screenshot-audit.ts"; +import { isBlankFrame, isExpectedPageLog } from "../src/lib/screenshot-audit.ts"; const cwd = process.cwd(); const outputDir = path.join(cwd, "public", "console"); @@ -307,7 +307,10 @@ async function runScenario(browser, scenario, theme, attempt) { const pageLog = []; page.on("pageerror", (error) => pageLog.push(`pageerror: ${error.message}`)); page.on("console", (message) => { - if (message.type() === "error") pageLog.push(`console.error: ${message.text()}`); + if (message.type() !== "error") return; + // A failed subresource load is reported without saying which one; the location carries it. + const { url } = message.location(); + pageLog.push(`console.error: ${message.text()}${url ? ` [${url}]` : ""}`); }); page.on("requestfailed", (request) => pageLog.push(`requestfailed: ${request.url()} ${request.failure()?.errorText}`)); @@ -344,7 +347,10 @@ async function runScenario(browser, scenario, theme, attempt) { if (isBlankFrame(data)) throw new Error(`Captured frame is blank or near-blank: ${path.basename(file)}`); await context.tracing.stop(); - return pageLog.map((message) => ({ scenario: `${scenario.name}-${theme}`, message })); + // Expected noise stays in the debug log written on failure, but is not a reason to hold the PR. + return pageLog + .filter((message) => !isExpectedPageLog(message)) + .map((message) => ({ scenario: `${scenario.name}-${theme}`, message })); } catch (error) { const prefix = path.join(debugDir, `${scenario.name}-${theme}-attempt-${attempt}`); await fs.mkdir(debugDir, { recursive: true }); diff --git a/src/lib/screenshot-audit.test.ts b/src/lib/screenshot-audit.test.ts index 84bd16e..3ba6908 100644 --- a/src/lib/screenshot-audit.test.ts +++ b/src/lib/screenshot-audit.test.ts @@ -13,6 +13,7 @@ import { formatComparisonTable, formatPercent, isBlankFrame, + isExpectedPageLog, REVIEW_CHANGE_RATIO, type CaptureWarning, type ScreenshotComparison, @@ -186,3 +187,40 @@ describe("formatComparisonTable", () => { assert.match(table, /\| `new\.png` \| new file \|/); }); }); + +describe("isExpectedPageLog", () => { + const notFound = "console.error: Failed to load resource: the server responded with a status of 404 (Not Found)"; + + it("ignores the clock shim being blocked in the sandboxed email preview", () => { + assert.ok( + isExpectedPageLog( + "console.error: Blocked script execution in 'about:srcdoc' because the document's frame is sandboxed and the 'allow-scripts' permission is not set. [about:srcdoc]", + ), + ); + }); + + it("ignores the 404 for a bucket with no lifecycle or website configuration", () => { + assert.ok(isExpectedPageLog(`${notFound} [http://localhost:4566/some-bucket?lifecycle&x-id=GetBucketLifecycleConfiguration]`)); + assert.ok(isExpectedPageLog(`${notFound} [http://some-bucket.localhost.overcast.sh:4566/?website&x-id=GetBucketWebsite]`)); + }); + + it("still flags a 404 for any other request", () => { + assert.equal(isExpectedPageLog(`${notFound} [http://localhost:4566/some-bucket/samples/missing.png]`), false); + assert.equal(isExpectedPageLog(`${notFound} [http://localhost:4567/api/things?lifecycle-hook=1]`), false); + }); + + it("flags a 404 that names no request, since nothing shows it is the expected one", () => { + assert.equal(isExpectedPageLog(notFound), false); + }); + + it("still flags a lifecycle request that failed some other way", () => { + const serverError = "console.error: Failed to load resource: the server responded with a status of 500 (Internal Server Error)"; + assert.equal(isExpectedPageLog(`${serverError} [http://localhost:4566/some-bucket?lifecycle]`), false); + }); + + it("does not swallow other console errors or other blocked scripts", () => { + assert.equal(isExpectedPageLog("console.error: Uncaught TypeError: x is not a function"), false); + assert.equal(isExpectedPageLog("console.error: Blocked script execution in 'http://localhost:4567/' because it is sandboxed"), false); + assert.equal(isExpectedPageLog("requestfailed: http://localhost:4567/x net::ERR_ABORTED"), false); + }); +}); diff --git a/src/lib/screenshot-audit.ts b/src/lib/screenshot-audit.ts index 3d9a78b..9b6632f 100644 --- a/src/lib/screenshot-audit.ts +++ b/src/lib/screenshot-audit.ts @@ -63,6 +63,36 @@ export interface CaptureWarning { message: string; } +/** + * Page-log entries that say nothing about the screenshot. Anything else logged by the + * console still holds the PR for a human, so each rule here is narrow: it names the exact + * message, and for a network error the exact request, rather than a whole class of failure. + * + * - Playwright's clock shim (`page.clock.install`) is injected into every frame, including + * the sandboxed email preview iframe. That iframe has no `allow-scripts` on purpose, so + * Chrome logs the block. The message comes from the harness; the seeded email has no + * script in it. + * - The console asks S3 for a bucket's lifecycle and website configuration on every bucket + * page. A bucket without one gets a 404 (`NoSuchLifecycleConfiguration`, + * `NoSuchWebsiteConfiguration`), which is what real S3 answers and which the console maps + * to "not configured". The page handles it, but Chrome logs any 404 before JS sees it. + */ +const SANDBOXED_SRCDOC_BLOCK = /^console\.error: Blocked script execution in 'about:srcdoc' because the document's frame is sandboxed/; +const RESOURCE_404 = /^console\.error: Failed to load resource: the server responded with a status of 404\b.*\[([^\]]*)\]$/; +const ABSENT_CONFIG_REQUEST = /[?&](?:lifecycle|website)(?:[=&]|$)/; + +/** + * Whether a page-log entry from scripts/capture-console.mjs is expected noise. An entry is + * `console.error: `, with ` []` appended when Chrome reported which resource + * failed to load (Playwright's `message.location().url`). + */ +export function isExpectedPageLog(entry: string): boolean { + if (SANDBOXED_SRCDOC_BLOCK.test(entry)) return true; + + const failedResource = RESOURCE_404.exec(entry); + return failedResource != null && ABSENT_CONFIG_REQUEST.test(failedResource[1]); +} + /** * The fraction of the image taken by its single most common colour. 1 means every pixel is * identical; a real page lands far below the blank threshold.