Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How does this interact with Error causes? #1

Open
voxpelli opened this issue Mar 2, 2022 · 11 comments
Open

How does this interact with Error causes? #1

voxpelli opened this issue Mar 2, 2022 · 11 comments

Comments

@voxpelli
Copy link

voxpelli commented Mar 2, 2022

Considering that error causes are now part of the JS standard, using that instead would be beneficial, no?

This is the approach eg I took with https://github.com/voxpelli/pony-cause

@staltz
Copy link
Owner

staltz commented Mar 2, 2022

Wow, I had no idea about this new ES feature! Thanks! I'll consider what to do now.

@voxpelli
Copy link
Author

voxpelli commented Mar 2, 2022

@staltz Feel free to pick my brain as much as you want, here or on Discord 👍

@staltz
Copy link
Owner

staltz commented Mar 2, 2022

Oh @voxpelli it seems like pony-cause doesn't cover a use case that's quite important for us: when an error doesn't have a .stack. Even if stackless errors shouldn't occur, they often happen in our software which is peer-to-peer (thus updating to newer libraries takes many years).

See:

console.log(
  stackWithCauses(
    new ErrorWithCause('bar', {cause: {name: 'TypeError', message: 'foo'}}),
  ),
);
ErrorWithCause: bar
    at Object.<anonymous> (/home/staltz/oss/playground/example.js:3:5)
    at Module._compile (internal/modules/cjs/loader.js:1015:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1035:10)
    at Module.load (internal/modules/cjs/loader.js:879:32)
    at Function.Module._load (internal/modules/cjs/loader.js:724:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:60:12)
    at internal/main/run_main_module.js:17:47

while

console.log(clarify('foo', 'bar'));
Error: bar
    at Object.<anonymous> (/home/staltz/oss/playground/example.js:1:13)
    at Module._compile (internal/modules/cjs/loader.js:1015:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1035:10)
    at Module.load (internal/modules/cjs/loader.js:879:32)
    at Function.Module._load (internal/modules/cjs/loader.js:724:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:60:12)
    at internal/main/run_main_module.js:17:47
  Error: foo

@voxpelli
Copy link
Author

voxpelli commented Mar 2, 2022

@staltz Good input! I have made an issue about adding that to stackWithCauses() 👍 voxpelli/pony-cause#22

@staltz
Copy link
Owner

staltz commented Mar 2, 2022

Thanks! And what about the case console.log(new ErrorWithCause(......))?

@voxpelli
Copy link
Author

voxpelli commented Mar 2, 2022

@staltz ErrorWithCause is meant to be a drop in replacement for the standardised new Error('foo', { cause: err }), to be used in contexts where the built in one isn't supported (older browsers and Node 14 and older I believe).

The standard proposal doesn't specify anything around how console.log() should handle it: https://github.com/tc39/proposal-error-cause

Though Firefox prints it like this:

Skärmavbild 2022-03-02 kl  11 08 21

And Node.js 16.14.0 and 17.3.0 shipped nodejs/node#41002, making it also printing the causes

@voxpelli
Copy link
Author

voxpelli commented Mar 2, 2022

Here's a screenshot of how Node 16.14.0 console.log handles it:

Skärmavbild 2022-03-02 kl  11 15 27

@voxpelli
Copy link
Author

voxpelli commented Mar 2, 2022

Eg. pino has got support merged as well: pinojs/pino-std-serializers#78

@staltz
Copy link
Owner

staltz commented Mar 2, 2022

Good to know! Note to self, here's how Node.js 16.14.0 renders "string errors" and stackless errors:

> console.log(new Error('bar', {cause: 'foo'}))
Error: bar
    at REPL10:1:13
    at Script.runInThisContext (node:vm:129:12)
    at REPLServer.defaultEval (node:repl:564:29)
    at bound (node:domain:421:15)
    at REPLServer.runBound [as eval] (node:domain:432:12)
    at REPLServer.onLine (node:repl:891:10)
    at REPLServer.emit (node:events:532:35)
    at REPLServer.emit (node:domain:475:12)
    at REPLServer.Interface._onLine (node:readline:487:10)
    at REPLServer.Interface._line (node:readline:864:8) {
  [cause]: 'foo'
}
> console.log(new Error('bar', {cause: {name:'TypeError',message:'foo'}}))
Error: bar
    at REPL11:1:13
    at Script.runInThisContext (node:vm:129:12)
    at REPLServer.defaultEval (node:repl:564:29)
    at bound (node:domain:421:15)
    at REPLServer.runBound [as eval] (node:domain:432:12)
    at REPLServer.onLine (node:repl:891:10)
    at REPLServer.emit (node:events:532:35)
    at REPLServer.emit (node:domain:475:12)
    at REPLServer.Interface._onLine (node:readline:487:10)
    at REPLServer.Interface._line (node:readline:864:8) {
  [cause]: { name: 'TypeError', message: 'foo' }
}

My use case is most important in Node.js, so would it be okay to ask that pony-cause follow the Node.js style of rendering in the cases of string errors and stackless errors?

@voxpelli
Copy link
Author

voxpelli commented Mar 2, 2022

@staltz That sounds like a good approach to it, I’ll try to make it happen soon 👍

@voxpelli
Copy link
Author

voxpelli commented Mar 2, 2022

@staltz What's your thoughts on this solution? voxpelli/pony-cause#23

voxpelli added a commit to voxpelli/pony-cause that referenced this issue May 31, 2022
voxpelli added a commit to voxpelli/pony-cause that referenced this issue May 31, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants