From dc6ebf82670fc2492d4109caa74af51ed3b1d125 Mon Sep 17 00:00:00 2001 From: Jiachi Liu Date: Mon, 26 Feb 2024 15:47:11 +0100 Subject: [PATCH 1/2] test: also check source code for node:internal related errors --- .../acceptance-app/ReactRefreshLogBox.test.ts | 39 +++++++++++++------ .../acceptance/ReactRefreshLogBox.test.ts | 35 +++++++++++------ 2 files changed, 51 insertions(+), 23 deletions(-) diff --git a/test/development/acceptance-app/ReactRefreshLogBox.test.ts b/test/development/acceptance-app/ReactRefreshLogBox.test.ts index 5a80e3702783d..c96f2fc440c53 100644 --- a/test/development/acceptance-app/ReactRefreshLogBox.test.ts +++ b/test/development/acceptance-app/ReactRefreshLogBox.test.ts @@ -839,17 +839,17 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => { await cleanup() }) - test('useless frames are hidden in stack trace', async () => { + test('should hide unrelated frames in stack trace with unknown anonymous calls', async () => { const { session, browser, cleanup } = await sandbox( next, new Map([ [ 'app/page.js', + // TODO: repro stringify () outdent` export default function Page() { const e = new Error("Boom!"); e.stack += \` - // REVIEW: how to reliably test the presence of these stack frames? at stringify () at () at foo (bar:1:1)\`; @@ -869,23 +869,38 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => { expect(texts).not.toContain('\n') expect(texts).toContain('foo\nbar (1:1)') - // Test that node:internal errors should be hidden + await cleanup() + }) - next.patchFile( - 'app/page.js', - // Node.js will throw an error about the invalid URL since this is a server component - outdent` - export default function Page() { - new URL("/", "invalid"); - }` + test('should hide unrelated frames in stack trace with node:internal calls', async () => { + const { session, browser, cleanup } = await sandbox( + next, + new Map([ + [ + 'app/page.js', + // Node.js will throw an error about the invalid URL since this is a server component + outdent` + export default function Page() { + new URL("/", "invalid"); + }`, + ], + ]) ) + // Test that node:internal errors should be hidden expect(await session.hasRedbox()).toBe(true) await expandCallStack(browser) - callStackFrames = await browser.elementsByCss( + + // Should still show the errored line in source code + const source = await session.getRedboxSource() + expect(source).toContain('app/page.js') + expect(source).toContain(`new URL("/", "invalid")`) + + await expandCallStack(browser) + const callStackFrames = await browser.elementsByCss( '[data-nextjs-call-stack-frame]' ) - texts = await Promise.all(callStackFrames.map((f) => f.innerText())) + const texts = await Promise.all(callStackFrames.map((f) => f.innerText())) expect(texts.filter((t) => t.includes('node:internal'))).toHaveLength(0) diff --git a/test/development/acceptance/ReactRefreshLogBox.test.ts b/test/development/acceptance/ReactRefreshLogBox.test.ts index 3c6234ea43703..db7ce3aa8dacb 100644 --- a/test/development/acceptance/ReactRefreshLogBox.test.ts +++ b/test/development/acceptance/ReactRefreshLogBox.test.ts @@ -784,17 +784,17 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox %s', () => { await cleanup() }) - test('useless frames are hidden in stack trace for pages error', async () => { + test('should hide unrelated frames in stack trace with unknown anonymous calls', async () => { const { session, browser, cleanup } = await sandbox( next, new Map([ [ 'pages/index.js', + // TODO: repro stringify () outdent` export default function Page() { const e = new Error("Client error!"); e.stack += \` - // REVIEW: how to reliably test the presence of these stack frames? at stringify () at () at foo (bar:1:1)\`; @@ -814,26 +814,39 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox %s', () => { expect(texts).not.toContain('\n') expect(texts).toContain('foo\nbar (1:1)') - // Test that node:internal errors should be hidden + await cleanup() + }) - next.patchFile( - 'pages/index.js', - // Node.js will throw an error about the invalid URL since it happens server-side - outdent` + test('should hide unrelated frames in stack trace with node:internal calls', async () => { + const { session, browser, cleanup } = await sandbox( + next, + new Map([ + [ + 'pages/index.js', + // Node.js will throw an error about the invalid URL since it happens server-side + outdent` export default function Page() {} export function getServerSideProps() { new URL("/", "invalid"); return { props: {} }; - }` + }`, + ], + ]) ) - + // Test that node:internal errors should be hidden expect(await session.hasRedbox()).toBe(true) await expandCallStack(browser) - callStackFrames = await browser.elementsByCss( + + // Should still show the errored line in source code + const source = await session.getRedboxSource() + expect(source).toContain('pages/index.js') + expect(source).toContain(`new URL("/", "invalid")`) + + const callStackFrames = await browser.elementsByCss( '[data-nextjs-call-stack-frame]' ) - texts = await Promise.all(callStackFrames.map((f) => f.innerText())) + const texts = await Promise.all(callStackFrames.map((f) => f.innerText())) expect(texts.filter((t) => t.includes('node:internal'))).toHaveLength(0) From 562059ee14fd005ddabbbf3baaa95c1794c850cd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bal=C3=A1zs=20Orb=C3=A1n?= Date: Mon, 26 Feb 2024 15:53:29 +0100 Subject: [PATCH 2/2] Apply suggestions from code review --- test/development/acceptance-app/ReactRefreshLogBox.test.ts | 1 - test/development/acceptance/ReactRefreshLogBox.test.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/test/development/acceptance-app/ReactRefreshLogBox.test.ts b/test/development/acceptance-app/ReactRefreshLogBox.test.ts index c96f2fc440c53..5c16e19c0cd15 100644 --- a/test/development/acceptance-app/ReactRefreshLogBox.test.ts +++ b/test/development/acceptance-app/ReactRefreshLogBox.test.ts @@ -887,7 +887,6 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox app %s', () => { ]) ) - // Test that node:internal errors should be hidden expect(await session.hasRedbox()).toBe(true) await expandCallStack(browser) diff --git a/test/development/acceptance/ReactRefreshLogBox.test.ts b/test/development/acceptance/ReactRefreshLogBox.test.ts index db7ce3aa8dacb..708e7a7969bf0 100644 --- a/test/development/acceptance/ReactRefreshLogBox.test.ts +++ b/test/development/acceptance/ReactRefreshLogBox.test.ts @@ -834,7 +834,6 @@ describe.each(['default', 'turbo'])('ReactRefreshLogBox %s', () => { ], ]) ) - // Test that node:internal errors should be hidden expect(await session.hasRedbox()).toBe(true) await expandCallStack(browser)