From 63f4441a8c229d4ee5543a348bf2896ff833a3cf Mon Sep 17 00:00:00 2001 From: Divanshu Chauhan Date: Tue, 28 Apr 2026 19:33:57 -0700 Subject: [PATCH 1/2] fix: add tests verifying strings thrown in Server Components are not swallowed (#810) Next.js had a bug where createReactServerErrorHandler returned early for typeof thrownValue === 'string', swallowing the error before logging. vinext's rscOnError already handles strings correctly (they flow through the normal error path and get wrapped into Error objects). Added: - Fixture page that throws a string in a Server Component - Integration test verifying the error boundary renders with the string message - Runtime test confirming rscOnError returns a valid digest for string throws - Static test confirming generated code has no early string-return path Upstream: vercel/next.js@b9ca95c62dd882eb3c0e5fc0be9b20bf0b198fb1 --- tests/app-router.test.ts | 41 ++++++++++++++++++- .../app-basic/app/throw-string-test/error.tsx | 19 +++++++++ .../app-basic/app/throw-string-test/page.tsx | 4 ++ 3 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 tests/fixtures/app-basic/app/throw-string-test/error.tsx create mode 100644 tests/fixtures/app-basic/app/throw-string-test/page.tsx diff --git a/tests/app-router.test.ts b/tests/app-router.test.ts index b8b204a8f..7fe2a908f 100644 --- a/tests/app-router.test.ts +++ b/tests/app-router.test.ts @@ -815,6 +815,13 @@ describe("App Router integration", () => { expect(html).not.toContain("Missing use client react hook test"); }); + it("error boundary catches string thrown in Server Component", async () => { + const { res, html } = await fetchHtml(baseUrl, "/throw-string-test"); + expect(res.status).toBe(200); + expect(html).toContain("this is a test string thrown in a server component"); + expect(html).toContain('data-testid="string-error-boundary"'); + }); + it("redirect() from Server Component returns redirect response", async () => { const res = await fetch(`${baseUrl}/redirect-test`, { redirect: "manual" }); expect(res.status).toBeGreaterThanOrEqual(300); @@ -3642,6 +3649,8 @@ describe("App Router next.config.js features (generateRscEntry)", () => { // wrong return values, broken control flow). describe("runtime behavior", () => { let rscOnError: (error: unknown) => string | undefined; + let digestFn: string; + let onErrorFn: string; beforeAll(() => { const code = generateRscEntry("/tmp/test/app", minimalRoutes, null, [], null, "", false); @@ -3662,8 +3671,8 @@ describe("App Router next.config.js features (generateRscEntry)", () => { throw new Error(`Unbalanced braces in ${name}`); } - const digestFn = extractFunction(code, "__errorDigest"); - const onErrorFn = extractFunction(code, "rscOnError"); + digestFn = extractFunction(code, "__errorDigest"); + onErrorFn = extractFunction(code, "rscOnError"); const body = `${digestFn}\n${onErrorFn}\nreturn rscOnError;`; // oxlint-disable-next-line no-new-func, @typescript-eslint/no-implied-eval -- reconstructing emitted runtime code is the behavior under test @@ -3705,6 +3714,34 @@ describe("App Router next.config.js features (generateRscEntry)", () => { spy.mockRestore(); } }); + + it("does not swallow strings thrown in Server Components", () => { + // oxlint-disable-next-line no-new-func, @typescript-eslint/no-implied-eval -- reconstructing emitted runtime code is the behavior under test + const digestFactory = new Function( + "process", + `${digestFn}\n${onErrorFn}\nreturn rscOnError;`, + ); + const prodRscOnError = digestFactory({ env: { NODE_ENV: "production" } }); + const result = prodRscOnError("this is a test string"); + // Should return a digest hash (not undefined), indicating the string + // was processed through the normal error path + expect(result).toBeDefined(); + expect(typeof result).toBe("string"); + expect((result as string).length).toBeGreaterThan(0); + // The digest should be based on the string content + const result2 = prodRscOnError("different string"); + expect(result2).not.toBe(result); + }); + + it("does not have an early return for string thrown values in generated code", () => { + // Next.js had a bug where typeof thrownValue === 'string' returned + // early, swallowing the error before logging. Verify the generated + // rscOnError has no such early-return path. + expect(onErrorFn).not.toContain("typeof thrownValue === 'string'"); + // Should still handle strings for digest generation + expect(onErrorFn).toContain("error instanceof Error"); + expect(onErrorFn).toContain("String(error)"); + }); }); }); diff --git a/tests/fixtures/app-basic/app/throw-string-test/error.tsx b/tests/fixtures/app-basic/app/throw-string-test/error.tsx new file mode 100644 index 000000000..d6016db0c --- /dev/null +++ b/tests/fixtures/app-basic/app/throw-string-test/error.tsx @@ -0,0 +1,19 @@ +"use client"; + +export default function ThrowStringErrorBoundary({ + error, + reset, +}: { + error: Error; + reset: () => void; +}) { + return ( +
+

String Error Caught

+

{error.message}

+ +
+ ); +} diff --git a/tests/fixtures/app-basic/app/throw-string-test/page.tsx b/tests/fixtures/app-basic/app/throw-string-test/page.tsx new file mode 100644 index 000000000..609abede3 --- /dev/null +++ b/tests/fixtures/app-basic/app/throw-string-test/page.tsx @@ -0,0 +1,4 @@ +export default function ThrowStringTestPage() { + throw "this is a test string thrown in a server component"; + return
This should never render
; +} From 295a49089fb420a74c780db9ab7ed4c51b353b69 Mon Sep 17 00:00:00 2001 From: Divanshu Chauhan Date: Wed, 29 Apr 2026 12:07:12 -0700 Subject: [PATCH 2/2] fix: address review feedback on string error handling tests (#950) - Move prodRscOnError creation into beforeAll to avoid duplicating new Function construction - Remove brittle positive assertions (instanceof Error, String(error)) from static test; keep only the negative anti-pattern guard - Use textContentByTestId helper for integration test precision over loose html.toContain --- tests/app-router.test.ts | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/tests/app-router.test.ts b/tests/app-router.test.ts index 7fe2a908f..44addf414 100644 --- a/tests/app-router.test.ts +++ b/tests/app-router.test.ts @@ -818,8 +818,9 @@ describe("App Router integration", () => { it("error boundary catches string thrown in Server Component", async () => { const { res, html } = await fetchHtml(baseUrl, "/throw-string-test"); expect(res.status).toBe(200); - expect(html).toContain("this is a test string thrown in a server component"); - expect(html).toContain('data-testid="string-error-boundary"'); + expect(textContentByTestId(html, "string-error-message")).toBe( + "this is a test string thrown in a server component", + ); }); it("redirect() from Server Component returns redirect response", async () => { @@ -3649,6 +3650,7 @@ describe("App Router next.config.js features (generateRscEntry)", () => { // wrong return values, broken control flow). describe("runtime behavior", () => { let rscOnError: (error: unknown) => string | undefined; + let prodRscOnError: (error: unknown) => string | undefined; let digestFn: string; let onErrorFn: string; @@ -3678,6 +3680,7 @@ describe("App Router next.config.js features (generateRscEntry)", () => { // oxlint-disable-next-line no-new-func, @typescript-eslint/no-implied-eval -- reconstructing emitted runtime code is the behavior under test const factory = new Function("process", body); rscOnError = factory({ env: { NODE_ENV: "development" } }); + prodRscOnError = factory({ env: { NODE_ENV: "production" } }); }); it("returns the digest string for navigation errors (redirect/notFound)", () => { @@ -3716,12 +3719,6 @@ describe("App Router next.config.js features (generateRscEntry)", () => { }); it("does not swallow strings thrown in Server Components", () => { - // oxlint-disable-next-line no-new-func, @typescript-eslint/no-implied-eval -- reconstructing emitted runtime code is the behavior under test - const digestFactory = new Function( - "process", - `${digestFn}\n${onErrorFn}\nreturn rscOnError;`, - ); - const prodRscOnError = digestFactory({ env: { NODE_ENV: "production" } }); const result = prodRscOnError("this is a test string"); // Should return a digest hash (not undefined), indicating the string // was processed through the normal error path @@ -3738,9 +3735,6 @@ describe("App Router next.config.js features (generateRscEntry)", () => { // early, swallowing the error before logging. Verify the generated // rscOnError has no such early-return path. expect(onErrorFn).not.toContain("typeof thrownValue === 'string'"); - // Should still handle strings for digest generation - expect(onErrorFn).toContain("error instanceof Error"); - expect(onErrorFn).toContain("String(error)"); }); }); });