Skip to content

Commit e818202

Browse files
lyzno1james-elicx
andauthored
fix: preserve console output for caught app errors in dev (#862)
* fix: preserve console output for caught app errors in dev * address bonk review comments on PR 862 --------- Co-authored-by: James <james@eli.cx>
1 parent 8f22405 commit e818202

5 files changed

Lines changed: 94 additions & 4 deletions

File tree

‎packages/vinext/src/check.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,11 @@ const CONFIG_SUPPORT: Record<string, { status: Status; detail?: string }> = {
168168
status: "partial",
169169
detail: "supported for Pages Router; App Router unchanged",
170170
},
171-
reactStrictMode: { status: "supported", detail: "always enabled" },
171+
reactStrictMode: {
172+
status: "partial",
173+
detail:
174+
"config option recognized but not yet enforced; root is not wrapped in <React.StrictMode>",
175+
},
172176
poweredByHeader: {
173177
status: "supported",
174178
detail: "not sent (matching Next.js default when disabled)",

‎packages/vinext/src/server/app-browser-entry.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ import {
7575
type AppRouterState,
7676
} from "./app-browser-state.js";
7777
import { ElementsContext, Slot } from "../shims/slot.js";
78+
import { devOnCaughtError } from "./app-browser-error.js";
7879

7980
type SearchParamInput = ConstructorParameters<typeof URLSearchParams>[0];
8081

@@ -872,7 +873,7 @@ async function main(): Promise<void> {
872873
initialElements: root,
873874
initialNavigationSnapshot,
874875
}),
875-
import.meta.env.DEV ? { onCaughtError() {} } : undefined,
876+
import.meta.env.DEV ? { onCaughtError: devOnCaughtError } : undefined,
876877
);
877878
window.__VINEXT_HYDRATED_AT = performance.now();
878879

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
// Preserve console visibility for errors caught during hydration in dev
2+
// without re-dispatching them through Vite's overlay path.
3+
//
4+
// Note: sentinel errors (NEXT_NOT_FOUND, NEXT_REDIRECT, etc.) are re-thrown
5+
// in getDerivedStateFromError before they reach onCaughtError, so they will
6+
// not appear here in practice.
7+
export function devOnCaughtError(
8+
error: unknown,
9+
errorInfo: { componentStack?: string; errorBoundary?: unknown },
10+
): void {
11+
console.error(error);
12+
if (errorInfo?.componentStack) {
13+
console.error("The above error occurred in a React component:\n" + errorInfo.componentStack);
14+
}
15+
}

‎tests/app-browser-entry.test.ts‎

Lines changed: 68 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import React from "react";
2-
import { describe, expect, it } from "vite-plus/test";
2+
import { describe, expect, it, vi } from "vite-plus/test";
3+
import { devOnCaughtError } from "../packages/vinext/src/server/app-browser-error.js";
34
import {
45
APP_INTERCEPTION_CONTEXT_KEY,
56
APP_ROOT_LAYOUT_KEY,
@@ -444,6 +445,72 @@ describe("app browser entry previousNextUrl helpers", () => {
444445
});
445446
});
446447

448+
describe("devOnCaughtError (hydrateRoot dev handler)", () => {
449+
it("logs caught errors to console.error", () => {
450+
const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {});
451+
try {
452+
const err = new Error("Maximum update depth exceeded");
453+
devOnCaughtError(err, { componentStack: "\n at List\n at Apps" });
454+
expect(consoleSpy).toHaveBeenCalled();
455+
const loggedErrors = consoleSpy.mock.calls.map((args) => args[0]);
456+
expect(loggedErrors).toContain(err);
457+
} finally {
458+
consoleSpy.mockRestore();
459+
}
460+
});
461+
462+
it("includes the React component stack in the log when provided", () => {
463+
const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {});
464+
try {
465+
devOnCaughtError(new Error("boom"), {
466+
componentStack: "\n at List (apps/list.tsx:202)",
467+
});
468+
expect(consoleSpy).toHaveBeenCalledTimes(2);
469+
expect(String(consoleSpy.mock.calls[1][0])).toContain("apps/list.tsx:202");
470+
} finally {
471+
consoleSpy.mockRestore();
472+
}
473+
});
474+
475+
it("does not re-dispatch a window 'error' event (would trigger Vite overlay)", () => {
476+
// This test runs in a Node environment where `window` is undefined, so the
477+
// listener registration is skipped and windowErrorCount stays 0 trivially.
478+
// The test still documents the contract: devOnCaughtError must not dispatch
479+
// window error events (which would re-trigger the Vite overlay). If a DOM
480+
// environment is ever added to this project, this will become a live check.
481+
const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {});
482+
let windowErrorCount = 0;
483+
const onError = (): void => {
484+
windowErrorCount += 1;
485+
};
486+
if (typeof window !== "undefined") {
487+
window.addEventListener("error", onError);
488+
}
489+
try {
490+
devOnCaughtError(new Error("caught by user error.tsx"), {});
491+
expect(windowErrorCount).toBe(0);
492+
} finally {
493+
if (typeof window !== "undefined") {
494+
window.removeEventListener("error", onError);
495+
}
496+
consoleSpy.mockRestore();
497+
}
498+
});
499+
500+
it("is not a no-op (regression guard against `() => {}`)", () => {
501+
// Explicit regression guard: the original implementation was `() => {}`,
502+
// which silently swallowed all caught errors. This test ensures the handler
503+
// always calls console.error at least once.
504+
const consoleSpy = vi.spyOn(console, "error").mockImplementation(() => {});
505+
try {
506+
devOnCaughtError(new Error("regression"), {});
507+
expect(consoleSpy.mock.calls.length).toBeGreaterThan(0);
508+
} finally {
509+
consoleSpy.mockRestore();
510+
}
511+
});
512+
});
513+
447514
describe("mounted slot helpers", () => {
448515
it("collects only mounted slot ids", () => {
449516
const elements: AppElements = createResolvedElements("route:/dashboard", "/", null, {

‎tests/check.test.ts‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,10 @@ describe("analyzeConfig", () => {
289289
const items = analyzeConfig(tmpDir);
290290
expect(items.find((i) => i.name === "basePath")?.status).toBe("supported");
291291
expect(items.find((i) => i.name === "trailingSlash")?.status).toBe("supported");
292-
expect(items.find((i) => i.name === "reactStrictMode")?.status).toBe("supported");
292+
// reactStrictMode is reported as `partial` until vinext actually wraps the
293+
// hydrated root in `<React.StrictMode>` — currently the config is read but
294+
// not enforced. See `packages/vinext/src/check.ts` for the rationale.
295+
expect(items.find((i) => i.name === "reactStrictMode")?.status).toBe("partial");
293296
});
294297

295298
it("detects unsupported webpack config", () => {

0 commit comments

Comments
 (0)