Repository navigation
Conversation
The preview is Bun's pin 4d45755fed plus the two commits of that pull request. A later importer of a module whose fetch failed gets the error of that fetch, not a copy with the message alone. A rejected fetch of a load by name registers nothing. This commit lets CI run the rest of the branch. Replace the tag with the merged sha before this lands.
…a file import(), require() and a static import of a file that Bun's parser refuses failed with a BuildMessage, or with an AggregateError of them for several diagnostics. A BuildMessage is not an Error: it has no stack, Error.isError() is false, and structuredClone() throws. process_fetch_log now makes one SyntaxError when the first error of the log is a Build diagnostic with a line. The error has own 1-based line and column, sourceURL, and a stack of one frame that names the file. It keeps the wrappers of up to 256 diagnostics under a private name. position, notes and level read the BuildMessage of the first error. The console, the uncaught printer, the test runner and the junit reporter print every diagnostic from the wrappers, so the code frames that they print stay as they were. A diagnostic without a line, a file that is nested too deeply for the parser, a file that cannot be read and a resolve failure still give a BuildMessage, a ResolveMessage or an AggregateError. The Worker error hook has no special case for this value. node:repl prints the location above the message, as Node does. A macro promise that rejects is marked as handled, because the macro runner reports it. Co-authored-by: Dylan Conway <dylan.conway567@gmail.com>
|
Status: draft. It waits for a maintainer's decision on the break, and for oven-sh/WebKit#451. Reproduction on main (b1305dd): echo 'const x;' > bad.js
bun -e 'import("./bad.js").catch(e => console.log(e.constructor.name, Error.isError(e), typeof e.stack))'
For CI (build 138177, a04a8ab): the 18 test files that this PR adds or changes pass on every lane. The red tests are not from this diff:
PR: #48069 |
|
@dylan-conway @Jarred-Sumner this draft needs a yes or a no before more work goes into it. The question: when Bun's parser refuses a file, can
The order of the four pull requests that this needs is in the Notes. oven-sh/WebKit#451 needs a human review first. |
|
Each multi-line comment that this PR added under |
Part of #7531
Draft: needs a maintainer's yes on the break in Downsides, and oven-sh/WebKit#451.
Problem
import()andrequire()of a file that Bun's parser refuses fail with aBuildMessage. For several diagnostics they fail with anAggregateError. Node gives oneSyntaxError.BuildMessageis not an Error: nostack,Error.isErroris false,structuredClonethrows.process_fetch_log(src/jsc/VirtualMachine.rs:3959) makes it.Fix
process_fetch_logmakes oneSyntaxErrorwhen the first error has a line. It has 1-basedlineandcolumn,sourceURL, and astackthat names the file.position,notesandlevelread the first. The console,bun testand the uncaught printer print the code frames as before.test/js/bun/resolve/build-error.test.tsand the suites in Notes.Background
BuildMessagewraps one parser diagnostic. It is no JavaScriptCoreErrorInstance.stackgetter onBuildMessage: still a non-Error. Considered a Bun-side refetch: later importers still get a copy.Downsides
name,constructor,instanceof BuildMessage,errors,toJSON()and the 0-basedlineandcolumnchange. Notes has the table.require()runs 65,141 instructions, was 61,043. A printed error runs 112 more (0.35%).AggregateErrorprints nothing.Notes
Order
BuildMessageandResolveMessage. It needs no WebKit change. The twopromise.set_handledlines inMacro.rshere are its lines. It is rebased onto main.evictModuleThatNothingFetchesinBunPlugin.cppis still needed after the second WebKit commit.Before and after
bad.jsisconst x;.two.jsisconst x;\nconst y;. Columns are main and this branch.constructor.nameBuildMessageAggregateErrorSyntaxErrorError.isError(e)falsetruetruee instanceof BuildMessagetruefalsefalsee.messageThe constant "x" must be initialized2 errors building "<dir>/two.js"The constant "x" must be initializede.stackundefinedundefinedforimport(), the frames of the caller forrequire()SyntaxError: <message>\n at <parse> (<dir>/bad.js:1:7)e.line/e.column0/6undefined, or the call site forrequire()1/7e.errorsBuildMessagee.position,e.notes,e.levelJSON.stringify(e){"name":"BuildMessage","position":{...},"message":...,"level":"error"}{}{}Object.prototype.toString.call(e)[object BuildMessage][object Error][object Error]structuredClone(e)DataCloneErrorErrorwithouterrorsSyntaxErrorwithstack,line,column,sourceURLutil.inspect(e)[BuildMessage: <message>][AggregateError: ...] { [errors]: [...] }SyntaxError: <message>and the frameconsole.log(e),Bun.inspect(e), a failedexpect(e)matcherStatic imports and the entry point give the same value as
import().Other changes in behaviour
import()andrequire()of the file itself read the file again and get a new error, as on main.BuildMessageis printed once per process.stackhas one frame, the position in the file. Forrequire()of a file with several diagnostics, main had the frames of the caller.AggregateError.Worker:ErrorEvent.messageis what the console prints for the error, as for every other error. For one diagnostic it wasSyntaxError: <message>. It is now the code frame with the file and the position. For several it does not change.node:worker_threads: the parent gets aSyntaxErrorwithstack,line,columnandsourceURL. It was aSyntaxErrorwith the message alone, or anErrorthat said2 errors building.node:replprints the file, the source line and a caret aboveUncaught SyntaxError, as Node does. It printedUncaught BuildMessage: <message>with no file.causegiven to theErrorconstructor, the error is printed with its code frame. Main printed nothing for that cause.--unhandled-rejections=throwthe diagnostics are printed twice. That mode prints every rejected Error twice today (Stop the rejection pass after a report that no listener took, and print an error once under --unhandled-rejections=throw #46199).What does not change
Bun.buildlogs,Bun.Transpiler, notes, and theBuildMessageandResolveMessageclasses.BuildMessage: a file that cannot be read, a file over 2 GiB, an error in a transcoded XML document,(a, ...b);(the parser gives it no position, A Loc is always a byte offset: Option<Loc> for absence, no -1 sentinel #38825).BuildMessage(Maximum call stack size exceeded). JavaScriptCore and Node report aRangeErrorfor it.ResolveMessage. A log whose first error is a resolve error stays anAggregateError.codeandcauseof the error itself are not printed at the top level, as for aBuildMessage. No GitHub annotation and no inspector event, as for aBuildMessage.What still differs from Node
.jsonfile, give aSyntaxErrorwithoutposition,notesandlevel, and with otherlineandstackrules (dylan-conway's [JSC] An error whose captured stack has no frames gets its stack from the host's hook WebKit#829 was the attempt for the first).SyntaxErrorhere: a reassigned constant (until parser: assignment to a const is a runtime TypeError, not a load-time error #45010),importnext tomodule.exports, a macro that throws.stackhas the frames of the caller.interface SyntaxErroraugmentation breaks everyclass X extends SyntaxError(TS2510).Open PRs that touch the same code
error_instance_from_locationis the model for the factory here. It removes theBuildMessagearm of the Worker hook, which this PR leaves alone. Its frame text isat <file>:<line>:<column>with a change to the stack parser. This PR writesat <parse> (<file>:<line>:<column>), which the printer reads today.vm.log. The printer arm here appends to it, as theBuildMessagearm does. Whichever lands second adapts.BuildMessagearm of the Worker hook and the bound of 256.ErrorEvent.filename,linenoandcolnofor every worker error. This value then needs no arm of its own.Measurements
Release builds. A is main b1305dd with the first preview of oven-sh/WebKit#451. B is this branch.
require(): 61,043 to 65,141 instructions. A failedimport(): 70,622 to 74,982.console.error(new TypeError("x")): +112 instructions of about 32,300. For aSyntaxError: +160. An uncaughtTypeError: 4,088,599 and 4,092,759 for the whole process, ranges overlap.bun -e 0: 4,029,403 and 4,030,467, ranges overlap.--trimsize is not measured: no build in my environment can trim.Suites run on the debug build
build-error.test.ts,inspect-error.test.js,plugins.test.ts,macro-test.test.ts,junit.test.js,xml.test.js,transpiler-cache.test.ts,node-module-module.test.js,test-util-types.test.js,mock-module.test.ts,mock-hoist.test.ts,resolve-error.test.ts, the new tests inworker.test.ts,worker_threads.test.tsandrepl.test.ts, andtest/js/node/test/parallel/test-esm-error-cache.js. On a release build: the fivetest/aot/*.test.tsfiles that pin this value, and 173 programs oftest/aot/parallel(modules, workers, module-loaders, and each program that names a SyntaxError).The test "a module whose onLoad callback rejects with the load error of another module" hangs on main for an
AggregateErrorand for an evaluation error. It passes with the second WebKit commit and no change in Bun.no test proof · iteration 1 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/node/module/node-module-module.test.js, test/js/bun/repl/repl.test.ts, test/js/bun/plugin/plugins.test.ts, test/cli/run/transpiler-cache.test.ts, test/aot/a-file-with-a-mistake-that-only-a-computed-name-finds-stops-no-build.test.ts