Skip to content

Commit

Permalink
fix #780: add origin information to errors from plugins
Browse files Browse the repository at this point in the history
  • Loading branch information
@evanw
evanw committed Feb 11, 2021
1 parent a846a60 commit d563697
Show file tree
Hide file tree
Showing 5 changed files with 147 additions and 71 deletions.
20 changes: 20 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,26 @@

With this release, notes are also supported in the JS and Go APIs. This means you can now generate your own notes using plugins as well as inspect the notes generated by esbuild.

* Add origin information to errors from plugins ([#780](https://github.com/evanw/esbuild/issues/780))

Errors thrown during JavaScript plugin callback evaluation will now be annoated to show where that plugin callback was registered. That looks like this:

```
> example-plugin.js: error: [example-plugin] foo.bar is not a function
15 │ foo.bar();
╵ ^
at ./example-plugin.js:15:13
at ./node_modules/esbuild/lib/main.js:750:34
example-plugin.js: note: This error came from the "onLoad" callback registered here
13 │ build.onLoad({ filter: /.*/ }, args => {
╵ ~~~~~~
at setup (./example-plugin.js:13:13)
at handlePlugins (./node_modules/esbuild/lib/main.js:668:7)
```

This should make it easier to debug crashes in plugin code.

## 0.8.43

* Support the `XDG_CACHE_HOME` environment variable ([#757](https://github.com/evanw/esbuild/issues/757))
Expand Down
34 changes: 21 additions & 13 deletions internal/bundler/bundler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,17 @@ func extractSourceMapFromComment(
return logger.Path{}, nil
}

func sanetizeLocation(res resolver.Resolver, loc *logger.MsgLocation) {
if loc != nil {
if loc.Namespace == "" {
loc.Namespace = "file"
}
if loc.File != "" {
loc.File = res.PrettyPath(logger.Path{Text: loc.File, Namespace: loc.Namespace})
}
}
}

func logPluginMessages(
res resolver.Resolver,
log logger.Log,
Expand All @@ -571,20 +582,17 @@ func logPluginMessages(
didLogError = true
}

// Sanitize the location
if msg.Data.Location != nil {
clone := *msg.Data.Location
if clone.Namespace == "" {
clone.Namespace = "file"
}
if clone.File != "" {
clone.File = res.PrettyPath(logger.Path{Text: clone.File, Namespace: clone.Namespace})
} else if importSource != nil {
clone.File = importSource.PrettyPath
}
msg.Data.Location = &clone
} else {
// Sanitize the locations
if msg.Data.Location == nil {
msg.Data.Location = logger.LocationOrNil(importSource, importPathRange)
} else {
sanetizeLocation(res, msg.Data.Location)
if msg.Data.Location.File == "" && importSource != nil {
msg.Data.Location.File = importSource.PrettyPath
}
}
for _, note := range msg.Notes {
sanetizeLocation(res, note.Location)
}

log.AddMsg(msg)
Expand Down
36 changes: 30 additions & 6 deletions internal/logger/logger.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
"strings"
"sync"
"time"
"unicode/utf8"
)

const defaultTerminalWidth = 80
Expand Down Expand Up @@ -726,9 +727,18 @@ func (msg Msg) String(options OutputOptions, terminalInfo TerminalInfo) string {
}
}

// Format the messages
// Format the message
text := msgString(options, terminalInfo, msg.Kind, msg.Data, maxMargin)

// Put a blank line between the message and the notes if the message has a stack trace
gap := ""
if loc := msg.Data.Location; loc != nil && strings.ContainsRune(loc.LineText, '\n') {
gap = "\n"
}

// Format the notes
for _, note := range msg.Notes {
text += gap
text += msgString(options, terminalInfo, Note, note, maxMargin)
}

Expand Down Expand Up @@ -926,10 +936,11 @@ func detailStruct(data MsgData, terminalInfo TerminalInfo, maxMargin int) MsgDet

spacesPerTab := 2
lineText := renderTabStops(firstLine, spacesPerTab)
indent := strings.Repeat(" ", len(renderTabStops(firstLine[:loc.Column], spacesPerTab)))
textUpToLoc := renderTabStops(firstLine[:loc.Column], spacesPerTab)
markerStart := len(textUpToLoc)
markerEnd := markerStart
indent := strings.Repeat(" ", estimateWidthInTerminal(textUpToLoc))
marker := "^"
markerStart := len(indent)
markerEnd := len(indent)

// Extend markers to cover the full range of the error
if loc.Length > 0 {
Expand Down Expand Up @@ -1005,13 +1016,13 @@ func detailStruct(data MsgData, terminalInfo TerminalInfo, maxMargin int) MsgDet
}

// Now we can compute the indent
indent = strings.Repeat(" ", markerStart)
lineText = slicedLine
indent = strings.Repeat(" ", estimateWidthInTerminal(lineText[:markerStart]))
}

// If marker is still multi-character after clipping, make the marker wider
if markerEnd-markerStart > 1 {
marker = strings.Repeat("~", markerEnd-markerStart)
marker = strings.Repeat("~", estimateWidthInTerminal(lineText[markerStart:markerEnd]))
}

// Put a margin before the marker indent
Expand All @@ -1034,6 +1045,19 @@ func detailStruct(data MsgData, terminalInfo TerminalInfo, maxMargin int) MsgDet
}
}

// Estimate the number of columns this string will take when printed
func estimateWidthInTerminal(text string) int {
// For now just assume each code point is one column. This is wrong but is
// less wrong than assuming each code unit is one column.
width := 0
for text != "" {
_, size := utf8.DecodeRuneInString(text)
text = text[size:]
width++
}
return width
}

func renderTabStops(withTabs string, spacesPerTab int) string {
if !strings.ContainsRune(withTabs, '\t') {
return withTabs
Expand Down
120 changes: 72 additions & 48 deletions lib/common.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ export function createChannel(streamIn: StreamIn): StreamOut {
throw new Error(`Invalid command: ` + (request as any)!.command);
}
} catch (e) {
sendResponse(id, { errors: [extractErrorMessageV8(e, streamIn, null)] } as any);
sendResponse(id, { errors: [extractErrorMessageV8(e, streamIn, null, void 0)] } as any);
}
};

Expand Down Expand Up @@ -557,13 +557,15 @@ export function createChannel(streamIn: StreamIn): StreamOut {
let onResolveCallbacks: {
[id: number]: {
name: string,
note: types.Note | undefined,
callback: (args: types.OnResolveArgs) =>
(types.OnResolveResult | null | undefined | Promise<types.OnResolveResult | null | undefined>),
},
} = {};
let onLoadCallbacks: {
[id: number]: {
name: string,
note: types.Note | undefined,
callback: (args: types.OnLoadArgs) =>
(types.OnLoadResult | null | undefined | Promise<types.OnLoadResult | null | undefined>),
},
Expand Down Expand Up @@ -592,24 +594,28 @@ export function createChannel(streamIn: StreamIn): StreamOut {

setup({
onResolve(options, callback) {
let registeredText = `This error came from the "onResolve" callback registered here`
let registeredNote = extractCallerV8(new Error(registeredText), streamIn, 'onResolve');
let keys: OptionKeys = {};
let filter = getFlag(options, keys, 'filter', mustBeRegExp);
let namespace = getFlag(options, keys, 'namespace', mustBeString);
checkForInvalidFlags(options, keys, `in onResolve() call for plugin ${JSON.stringify(name)}`);
if (filter == null) throw new Error(`[${plugin.name}] onResolve() call is missing a filter`);
let id = nextCallbackID++;
onResolveCallbacks[id] = { name: name!, callback };
onResolveCallbacks[id] = { name: name!, callback, note: registeredNote };
plugin.onResolve.push({ id, filter: filter.source, namespace: namespace || '' });
},

onLoad(options, callback) {
let registeredText = `This error came from the "onLoad" callback registered here`
let registeredNote = extractCallerV8(new Error(registeredText), streamIn, 'onLoad');
let keys: OptionKeys = {};
let filter = getFlag(options, keys, 'filter', mustBeRegExp);
let namespace = getFlag(options, keys, 'namespace', mustBeString);
checkForInvalidFlags(options, keys, `in onLoad() call for plugin ${JSON.stringify(name)}`);
if (filter == null) throw new Error(`[${plugin.name}] onLoad() call is missing a filter`);
let id = nextCallbackID++;
onLoadCallbacks[id] = { name: name!, callback };
onLoadCallbacks[id] = { name: name!, callback, note: registeredNote };
plugin.onLoad.push({ id, filter: filter.source, namespace: namespace || '' });
},
});
Expand All @@ -620,10 +626,10 @@ export function createChannel(streamIn: StreamIn): StreamOut {
const callback: PluginCallback = async (request) => {
switch (request.command) {
case 'resolve': {
let response: protocol.OnResolveResponse = {};
let response: protocol.OnResolveResponse = {}, name, callback, note;
for (let id of request.ids) {
try {
let { name, callback } = onResolveCallbacks[id];
({ name, callback, note } = onResolveCallbacks[id]);
let result = await callback({
path: request.path,
importer: request.importer,
Expand Down Expand Up @@ -655,17 +661,17 @@ export function createChannel(streamIn: StreamIn): StreamOut {
break;
}
} catch (e) {
return { id, errors: [extractErrorMessageV8(e, streamIn, stash)] };
return { id, errors: [extractErrorMessageV8(e, streamIn, stash, note)] };
}
}
return response;
}

case 'load': {
let response: protocol.OnLoadResponse = {};
let response: protocol.OnLoadResponse = {}, name, callback, note;
for (let id of request.ids) {
try {
let { name, callback } = onLoadCallbacks[id];
({ name, callback, note } = onLoadCallbacks[id]);
let result = await callback({
path: request.path,
namespace: request.namespace,
Expand Down Expand Up @@ -696,7 +702,7 @@ export function createChannel(streamIn: StreamIn): StreamOut {
break;
}
} catch (e) {
return { id, errors: [extractErrorMessageV8(e, streamIn, stash)] };
return { id, errors: [extractErrorMessageV8(e, streamIn, stash, note)] };
}
}
return response;
Expand Down Expand Up @@ -911,7 +917,7 @@ export function createChannel(streamIn: StreamIn): StreamOut {
} catch (e) {
let flags: string[] = [];
try { pushLogFlags(flags, options, {}, isTTY, logLevelDefault) } catch { }
const error = extractErrorMessageV8(e, streamIn, details)
const error = extractErrorMessageV8(e, streamIn, details, void 0)
sendRequest(refs, { command: 'error', flags, error }, () => {
error.detail = details.load(error.detail);
callback(failureErrorWithLog('Build failed', [error], []), null);
Expand Down Expand Up @@ -986,7 +992,7 @@ export function createChannel(streamIn: StreamIn): StreamOut {
} catch (e) {
let flags: string[] = [];
try { pushLogFlags(flags, options, {}, isTTY, logLevelDefault) } catch { }
const error = extractErrorMessageV8(e, streamIn, details);
const error = extractErrorMessageV8(e, streamIn, details, void 0);
sendRequest(refs, { command: 'error', flags, error }, () => {
error.detail = details.load(error.detail);
callback(failureErrorWithLog('Transform failed', [error], []), null);
Expand Down Expand Up @@ -1029,7 +1035,19 @@ function createObjectStash(): ObjectStash {
};
}

function extractErrorMessageV8(e: any, streamIn: StreamIn, stash: ObjectStash | null): types.Message {
function extractCallerV8(e: Error, streamIn: StreamIn, ident: string): types.Note | undefined {
try {
let lines = (e.stack + '').split('\n', 4)
lines.splice(1, 1)
let location = parseStackLinesV8(streamIn, lines, ident)
if (location) {
return { text: e.message, location }
}
} catch {
}
}

function extractErrorMessageV8(e: any, streamIn: StreamIn, stash: ObjectStash | null, note: types.Note | undefined): types.Message {
let text = 'Internal error'
let location: types.Location | null = null

Expand All @@ -1040,49 +1058,55 @@ function extractErrorMessageV8(e: any, streamIn: StreamIn, stash: ObjectStash |

// Optionally attempt to extract the file from the stack trace, works in V8/node
try {
let stack = e.stack + ''
let lines = stack.split('\n', 3)
let at = ' at '

// Check to see if this looks like a V8 stack trace
if (streamIn.readFileSync && !lines[0].startsWith(at) && lines[1].startsWith(at)) {
let line = lines[1].slice(at.length)
while (true) {
// Unwrap a function name
let match = /^\S+ \((.*)\)$/.exec(line)
if (match) {
line = match[1]
continue
}
location = parseStackLinesV8(streamIn, (e.stack + '').split('\n', 3), '')
} catch {
}

// Unwrap an eval wrapper
match = /^eval at \S+ \((.*)\)(?:, \S+:\d+:\d+)?$/.exec(line)
if (match) {
line = match[1]
continue
}
return { text, location, notes: note ? [note] : [], detail: stash ? stash.store(e) : -1 }
}

// Match on the file location
match = /^(\S+):(\d+):(\d+)$/.exec(line)
if (match) {
let contents = streamIn.readFileSync(match[1], 'utf8')
let lineText = contents.split(/\r\n|\r|\n|\u2028|\u2029/)[+match[2] - 1] || ''
location = {
file: match[1],
namespace: 'file',
line: +match[2],
column: +match[3] - 1,
length: 0,
lineText: lineText + '\n' + lines.slice(1).join('\n'),
}
function parseStackLinesV8(streamIn: StreamIn, lines: string[], ident: string): types.Location | null {
let at = ' at '

// Check to see if this looks like a V8 stack trace
if (streamIn.readFileSync && !lines[0].startsWith(at) && lines[1].startsWith(at)) {
let line = lines[1].slice(at.length)
while (true) {
// Unwrap a function name
let match = /^\S+ \((.*)\)$/.exec(line)
if (match) {
line = match[1]
continue
}

// Unwrap an eval wrapper
match = /^eval at \S+ \((.*)\)(?:, \S+:\d+:\d+)?$/.exec(line)
if (match) {
line = match[1]
continue
}

// Match on the file location
match = /^(\S+):(\d+):(\d+)$/.exec(line)
if (match) {
let contents = streamIn.readFileSync(match[1], 'utf8')
let lineText = contents.split(/\r\n|\r|\n|\u2028|\u2029/)[+match[2] - 1] || ''
let column = +match[3] - 1
let length = lineText.slice(column, column + ident.length) === ident ? ident.length : 0
return {
file: match[1],
namespace: 'file',
line: +match[2],
column: protocol.encodeUTF8(lineText.slice(0, column)).length,
length: protocol.encodeUTF8(lineText.slice(column, column + length)).length,
lineText: lineText + '\n' + lines.slice(1).join('\n'),
}
break
}
break
}
} catch {
}

return { text, location, notes: [], detail: stash ? stash.store(e) : -1 }
return null;
}

function failureErrorWithLog(text: string, errors: types.Message[], warnings: types.Message[]): types.BuildFailure {
Expand Down
Loading

0 comments on commit d563697

Please sign in to comment.