From e314c9f651eef1ea6fb6ea5e6e4866f9e11ed462 Mon Sep 17 00:00:00 2001 From: sapphi-red Date: Fri, 25 Nov 2022 18:33:19 +0900 Subject: [PATCH 1/3] test: error thrown from module should share instance --- .../node/ssr/__tests__/ssrLoadModule.spec.ts | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/packages/vite/src/node/ssr/__tests__/ssrLoadModule.spec.ts b/packages/vite/src/node/ssr/__tests__/ssrLoadModule.spec.ts index 1df5f2b128da5d..10b6198da96458 100644 --- a/packages/vite/src/node/ssr/__tests__/ssrLoadModule.spec.ts +++ b/packages/vite/src/node/ssr/__tests__/ssrLoadModule.spec.ts @@ -21,3 +21,23 @@ test('ssrLoad', async () => { ) } }) + +test('error has same instance', async () => { + expect.assertions(3) + const s = Symbol() + + const server = await createDevServer() + try { + await server.ssrLoadModule('/fixtures/modules/has-error.js') + } catch (e) { + expect(e[s]).toBeUndefined() + e[s] = true + expect(e[s]).toBe(true) + } + + try { + await server.ssrLoadModule('/fixtures/modules/has-error.js') + } catch (e) { + expect(e[s]).toBe(true) + } +}) From b2e089fc6f30fdb9003a1fe47c6ec6e4e53f10a9 Mon Sep 17 00:00:00 2001 From: sapphi-red Date: Fri, 25 Nov 2022 18:47:18 +0900 Subject: [PATCH 2/3] fix(ssr): skip rewriting stack trace if it's already rewritten --- packages/vite/src/node/server/index.ts | 10 ++------- .../__tests__/fixtures/modules/has-error.js | 1 + .../node/ssr/__tests__/ssrStacktrace.spec.ts | 22 +++++++++++++++++++ packages/vite/src/node/ssr/ssrModuleLoader.ts | 7 +++--- packages/vite/src/node/ssr/ssrStacktrace.ts | 16 ++++++++++++++ 5 files changed, 44 insertions(+), 12 deletions(-) create mode 100644 packages/vite/src/node/ssr/__tests__/fixtures/modules/has-error.js create mode 100644 packages/vite/src/node/ssr/__tests__/ssrStacktrace.spec.ts diff --git a/packages/vite/src/node/server/index.ts b/packages/vite/src/node/server/index.ts index db76f2852678d5..66f1c14044f373 100644 --- a/packages/vite/src/node/server/index.ts +++ b/packages/vite/src/node/server/index.ts @@ -31,10 +31,7 @@ import { } from '../utils' import { ssrLoadModule } from '../ssr/ssrModuleLoader' import { cjsSsrResolveExternals } from '../ssr/ssrExternal' -import { - rebindErrorStacktrace, - ssrRewriteStacktrace -} from '../ssr/ssrStacktrace' +import { ssrFixStacktrace, ssrRewriteStacktrace } from '../ssr/ssrStacktrace' import { ssrTransform } from '../ssr/ssrTransform' import { getDepsOptimizer, @@ -379,10 +376,7 @@ export async function createServer( ) }, ssrFixStacktrace(e) { - if (e.stack) { - const stacktrace = ssrRewriteStacktrace(e.stack, moduleGraph) - rebindErrorStacktrace(e, stacktrace) - } + ssrFixStacktrace(e, moduleGraph) }, ssrRewriteStacktrace(stack: string) { return ssrRewriteStacktrace(stack, moduleGraph) diff --git a/packages/vite/src/node/ssr/__tests__/fixtures/modules/has-error.js b/packages/vite/src/node/ssr/__tests__/fixtures/modules/has-error.js new file mode 100644 index 00000000000000..ef6d5d4df85621 --- /dev/null +++ b/packages/vite/src/node/ssr/__tests__/fixtures/modules/has-error.js @@ -0,0 +1 @@ +throw new Error() diff --git a/packages/vite/src/node/ssr/__tests__/ssrStacktrace.spec.ts b/packages/vite/src/node/ssr/__tests__/ssrStacktrace.spec.ts new file mode 100644 index 00000000000000..c7f89822171794 --- /dev/null +++ b/packages/vite/src/node/ssr/__tests__/ssrStacktrace.spec.ts @@ -0,0 +1,22 @@ +import { fileURLToPath } from 'node:url' +import { test } from 'vitest' +import { createServer } from '../../server' + +const root = fileURLToPath(new URL('./', import.meta.url)) + +async function createDevServer() { + const server = await createServer({ configFile: false, root }) + server.pluginContainer.buildStart({}) + return server +} + +test('call rewriteStacktrace twice', async () => { + const server = await createDevServer() + for (let i = 0; i < 2; i++) { + try { + await server.ssrLoadModule('/fixtures/modules/has-error.js') + } catch (e) { + server.ssrFixStacktrace(e) + } + } +}) diff --git a/packages/vite/src/node/ssr/ssrModuleLoader.ts b/packages/vite/src/node/ssr/ssrModuleLoader.ts index eaae5e73015b46..3fb27a6ff210cf 100644 --- a/packages/vite/src/node/ssr/ssrModuleLoader.ts +++ b/packages/vite/src/node/ssr/ssrModuleLoader.ts @@ -19,7 +19,7 @@ import { ssrImportMetaKey, ssrModuleExportsKey } from './ssrTransform' -import { rebindErrorStacktrace, ssrRewriteStacktrace } from './ssrStacktrace' +import { ssrFixStacktrace } from './ssrStacktrace' interface SSRContext { global: typeof globalThis @@ -210,10 +210,9 @@ async function instantiateModule( } catch (e) { mod.ssrError = e if (e.stack && fixStacktrace) { - const stacktrace = ssrRewriteStacktrace(e.stack, moduleGraph) - rebindErrorStacktrace(e, stacktrace) + ssrFixStacktrace(e, moduleGraph) server.config.logger.error( - `Error when evaluating SSR module ${url}:\n${stacktrace}`, + `Error when evaluating SSR module ${url}:\n${e.stack}`, { timestamp: true, clear: server.config.clearScreen, diff --git a/packages/vite/src/node/ssr/ssrStacktrace.ts b/packages/vite/src/node/ssr/ssrStacktrace.ts index 2d71fada82065f..36f7e9a94ef856 100644 --- a/packages/vite/src/node/ssr/ssrStacktrace.ts +++ b/packages/vite/src/node/ssr/ssrStacktrace.ts @@ -71,3 +71,19 @@ export function rebindErrorStacktrace(e: Error, stacktrace: string): void { e.stack = stacktrace } } + +const rewroteStacktraceSymbol = Symbol('Vite.rewroteStacktrace') + +export function ssrFixStacktrace( + e: Error & { [rewroteStacktraceSymbol]?: true }, + moduleGraph: ModuleGraph +): void { + if (!e.stack) return + // stacktrace shouldn't be rewritten more than once + if (e[rewroteStacktraceSymbol]) return + + const stacktrace = ssrRewriteStacktrace(e.stack, moduleGraph) + rebindErrorStacktrace(e, stacktrace) + + e[rewroteStacktraceSymbol] = true +} From 575d1422fc884c44bc46aa857e1fdc4003f419ef Mon Sep 17 00:00:00 2001 From: sapphi-red Date: Fri, 9 Dec 2022 00:14:27 +0900 Subject: [PATCH 3/3] refactor: use WeakSet --- packages/vite/src/node/ssr/ssrStacktrace.ts | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/packages/vite/src/node/ssr/ssrStacktrace.ts b/packages/vite/src/node/ssr/ssrStacktrace.ts index 36f7e9a94ef856..7fc1082757a4b0 100644 --- a/packages/vite/src/node/ssr/ssrStacktrace.ts +++ b/packages/vite/src/node/ssr/ssrStacktrace.ts @@ -72,18 +72,15 @@ export function rebindErrorStacktrace(e: Error, stacktrace: string): void { } } -const rewroteStacktraceSymbol = Symbol('Vite.rewroteStacktrace') +const rewroteStacktraces = new WeakSet() -export function ssrFixStacktrace( - e: Error & { [rewroteStacktraceSymbol]?: true }, - moduleGraph: ModuleGraph -): void { +export function ssrFixStacktrace(e: Error, moduleGraph: ModuleGraph): void { if (!e.stack) return // stacktrace shouldn't be rewritten more than once - if (e[rewroteStacktraceSymbol]) return + if (rewroteStacktraces.has(e)) return const stacktrace = ssrRewriteStacktrace(e.stack, moduleGraph) rebindErrorStacktrace(e, stacktrace) - e[rewroteStacktraceSymbol] = true + rewroteStacktraces.add(e) }