From 0929cd2fac1d55105d882d92d6cd05209f55521c Mon Sep 17 00:00:00 2001 From: Chris Dyson Date: Mon, 21 Sep 2026 07:27:02 +1200 Subject: [PATCH] Stop the capture's own console noise from holding every screenshot PR The audit added in #61 records every console.error the console logs during a capture and holds the PR for a human when there is one. Two of those are always there and say nothing about the screenshot: - Playwright's clock shim is injected into every frame, including the sandboxed email preview iframe, which has no allow-scripts on purpose. Chrome logs "Blocked script execution in 'about:srcdoc'". Reproduced with a script-free srcdoc iframe: only page.clock.install triggers it. - Every bucket page asks S3 for the lifecycle and website configuration. A bucket without one answers 404 NoSuchLifecycleConfiguration / NoSuchWebsiteConfiguration, as real S3 does, and the console maps that to "not configured". Chrome logs any 404 before the page handles it. The capture's bucket is the example stack's VersionedBucket, which has no lifecycle rules. isExpectedPageLog names both exactly. The 404 rule needs the failing request, so the capture now appends message.location().url to console.error entries (a "Failed to load resource" message does not say which resource). A 404 for any other URL, a lifecycle request that fails with anything but 404, and every other console.error still hold the PR. Filtered entries stay in the per-attempt debug log. --- scripts/capture-console.mjs | 12 +++++++--- src/lib/screenshot-audit.test.ts | 38 ++++++++++++++++++++++++++++++++ src/lib/screenshot-audit.ts | 30 +++++++++++++++++++++++++ 3 files changed, 77 insertions(+), 3 deletions(-) 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.