Skip to content

Commit

Permalink
fix(ssr): skip rewriting stack trace if it's already rewritten (fixes #…
Browse files Browse the repository at this point in the history
  • Loading branch information
sapphi-red committed Dec 8, 2022
1 parent 9602686 commit feb8ce0
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 12 deletions.
10 changes: 2 additions & 8 deletions packages/vite/src/node/server/index.ts
Expand Up @@ -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,
Expand Down Expand Up @@ -388,10 +385,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)
Expand Down
@@ -0,0 +1 @@
throw new Error()
20 changes: 20 additions & 0 deletions packages/vite/src/node/ssr/__tests__/ssrLoadModule.spec.ts
Expand Up @@ -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)
}
})
22 changes: 22 additions & 0 deletions 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)
}
}
})
7 changes: 3 additions & 4 deletions packages/vite/src/node/ssr/ssrModuleLoader.ts
Expand Up @@ -17,7 +17,7 @@ import {
ssrImportMetaKey,
ssrModuleExportsKey,
} from './ssrTransform'
import { rebindErrorStacktrace, ssrRewriteStacktrace } from './ssrStacktrace'
import { ssrFixStacktrace } from './ssrStacktrace'

interface SSRContext {
global: typeof globalThis
Expand Down Expand Up @@ -203,10 +203,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,
Expand Down
13 changes: 13 additions & 0 deletions packages/vite/src/node/ssr/ssrStacktrace.ts
Expand Up @@ -71,3 +71,16 @@ export function rebindErrorStacktrace(e: Error, stacktrace: string): void {
e.stack = stacktrace
}
}

const rewroteStacktraces = new WeakSet()

export function ssrFixStacktrace(e: Error, moduleGraph: ModuleGraph): void {
if (!e.stack) return
// stacktrace shouldn't be rewritten more than once
if (rewroteStacktraces.has(e)) return

const stacktrace = ssrRewriteStacktrace(e.stack, moduleGraph)
rebindErrorStacktrace(e, stacktrace)

rewroteStacktraces.add(e)
}

0 comments on commit feb8ce0

Please sign in to comment.