mirror of
https://github.com/webstudio-is/webstudio.git
synced 2026-10-23 03:27:27 +09:00
fix: Report runtime errors from MCP screenshots (#6545)
A screenshot could return success when a generated page rendered a React error boundary. The HTTP response and Webstudio root marker were valid, so the error was easy to miss. The capture now records browser JavaScript exceptions and error-level console events. MCP path screenshots of generated projects return `SCREENSHOT_JAVASCRIPT_ERROR` instead of success when either is detected. Other URL-based captures keep their existing behavior. Closes #6538 ### Checklist - [x] Capture JavaScript exceptions and error-level console events from the browser. - [x] Reject generated-project screenshots with a structured error code. - [x] Add regression coverage and verify the tests fail before the fix. - [x] Review authoritative documentation; no documentation change is needed for this error-reporting correction. - [x] Run vision and CLI tests, typechecks, lint, and a local Chrome React error-boundary capture. ### Verification - `pnpm --filter @webstudio-is/vision test` — 103 passed. - `pnpm --filter webstudio test` — 1,278 passed. - `pnpm --filter @webstudio-is/vision typecheck` — passed. - `pnpm --filter webstudio typecheck` — passed. - Oxlint and `git diff --check` — passed. - Local Chrome end-to-end: an HTTP 200 page rendered a React error boundary and was identified as a JavaScript error. Local agent evaluations were not run; they require explicit approval.
This commit is contained in:
1 parent
3f18f44456
commit
8439a1a644
4 files changed
+331
-10
No files matched your search
@@ -1917,6 +1917,7 @@ test("reports an interior route HTTP failure instead of a wrong-site error", asy
|
||||
{
|
||||
navigation: {
|
||||
generatedSiteRootPresent: false,
|
||||
javascriptErrorPresent: true,
|
||||
status: 503,
|
||||
documentReadyState: "complete",
|
||||
redirects: [],
|
||||
@@ -1943,3 +1944,44 @@ test("reports an interior route HTTP failure instead of a wrong-site error", asy
|
||||
],
|
||||
});
|
||||
});
|
||||
|
||||
test("reports a JavaScript error instead of a successful generated-site screenshot", async () => {
|
||||
const handlers = createMcpPreviewHandlers({
|
||||
preview: {
|
||||
status: vi.fn(() => ({
|
||||
url: "http://127.0.0.1:5173/",
|
||||
running: true,
|
||||
mode: "iterative" as const,
|
||||
})),
|
||||
startAndWait: vi.fn(),
|
||||
resolveUrl: vi.fn(() => "http://127.0.0.1:5173/"),
|
||||
},
|
||||
isStale: () => false,
|
||||
createCaptureSession: vi.fn(() => ({
|
||||
capture: vi.fn(async () => ({
|
||||
output: "builder.png",
|
||||
navigation: {
|
||||
generatedSiteRootPresent: true,
|
||||
javascriptErrorPresent: true,
|
||||
},
|
||||
})),
|
||||
capturePage: vi.fn(),
|
||||
close: vi.fn(),
|
||||
})) as never,
|
||||
});
|
||||
|
||||
await expect(
|
||||
handlers.captureScreenshot({
|
||||
path: "/",
|
||||
viewport: { width: 1280, height: 720 },
|
||||
})
|
||||
).rejects.toMatchObject({
|
||||
code: "SCREENSHOT_JAVASCRIPT_ERROR",
|
||||
issues: [
|
||||
{
|
||||
code: "screenshot_javascript_error",
|
||||
constraint: "javascript_runtime_error",
|
||||
},
|
||||
],
|
||||
});
|
||||
});
|
||||
@@ -535,11 +535,10 @@ export const createMcpPreviewHandlers = ({
|
||||
input: McpScreenshotInput,
|
||||
result: Awaited<ReturnType<typeof captureScreenshot>>
|
||||
) => {
|
||||
if (
|
||||
input.path !== undefined &&
|
||||
input.baseUrl === undefined &&
|
||||
result.navigation?.generatedSiteRootPresent === false
|
||||
) {
|
||||
if (input.path === undefined || input.baseUrl !== undefined) {
|
||||
return result;
|
||||
}
|
||||
if (result.navigation?.generatedSiteRootPresent === false) {
|
||||
const status = result.navigation.status;
|
||||
const routeFailed = status !== undefined && status >= 400;
|
||||
const code = routeFailed
|
||||
@@ -564,6 +563,21 @@ export const createMcpPreviewHandlers = ({
|
||||
],
|
||||
});
|
||||
}
|
||||
if (result.navigation?.javascriptErrorPresent === true) {
|
||||
const message =
|
||||
"The page reported a JavaScript error during screenshot capture.";
|
||||
throw Object.assign(new Error(message), {
|
||||
code: "SCREENSHOT_JAVASCRIPT_ERROR",
|
||||
issues: [
|
||||
{
|
||||
code: "screenshot_javascript_error",
|
||||
path: [],
|
||||
message,
|
||||
constraint: "javascript_runtime_error",
|
||||
},
|
||||
],
|
||||
});
|
||||
}
|
||||
return result;
|
||||
};
|
||||
const getManagedPreviewMetadata = (input: McpScreenshotInput) =>
|
||||
|
||||
@@ -61,6 +61,11 @@ class FakeWebSocket {
|
||||
readonly readinessWidths?: number[];
|
||||
readonly lifecycleFrames: string[];
|
||||
readonly pendingFonts: boolean;
|
||||
readonly runtimeErrorScenario?:
|
||||
| "console-error"
|
||||
| "stale-outgoing-error"
|
||||
| "hash-navigation-error";
|
||||
private navigationCount = 0;
|
||||
imageInspectionFinished = false;
|
||||
screenshotStartedBeforeImageInspectionFinished = false;
|
||||
private currentUrl = "about:blank";
|
||||
@@ -71,7 +76,8 @@ class FakeWebSocket {
|
||||
subframeStatus?: number,
|
||||
readinessWidths?: number[],
|
||||
lifecycleFrames = ["main"],
|
||||
pendingFonts = false
|
||||
pendingFonts = false,
|
||||
runtimeErrorScenario?: FakeWebSocket["runtimeErrorScenario"]
|
||||
) {
|
||||
this.status = status;
|
||||
this.finalUrl = finalUrl;
|
||||
@@ -79,6 +85,7 @@ class FakeWebSocket {
|
||||
this.readinessWidths = readinessWidths;
|
||||
this.lifecycleFrames = lifecycleFrames;
|
||||
this.pendingFonts = pendingFonts;
|
||||
this.runtimeErrorScenario = runtimeErrorScenario;
|
||||
setTimeout(() => this.emit("open"), 0);
|
||||
}
|
||||
|
||||
@@ -359,12 +366,34 @@ class FakeWebSocket {
|
||||
return;
|
||||
}
|
||||
setTimeout(() => {
|
||||
const navigationNumber = this.navigationCount + 1;
|
||||
const sameDocumentNavigation =
|
||||
message.method === "Page.navigate" &&
|
||||
this.runtimeErrorScenario === "hash-navigation-error" &&
|
||||
navigationNumber === 2;
|
||||
respond(
|
||||
message.method === "Page.navigate"
|
||||
? { frameId: "main", loaderId: "main-loader" }
|
||||
? sameDocumentNavigation
|
||||
? { frameId: "main" }
|
||||
: { frameId: "main", loaderId: "main-loader" }
|
||||
: {}
|
||||
);
|
||||
if (message.method === "Page.navigate") {
|
||||
this.navigationCount = navigationNumber;
|
||||
const executionContextId = navigationNumber;
|
||||
if (!sameDocumentNavigation) {
|
||||
this.emit("message", {
|
||||
data: JSON.stringify({
|
||||
method: "Runtime.executionContextCreated",
|
||||
params: {
|
||||
context: {
|
||||
id: executionContextId,
|
||||
auxData: { frameId: "main", isDefault: true },
|
||||
},
|
||||
},
|
||||
}),
|
||||
});
|
||||
}
|
||||
const requestedUrl = String(message.params?.url);
|
||||
this.currentUrl = this.finalUrl ?? requestedUrl;
|
||||
if (this.currentUrl !== requestedUrl) {
|
||||
@@ -411,7 +440,29 @@ class FakeWebSocket {
|
||||
}),
|
||||
});
|
||||
}
|
||||
for (const frameId of this.lifecycleFrames) {
|
||||
const shouldLogError =
|
||||
this.runtimeErrorScenario === "console-error" ||
|
||||
((this.runtimeErrorScenario === "stale-outgoing-error" ||
|
||||
sameDocumentNavigation) &&
|
||||
navigationNumber === 2);
|
||||
if (shouldLogError) {
|
||||
this.emit("message", {
|
||||
data: JSON.stringify({
|
||||
method: "Runtime.consoleAPICalled",
|
||||
params: {
|
||||
type: "error",
|
||||
executionContextId:
|
||||
shouldLogError &&
|
||||
this.runtimeErrorScenario !== "console-error"
|
||||
? 1
|
||||
: executionContextId,
|
||||
},
|
||||
}),
|
||||
});
|
||||
}
|
||||
for (const frameId of sameDocumentNavigation
|
||||
? []
|
||||
: this.lifecycleFrames) {
|
||||
this.emit("message", {
|
||||
data: JSON.stringify({
|
||||
method: "Page.lifecycleEvent",
|
||||
@@ -1920,6 +1971,116 @@ test("returns final URL, redirects, and HTTP error status", async () => {
|
||||
});
|
||||
});
|
||||
|
||||
test("reports React error boundary errors logged by the page", async () => {
|
||||
const layout = await captureBrowserScreenshot(
|
||||
{
|
||||
url: "https://example.com",
|
||||
output: "/tmp/react-error.png",
|
||||
width: 800,
|
||||
height: 600,
|
||||
browserPath: "/usr/bin/chromium",
|
||||
waitUntil: "networkidle",
|
||||
waitForTimeout: 0,
|
||||
timeout: 1000,
|
||||
},
|
||||
createDependencies({
|
||||
socket: new FakeWebSocket(
|
||||
200,
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
["main"],
|
||||
false,
|
||||
"console-error"
|
||||
),
|
||||
})
|
||||
);
|
||||
|
||||
expect(layout.navigation?.javascriptErrorPresent).toBe(true);
|
||||
});
|
||||
|
||||
test("does not attribute an outgoing page error to the next route", async () => {
|
||||
const options = {
|
||||
url: "https://example.com/one",
|
||||
output: "/tmp/one.png",
|
||||
width: 800,
|
||||
height: 600,
|
||||
browserPath: "/usr/bin/chromium",
|
||||
waitUntil: "networkidle" as const,
|
||||
waitForTimeout: 0,
|
||||
timeout: 1000,
|
||||
};
|
||||
const session = await createBrowserScreenshotSession(
|
||||
options,
|
||||
createDependencies({
|
||||
socket: new FakeWebSocket(
|
||||
200,
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
["main"],
|
||||
false,
|
||||
"stale-outgoing-error"
|
||||
),
|
||||
})
|
||||
);
|
||||
|
||||
const layouts = await session.capturePage([
|
||||
options,
|
||||
{
|
||||
...options,
|
||||
url: "https://example.com/two",
|
||||
output: "/tmp/two.png",
|
||||
},
|
||||
]);
|
||||
|
||||
expect(
|
||||
layouts.map(({ navigation }) => navigation?.javascriptErrorPresent)
|
||||
).toEqual([undefined, undefined]);
|
||||
await session.close();
|
||||
});
|
||||
|
||||
test("keeps tracking the document context across hash navigations", async () => {
|
||||
const options = {
|
||||
url: "https://example.com/one",
|
||||
output: "/tmp/one.png",
|
||||
width: 800,
|
||||
height: 600,
|
||||
browserPath: "/usr/bin/chromium",
|
||||
waitUntil: "networkidle" as const,
|
||||
waitForTimeout: 0,
|
||||
timeout: 1000,
|
||||
};
|
||||
const session = await createBrowserScreenshotSession(
|
||||
options,
|
||||
createDependencies({
|
||||
socket: new FakeWebSocket(
|
||||
200,
|
||||
undefined,
|
||||
undefined,
|
||||
undefined,
|
||||
["main"],
|
||||
false,
|
||||
"hash-navigation-error"
|
||||
),
|
||||
})
|
||||
);
|
||||
|
||||
const layouts = await session.capturePage([
|
||||
options,
|
||||
{
|
||||
...options,
|
||||
url: "https://example.com/one#section",
|
||||
output: "/tmp/one-hash.png",
|
||||
},
|
||||
]);
|
||||
|
||||
expect(
|
||||
layouts.map(({ navigation }) => navigation?.javascriptErrorPresent)
|
||||
).toEqual([undefined, true]);
|
||||
await session.close();
|
||||
});
|
||||
|
||||
test("ignores child-frame document responses when reporting navigation", async () => {
|
||||
const layout = await captureBrowserScreenshot(
|
||||
{
|
||||
|
||||
@@ -432,6 +432,14 @@ type NavigationResult = {
|
||||
loaderId?: string;
|
||||
};
|
||||
|
||||
const getExecutionContextId = (params: unknown) =>
|
||||
typeof params === "object" &&
|
||||
params !== null &&
|
||||
"executionContextId" in params &&
|
||||
typeof params.executionContextId === "number"
|
||||
? params.executionContextId
|
||||
: undefined;
|
||||
|
||||
const navigateAndWaitForLifecycle = async (
|
||||
cdp: CdpSession,
|
||||
waitUntil: ScreenshotWaitUntil,
|
||||
@@ -473,7 +481,11 @@ const navigateAndWaitForLifecycle = async (
|
||||
});
|
||||
try {
|
||||
navigation = await navigate();
|
||||
if (navigation.errorText !== undefined || waitUntil === "commit") {
|
||||
if (
|
||||
navigation.errorText !== undefined ||
|
||||
navigation.loaderId === undefined ||
|
||||
waitUntil === "commit"
|
||||
) {
|
||||
return navigation;
|
||||
}
|
||||
if (pendingEvents.some(matchesNavigation)) {
|
||||
@@ -773,6 +785,7 @@ export type BrowserScreenshotNavigation = {
|
||||
mimeType?: string;
|
||||
redirects: string[];
|
||||
documentReadyState: string;
|
||||
javascriptErrorPresent?: boolean;
|
||||
pageMetadata?: BrowserScreenshotPageMetadata;
|
||||
layoutStable: boolean;
|
||||
};
|
||||
@@ -1695,6 +1708,64 @@ const capturePageWithBrowserRuntime = async (
|
||||
mimeType?: string;
|
||||
}
|
||||
>();
|
||||
let javascriptErrorPresent = false;
|
||||
let navigationFrameId: string | undefined;
|
||||
let navigationExecutionContextFrames: Map<number, string> | undefined;
|
||||
let navigationJavascriptErrorContextIds = new Set<number>();
|
||||
let activeExecutionContextIds = new Set<number>();
|
||||
const recordJavascriptError = (params: unknown) => {
|
||||
const executionContextId = getExecutionContextId(params);
|
||||
if (executionContextId === undefined) {
|
||||
return;
|
||||
}
|
||||
if (navigationExecutionContextFrames !== undefined) {
|
||||
navigationJavascriptErrorContextIds.add(executionContextId);
|
||||
} else if (activeExecutionContextIds.has(executionContextId)) {
|
||||
javascriptErrorPresent = true;
|
||||
}
|
||||
};
|
||||
cdp.on("Runtime.executionContextCreated", (params) => {
|
||||
const context = (
|
||||
params as {
|
||||
context?: {
|
||||
id: number;
|
||||
auxData?: { frameId?: string; isDefault?: boolean };
|
||||
};
|
||||
} | null
|
||||
)?.context;
|
||||
if (context?.auxData?.isDefault !== true) {
|
||||
return;
|
||||
}
|
||||
const { id, auxData } = context;
|
||||
const { frameId } = auxData;
|
||||
if (typeof frameId !== "string") {
|
||||
return;
|
||||
}
|
||||
if (navigationExecutionContextFrames !== undefined) {
|
||||
navigationExecutionContextFrames.set(id, frameId);
|
||||
} else if (frameId === navigationFrameId) {
|
||||
activeExecutionContextIds.add(id);
|
||||
}
|
||||
});
|
||||
cdp.on("Runtime.exceptionThrown", (params) => {
|
||||
if (
|
||||
typeof params === "object" &&
|
||||
params !== null &&
|
||||
"exceptionDetails" in params
|
||||
) {
|
||||
recordJavascriptError(params.exceptionDetails);
|
||||
}
|
||||
});
|
||||
cdp.on("Runtime.consoleAPICalled", (params) => {
|
||||
if (
|
||||
typeof params === "object" &&
|
||||
params !== null &&
|
||||
"type" in params &&
|
||||
params.type === "error"
|
||||
) {
|
||||
recordJavascriptError(params);
|
||||
}
|
||||
});
|
||||
cdp.on("Network.requestWillBeSent", (params) => {
|
||||
if (
|
||||
typeof params !== "object" ||
|
||||
@@ -1755,7 +1826,6 @@ const capturePageWithBrowserRuntime = async (
|
||||
send("Page.setLifecycleEventsEnabled", { enabled: true }),
|
||||
]);
|
||||
const targetSetupMs = Date.now() - setupStartedAt;
|
||||
let navigationFrameId: string | undefined;
|
||||
let previousUrl: string | undefined;
|
||||
for (const [index, options] of optionsList.entries()) {
|
||||
const viewportStartedAt = Date.now();
|
||||
@@ -1771,6 +1841,14 @@ const capturePageWithBrowserRuntime = async (
|
||||
);
|
||||
let navigationMs = 0;
|
||||
if (options.url !== previousUrl) {
|
||||
const previousNavigationFrameId = navigationFrameId;
|
||||
const previousExecutionContextIds = new Set(
|
||||
activeExecutionContextIds
|
||||
);
|
||||
javascriptErrorPresent = false;
|
||||
navigationExecutionContextFrames = new Map();
|
||||
navigationJavascriptErrorContextIds.clear();
|
||||
activeExecutionContextIds = new Set();
|
||||
redirectsByFrame.clear();
|
||||
documentResponsesByFrame.clear();
|
||||
const navigationStartedAt = Date.now();
|
||||
@@ -1790,7 +1868,32 @@ const capturePageWithBrowserRuntime = async (
|
||||
`Browser navigation failed: ${navigation.errorText}`
|
||||
);
|
||||
}
|
||||
const sameDocumentNavigation =
|
||||
navigation.loaderId === undefined &&
|
||||
navigation.frameId !== undefined &&
|
||||
navigation.frameId === previousNavigationFrameId;
|
||||
navigationFrameId = navigation.frameId;
|
||||
if (sameDocumentNavigation) {
|
||||
for (const executionContextId of previousExecutionContextIds) {
|
||||
activeExecutionContextIds.add(executionContextId);
|
||||
}
|
||||
}
|
||||
for (const [
|
||||
executionContextId,
|
||||
frameId,
|
||||
] of navigationExecutionContextFrames ?? []) {
|
||||
if (frameId === navigationFrameId) {
|
||||
activeExecutionContextIds.add(executionContextId);
|
||||
}
|
||||
}
|
||||
for (const executionContextId of activeExecutionContextIds) {
|
||||
if (navigationJavascriptErrorContextIds.has(executionContextId)) {
|
||||
javascriptErrorPresent = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
navigationExecutionContextFrames = undefined;
|
||||
navigationJavascriptErrorContextIds.clear();
|
||||
navigationMs = Date.now() - navigationStartedAt;
|
||||
previousUrl = options.url;
|
||||
} else {
|
||||
@@ -2009,6 +2112,7 @@ const capturePageWithBrowserRuntime = async (
|
||||
}),
|
||||
redirects,
|
||||
...readiness,
|
||||
...(javascriptErrorPresent ? { javascriptErrorPresent: true } : {}),
|
||||
};
|
||||
if (images === undefined) {
|
||||
delete layout.images;
|
||||
|
||||
Reference in new issue
Block a user