Skip to content

Commit

Permalink
fix(error-logging): rollup errors weren't displaying id and codeframe
Browse files Browse the repository at this point in the history
The code which modified the error message to have additional context about the module id
and code frame where the error originated was relying on the error's stack having never
been accessed prior to modifying the error's message. At some point, this no longer was
true for Rollup errors, and no context information was being displayed as a result other
than the stack trace within the rollup library that doesn't make it clear what or where
the problem is.

This PR changes the code that add's the id and code-frame context to the error message
to also modify the error's stack. To do so, it attempts to extract the original stack
trace by removing the existing error message from error.stack if it matches, and
otherwise falls back to just displaying the stack trace with its own message since it
varies from the error.message.

Some additional normalization was done to the error.frame field if it exists to ensure
that consistent padding is used in the error output. This is because the rollup code-
frame doesn't have new-lines around it, but the esbuild one does.

Fixes #16539
  • Loading branch information
thebanjomatic committed Apr 29, 2024
1 parent 6c323d5 commit 902b4eb
Showing 1 changed file with 47 additions and 6 deletions.
53 changes: 47 additions & 6 deletions packages/vite/src/node/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -541,23 +541,64 @@ export async function build(
},
}

const mergeRollupError = (e: RollupError) => {
/**
* The stack string usually contains a copy of the message at the start of the stack.
* If the stack starts with the message, we remove it and just return the stack trace
* portion. Otherwise the original stack trace is used.
*/
function extractStack(e: RollupError) {
const { stack, name = 'Error', message } = e

// If we don't have a stack, not much we can do.
if (!stack) {
return stack
}

const expectedPrefix = `${name}: ${message}\n`
if (stack.startsWith(expectedPrefix)) {
return stack.slice(expectedPrefix.length)
}

return stack
}

/**
* Esbuild code frames have newlines at the start and end of the frame, rollup doesn't
* This function normalizes the frame to match the esbuild format which has more pleasing padding
*/
const normalizeCodeFrame = (frame: string) => {
const trimmedPadding = frame.replace(/^\n|\n$/g, '')
return `\n${trimmedPadding}\n`
}

const enhanceRollupError = (e: RollupError) => {
const stackOnly = extractStack(e)

let msg = colors.red((e.plugin ? `[${e.plugin}] ` : '') + e.message)
if (e.id) {
msg += `\nfile: ${colors.cyan(
e.id + (e.loc ? `:${e.loc.line}:${e.loc.column}` : ''),
)}`
}
if (e.frame) {
msg += `\n` + colors.yellow(e.frame)
msg += `\n` + colors.yellow(normalizeCodeFrame(e.frame))
}

e.message = msg

// We are rebuilding the stack trace to include the more detailed message at the top.
// Previously this code was relying on mutating e.message changing the generated stack
// when it was accessed, but we don't have any guarantees that the error we are working
// with hasn't already had its stack accessed before we get here.
if (stackOnly !== undefined) {
e.stack = `${e.message}\n${stackOnly}`
}
return msg
}

const outputBuildError = (e: RollupError) => {
const msg = mergeRollupError(e)
enhanceRollupError(e)
clearLine()
config.logger.error(msg, { error: e })
config.logger.error(e.message, { error: e })
}

let bundle: RollupBuild | undefined
Expand Down Expand Up @@ -711,7 +752,7 @@ export async function build(
)
return Array.isArray(outputs) ? res : res[0]
} catch (e) {
e.message = mergeRollupError(e)
enhanceRollupError(e)
clearLine()
if (startTime) {
config.logger.error(
Expand Down

0 comments on commit 902b4eb

Please sign in to comment.