Repository navigation
Test runner does not print Error cause #44656
Description
Activity
It looks like
err.stackis used for the output, but usingutil.format(err)is what yields the correct cause output. Not sure if that's the Correct Way but I'm willing to submit a PR if that's an appropriate fix.I'm sure somewhere a conversation has been had about what
.stackreturns for legacy reasons or something, but first reaction is that that would be where this should be updated.Reacted by Moshe AtlowReacted by Ruben BridgewaterThat would be great! Using
util.inspect(error)would indeed be the correct way.Side note: we might want to add a linter rule that prevents using
.stackinlib. It might just have a couple of false positives though.Reacted by Josh Junon@BridgeAR to be clear, is
.inspect()or.format()more preferable? My gut instinct also says.inspect()but just want to be sure.@MoLow looks like just a
|-with the first line removed and properly indented, no?Something like
util.inspect(error).split('\n').slice(1).forEach(line => stream.write(`${indent}${line}\n`))
Extra error properties would kind of blend in though, was there a specific reason
atwas omitted?e.g. without any processing (though I manually removed the first line)
> console.log(util.inspect((() => { e = new Error('hi'); e.foo = 'bar'; return e})())) at REPL25:1:39 at REPL25:1:81 at Script.runInThisContext (node:vm:129:12) at REPLServer.defaultEval (node:repl:572:29) at bound (node:domain:433:15) at REPLServer.runBound [as eval] (node:domain:444:12) at REPLServer.onLine (node:repl:902:10) at REPLServer.emit (node:events:525:35) at REPLServer.emit (node:domain:489:12) at [_onLine] [as _onLine] (node:internal/readline/interface:425:12) { foo: 'bar' }vs with trim +
atremoval:> console.log(util.inspect((() => { e = new Error('hi'); e.foo = 'bar'; return e})()).split('\n').map(l => l.trimLeft().replace(/^at /, '')).join('\n')) REPL28:1:39 REPL28:1:81 Script.runInThisContext (node:vm:129:12) REPLServer.defaultEval (node:repl:572:29) bound (node:domain:433:15) REPLServer.runBound [as eval] (node:domain:444:12) REPLServer.onLine (node:repl:902:10) REPLServer.emit (node:events:525:35) REPLServer.emit (node:domain:489:12) [_onLine] [as _onLine] (node:internal/readline/interface:425:12) { foo: 'bar' }No particular reason. It was more of a stylistic choice.
Reacted by Moshe AtlowIn case leading whitespace is not required: it's possible to define that as an option using
util.inspect(). That way there's no need to trim the whitespace (even though I believe it's easier to read in case it's kept. I am just not sure if it's required to keep it for a specific output).Haven't forgotten about this - trying to get tests on
mainworking first.Looking at the implementation, I'd be stripping away a lot of logic that's there just by switching to
util.inspect(). Since we're trying to serve YAML results a la TAP, perhaps I can do something a bit more 'proper'.Would it be beneficial to instead output a YAML array of
causes (iterating over the error object until!causeis true) and outputting each of the errors as an array entry? That would also allow for custom properties to be mapped to YAML object keys for each array entry as well.Something like...
try { somethingThatThrows(); } catch (error) { const err = new Error('operation failed', {cause: error}); err.foo = 'BAR'; throw err; }
- message: operation failed stack: - ... - ... - ... foo: BAR - message: internal error code: INTERNAL_ERROR stack: - somethingThatThrows() (...) - ... - ...
Not sure if that's breaking the TAP protocol at all but handles this case well whilst adding more value to the YAML output, in my opinion.
- linked a pull request that will close this issuetest_runner: use v8.serialize instead of TAP #47867
on May 10, 2023 - added a commit that references this issue
on May 15, 2023 - added a commit that references this issue
on May 15, 2023 - added a commit that references this issue
on Jul 6, 2023 - added a commit that references this issue
on Jul 6, 2023
Version
v18.7.0
Platform
Darwin Joshs-MBP-2 21.6.0 Darwin Kernel Version 21.6.0: Sat Jun 18 17:07:22 PDT 2022; root:xnu-8020.140.41~1/RELEASE_ARM64_T6000 arm64
Subsystem
Test Runner
What steps will reproduce the bug?
How often does it reproduce? Is there a required condition?
Unconditional
What is the expected behavior?
Getting
causeoutput such as:What do you see instead?
(partial TAP output)
Additional information
The
causeparameter is essential in understanding e.g.undici's (Node.js's "native"fetch()implementation) error messages as otherwise you just get a crypticfetch failederror message. This is a huge pain point in trying to adopt the test runner for a new project.