doc: Fix message type in Assert docs

[cottrellio]

doc: fix message type in assert docs

The Assert Docs currently state that type {any} is accepted for the
message arg in all of the assert methods. It should be
{string|undefined}.

Checklist

• documentation is changed or added
• commit message follows commit guidelines

Affected core subsystem(s)

• doc

[gireeshpunathil]

I think {any} is fine:

#node -v
v8.1.2
#node
> var assert = require('assert')
undefined
> var l = new Array()
undefined
> var r = new Object()
undefined
> assert(l != r, l)
undefined
> assert(l == r, l)
AssertionError [ERR_ASSERTION]
    at repl:1:1
    at ContextifyScript.Script.runInThisContext (vm.js:44:33)
    at REPLServer.defaultEval (repl.js:239:29)
    at bound (domain.js:301:14)
    at REPLServer.runBound [as eval] (domain.js:314:12)
    at REPLServer.onLine (repl.js:433:10)
    at emitOne (events.js:120:20)
    at REPLServer.emit (events.js:210:7)
    at REPLServer.Interface._onLine (readline.js:278:10)
    at REPLServer.Interface._line (readline.js:625:8)
> assert(l == r, ['h', 'e', 'l', 'l', 'o'])
AssertionError [ERR_ASSERTION]: h,e,l,l,o
    at repl:1:1
    at ContextifyScript.Script.runInThisContext (vm.js:44:33)
    at REPLServer.defaultEval (repl.js:239:29)
    at bound (domain.js:301:14)
    at REPLServer.runBound [as eval] (domain.js:314:12)
    at REPLServer.onLine (repl.js:433:10)
    at emitOne (events.js:120:20)
    at REPLServer.emit (events.js:210:7)
    at REPLServer.Interface._onLine (readline.js:278:10)
    at REPLServer.Interface._line (readline.js:625:8)
> 
> var msg = {}
undefined
> msg.toString = () => { return 'custom'};
[Function]
> assert(l == r, msg)
AssertionError [ERR_ASSERTION]: custom
    at repl:1:1
    at ContextifyScript.Script.runInThisContext (vm.js:44:33)
    at REPLServer.defaultEval (repl.js:239:29)
    at bound (domain.js:301:14)
    at REPLServer.runBound [as eval] (domain.js:314:12)
    at REPLServer.onLine (repl.js:433:10)
    at emitOne (events.js:120:20)
    at REPLServer.emit (events.js:210:7)
    at REPLServer.Interface._onLine (readline.js:278:10)
    at REPLServer.Interface._line (readline.js:625:8)
> 

[cottrellio]

Thank you for responding so quickly. This is what I am experiencing.

I am unable to access properties on the message of type object because it is being "stringified" under the hood. In your example of message being an array, the result is a string not an array. Here is an example using an object.

#node -v
v8.5.0
#node
> const assert = require('assert')
undefined
> const obj = { status: 404, detail: 'Not Found' }
undefined
> try {
... assert.equal(1, 2, obj)
... } catch(e) {
... console.log(e.message);
... console.log(e.message.status);
... }
[object Object]
undefined

It appears you can input {any}, but you are returned {string|undefined}. Perhaps this PR should be to add expected return types to the docs.

In order to access the properties of my object message I had to JSON.stringify and then JSON.parse which defeats the purpose.

try {
  assert.equal(1, 2, JSON.stringify({ status: 404, detail: 'Not Found' }));
} catch (e) {
  const parsed = JSON.parse(e.message);

  ctx.throw(parsed.status, parsed.detail);
}

[gireeshpunathil]

@cottrellio - thanks for the quick revert. As long as a toString() is present, the data is meaningfully transported to the assertion site, but definitely not iterable, as it is now a flat string.

const assert = require('assert')
const util = require('util')

const obj = {status: 404, detail: 'Not found', 
             toString: () => { return "'status': 404, 'detail': 'not found'"}}
try {
  assert.equal(1, 2, obj)
} catch(e) {
  console.log(obj === e)
  console.log(util.inspect(obj, {showHidden: true, depth: null}))
  console.log(util.inspect(e))
}

Assuming that the purpose of the assertion is to communicate the violation of a pre-condition or an un-expected program state, the message argument being designed as a string or anything that can be converted to a string makes sense to me. Complex user objects do not make much meaning in a throw site is probably the rational.

Adding a caveat to the doc to this effect (that user defined objects passed to the assert will be stringified based on toString()) looks to be more reasonable to me, but let us wait for more views from other collaborators.

[cottrellio]

@gireeshpunathil Again, thank you for your response. I agree with your rational. Thank you for clarifying.

[lpinca]

It is basically like doing:

new Error({ status: 404, detail: 'Not Found' })

I think it makes sense.

[gireeshpunathil]

sort of stuck here. @lpinca - can you please clarify your last comment? do you endorse the changes as it is, or propose alternatives?

[lpinca]

@gireeshpunathil I agree with what you wrote in #16427 (comment). Keeping any is better imho.

[gireeshpunathil]

ping @nodejs/collaborators to get some concurrence on the proposal, and based on it ping @cottrellio to amend the doc accordingly

[Trott]

any is correct. It refers to what is accepted for the message argument, not any result.

[Trott]

Perhaps this PR should be to add expected return types to the docs.

There is no return type. assert.fail('some arbitrary mesage') throws an error, but doesn't return a value.

const assert = require('assert');

var foo = assert.ok(true, 'message value');
console.log(foo); // undefined

try {
  foo = assert.fail('message here');
} catch (e) {
}

console.log(foo); // undefined

[Trott]

In order to access the properties of my object message I had to JSON.stringify and then JSON.parse which defeats the purpose.

I'm not sure it defeats the purpose. Assertions should terminate your program. Using them for programmatic logic seems to suggest that something else would be better and/or that we're missing functionality in Node.js that needs to be added.

[Trott]

@cottrellio Thanks for the pull request! It seems to me like this can be closed, but if you disagree, leave a comment and we'll re-open. Hope you have everything you need!