`console.assert(false, Symbol())` throws

[jcbhmr]

Version

v20.6.1

Platform

Linux PIG-2016 5.15.90.1-microsoft-standard-WSL2 #1 SMP Fri Jan 27 02:56:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

console

What steps will reproduce the bug?

node -e 'console.assert(false, Symbol())'

How often does it reproduce? Is there a required condition?

No response

What is the expected behavior? Why is that the expected behavior?

What do you see instead?

internal/console/constructor.js:401
      args[0] = `Assertion failed${args.length === 0 ? '' : `: ${args[0]}`}`;
                                                                     ^

TypeError: Cannot convert a Symbol value to a string
    at console.assert (internal/console/constructor.js:401:70)
    at Object.<anonymous> (/home/cg/root/6509ba68c2e82/main.js:1:9)
    at Module._compile (internal/modules/cjs/loader.js:999:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1027:10)
    at Module.load (internal/modules/cjs/loader.js:863:32)
    at Function.Module._load (internal/modules/cjs/loader.js:708:14)
    at Function.executeUserEntryPoint [as runMain] (internal/modules/run_main.js:60:12)
    at internal/main/run_main_module.js:17:47

Additional information

No response

[bnoordhuis]

Working as expected/documented. Try this instead:

console.assert(false, "%o", { hello: "world" })
// or even just:
console.assert(false, "", { hello: "world" })

It wouldn't be hard to use util.inspect() by default but:

1. it's the documented behavior
2. is mandated by the console spec
3. would be a rather visible change (can spam large amounts of text to the console) and therefore at least a semver-major change

You're welcome to open a pull request and see how it's received if you still think it's a good idea but be prepared for rejection.

[jcbhmr]

Are you sure? 🤔 it seems like console.assert just prepends a string to the vararg list and then passes it to the same Logger() that all the other console methods use.

Given that console.log(...data) is essentially just Logger(level, data) it would seem that it should inspect the object since console.assert(condition, ...data) also calls Logger(level, [generatedMessage, ...data]) which is analogous to console.log(generatedMessage, ...data) which does properly inspec the stuff in the data list. My logic:

console.log("Assertion failed", { a:1 })
//=> Logger("log", ["Assertion failed", { a:1 }])
//=> prints 'Assertion failed { a:1 }'

console.assert(false, { a:1 })
//=> Logger("assert", ["Assertion failed", { a:1 }])
//=> should print 'Assertion failed { a:1 }'

Idk tho I could be misunderstanding or mistaken 🤷‍♂️

[bnoordhuis]

Yes, I'm pretty sure. The more human-readable prose from the MDN lemma (emphasis mine):

A list of JavaScript objects to output. The string representations of each of these objects are appended together in the order listed and output.

It's only when you use a message format string that object formatting comes into play:

JavaScript objects with which to replace substitution strings within msg.

[jcbhmr]

After further investigation, here's the erroneous line:

node/lib/internal/console/constructor.js

Line 435 in cd97e28

args[0] = `Assertion failed${args.length === 0 ? '' : `: ${args[0]}`}`;

👆 this isn't precisely spec compliant. Why? Because there's supposed to be this:

which means something more like this:

  assert(expression, ...args) {
    if (!expression) {
      if (typeof args[0] === "string") {
        // if you provide a fmt string, it still gets used (first arg)
        args[0] = `Assertion failed: ${args[0]}`
      } else {
        // but if it's anything else, it's not stringified as a format string;
        // its just an object to be printed. so we prepend a fmt string arg (with no specifiers
        // that would eat any user-supplied objects) as a nice "Assertion failed" message
        ArrayPrototypeUnshift(args, "Assertion failed")
      }
      // The arguments will be formatted in warn() again
      ReflectApply(this.warn, this, args);
    }
  },

you can see a bug here from the current Node.js impl that highlights what's happening:

const mySymbol = Symbol()
console.assert(false, mySymbol)
// THROWS because Symbol() cant be stringified like `symbol string: ${Symbol()}`!

the mdn thing you cited:

A list of JavaScript objects to output. The string representations of each of these objects are appended together in the order listed and output.

this same language "string representations" is also used in console.info() and other console methods to mean the formatted pretty output not the .toString() version:
https://developer.mozilla.org/en-US/docs/Web/API/console/info

also backed up by someone from the whatwg/console repo:

My understanding of the spec is the object {hello: "world"} would get passed as a member of args (eg, ["Assertion failed", {hello: "world"}]) to Printer, which is implementation defined. Converting a POJO to the string "[object Object]" is certainly a valid way of being implementation defined, but I wouldn't judge it "to be maximally useful and informative."

whatwg/console#226 (comment)

so in summary it would seem:

• [object Object] is a valid string representation of the object
• ...but it's not very helpful
• there's the edge case with Symbol() not being stringifiable in the way console.assert() is implemented
• most other runtimes Chrome/Firefox/Deno/Bun do it where console.assert(false, {a:1}) is properly inspected; Node.js is the outlier.

i also am making an effort to clarify the mdn wording that had us all confused 🤣 mdn/content#29172

[jcbhmr]

Changed title and body to reflect exact buggy impl:

console.assert(false, Symbol())

[BridgeAR]

@bnoordhuis I believe it's fine to handle it that way. It aligns well with the other APIs where the object would be printed in a human readable way. There's also already an open PR and I guess we could reopen the issue as such?

[bnoordhuis]

Yeah, I'm fine with that, I don't really have a strong opinion either way except that changing the default should be a semver-major change.