deno icon indicating copy to clipboard operation
deno copied to clipboard

fix(ext/console): Error Cause Not Inspect-Formatted when printed

Open MujahedSafaa opened this issue 1 year ago • 4 comments

This pull request addresses an issue where the Error.cause property was not formatted correctly when printed using console.log, leading to confusion.

solution: Implemented a fix to ensure that Error.cause is formatted properly when printed by console.log, and the fix done by using JSON.stringify

This PR fixes https://github.com/denoland/deno/issues/23416

MujahedSafaa avatar Jul 11 '24 11:07 MujahedSafaa

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Jul 11 '24 11:07 CLAassistant

The PR seems to be missing a test case that exercises the change and shows why it was made. What happens when the .stack property holds something that JSON.stringify cannot deal with like a bigint?

The code run JSONstringify only in case the stack value is null or undefined.

MujahedSafaa avatar Jul 16 '24 12:07 MujahedSafaa

Try running this snippet with the changes in this PR. On my end it causes an unexpected exception:

console.log(new Error("foo", { cause: 9007199254740991n }));

Output:

error: Uncaught (in promise) TypeError: Do not know how to serialize a BigInt
console.log(new Error("foo", { cause: 9007199254740991n }));
        ^
    at stringify (<anonymous>)
    at ext:deno_console/01_console.js:1496:13
    at Array.map (<anonymous>)
    at inspectError (ext:deno_console/01_console.js:1490:5)
    at formatRaw (ext:deno_console/01_console.js:852:16)
    at formatValue (ext:deno_console/01_console.js:530:10)
    at inspectArgs (ext:deno_console/01_console.js:3044:17)
    at console.log (ext:deno_console/01_console.js:3113:7)
    at file:///Users/marvinh/dev/test/deno-cause/main.ts:1:9

Deno should not error like that. To make it work we must ensure that any JS value can be printed, not just a subset.

marvinhagemeister avatar Jul 16 '24 12:07 marvinhagemeister

Try running this snippet with the changes in this PR. On my end it causes an unexpected exception:

console.log(new Error("foo", { cause: 9007199254740991n }));

Output:

error: Uncaught (in promise) TypeError: Do not know how to serialize a BigInt
console.log(new Error("foo", { cause: 9007199254740991n }));
        ^
    at stringify (<anonymous>)
    at ext:deno_console/01_console.js:1496:13
    at Array.map (<anonymous>)
    at inspectError (ext:deno_console/01_console.js:1490:5)
    at formatRaw (ext:deno_console/01_console.js:852:16)
    at formatValue (ext:deno_console/01_console.js:530:10)
    at inspectArgs (ext:deno_console/01_console.js:3044:17)
    at console.log (ext:deno_console/01_console.js:3113:7)
    at file:///Users/marvinh/dev/test/deno-cause/main.ts:1:9

Deno should not error like that. To make it work we must ensure that any JS value can be printed, not just a subset.

Yes, it causes an error also on my side, I will add a fix to it.

MujahedSafaa avatar Jul 16 '24 12:07 MujahedSafaa

@marvinhagemeister I changed my code to use inspect rather than JSONstringify, since it handles all the data types

MujahedSafaa avatar Jul 17 '24 09:07 MujahedSafaa