node 9 loses error call site information in Buffer.concat

[reviewher]

• Version: node 9.1.0
• Platform: darwin

var f = (function () {
});
var g = (function() {
	Buffer.concat([new Buffer(3), f]);
});
g();

This results in an error. Using node 8.9.1, the error message clearly indicates that the problem is in the source:

buffer.js:444
      throw new TypeError(kConcatErrMsg);
      ^

TypeError: "list" argument must be an Array of Buffer or Uint8Array instances
    at Function.Buffer.concat (buffer.js:444:13)
    at g (/tmp/t.js:4:9) <--- notice the reference to t.js here, this is expected
    at Object.<anonymous> (/tmp/t.js:6:1)
    at Module._compile (module.js:635:30)

In node 9.1.0, the error message appears to come from the core:

buffer.js:475
      throw kConcatErr;
      ^

TypeError [ERR_INVALID_ARG_TYPE]: The "list" argument must be one of type array, buffer, or uint8Array
    at buffer.js:450:20
    at NativeModule.compile (bootstrap_node.js:601:7)
    at NativeModule.require (bootstrap_node.js:546:18)
    at util.js:26:22
    at NativeModule.compile (bootstrap_node.js:601:7)
    at Function.NativeModule.require (bootstrap_node.js:546:18)
    at setupGlobalVariables (bootstrap_node.js:263:31)
    at startup (bootstrap_node.js:32:5)
    at bootstrap_node.js:613:3 

[apapirovski]

ERR_INVALID_ARG_TYPE clearly indicates that the error is with the code that's passing the wrong type of an argument.

https://nodejs.org/api/errors.html#errors_err_invalid_arg_type

[apapirovski]

(That said, I understand that the stack trace could be better.)

[apapirovski]

Looks like we need to not predefine the error as it loses the stack trace information.

node/lib/buffer.js

Lines 450 to 452 in 1601a3c

	const kConcatErr = new errors.TypeError(
	'ERR_INVALID_ARG_TYPE', 'list', ['Array', 'Buffer', 'Uint8Array']
	);

Labeling as good first issue and happy to assist anyone that wants to fix.

[reviewher]

The relevant PR that made the shift from the old style (generating an error in call site) to the new one (precomputing error) is #13976 , which references the general migration #11273

The notes https://github.com/nodejs/node/blob/master/doc/guides/using-internal-errors.md don't give any comments on preserving error stack, so maybe this is intentional behavior?

ping @jasnell @starkwang

[apapirovski]

Definitely not intentional. I think it was just an oversight. The error format should remain but it should be generated in the call site.

[ah-yu]

I'd like to fix it