util: fix crashing when emitting new Buffer() deprecation warning
Closes #53075
@anonrig
I tried to implement a test few weeks ago, and now again invested an hour. I cant catch the thrown error.
Just merge it please.
Well, tbh a comment is superficial. getFileName can return undefined
Do you insist on the comment?
I don't insist.
I found the deleted comment useful to understand the PR without reading more context.
By adding an approval with it I intended to say it's good as is.
Also, thanks for the fix. I work with some people who are waiting for it to ship :)
Well, just for the context:
This is a potential test:
try { new Buffer(1); console.log(0); ; process.exit(0); } catch (e) { console.log(1); process.exit(1); }
I just get the error, if I run node as repl. So run node and copy paste it. Enter. And you get a exit code of 1
store the line in a file, e.g. test.js, and run it: node test.js
It returns 0
This is because if you store it in a file, you have a filename
So this was my approach to write a test, but it doesnt work.
'use strict';
require('../common');
const assert = require('assert');
const { spawn } = require('child_process');
const child = spawn(process.execPath, [
'--interactive',
]);
child.stdin.end(`try { new Buffer(1) } catch (e) {process.exit(1)}`);
child.on('exit', (code) => {
assert.strictEqual(code, 1);
});
This is what I get:
aras@aras-Lenovo-Legion-5-17ARH05H:~/workspace/node$ node ./test/parallel/test-buffer-constructor-no-throw.js
node:assert:126
throw new AssertionError(obj);
^
AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:
0 !== 1
at ChildProcess.<anonymous> (/home/aras/workspace/node/test/parallel/test-buffer-constructor-no-throw.js:13:10)
at ChildProcess.emit (node:events:520:28)
at ChildProcess._handle.onexit (node:internal/child_process:294:12) {
generatedMessage: true,
code: 'ERR_ASSERTION',
actual: 0,
expected: 1,
operator: 'strictEqual'
}
Node.js v23.0.0-nightly20240507be8d64ec14
I have no clue, why the test isnt passing. Maybe somebody has some deeper insight. Maybe I just have to spawn the node process differently?
Tomorrow is my last day before vacation but I'll see if I can put some time in an attempt to isolate the code that throws this error for us.
CI: https://ci.nodejs.org/job/node-test-pull-request/60001/
what now?
@anonrig
what now?
@anonrig
There is a CI lockdown. We need to wait couple of days to land this.
@anonrig Can we merge it now?
@anonrig
Can we now merge it?
@jasnell @anonrig @mcollina
is it mergable?
CI: https://ci.nodejs.org/job/node-test-pull-request/60239/
CI: https://ci.nodejs.org/job/node-test-pull-request/60240/
Landed in 096e44a8a9dd20e3159ee15bc2563c6a58dc344f
Is there an ETA for this getting backported to v20?
It seems that it's not part of just released v20.16.0? (#53945)
Is there an ETA for this getting backported to v20?
It seems that it's not part of just released
v20.16.0? (#53945)
I've added the LTS watch label for v20.
If a someone disagrees with the addition of that label, feel free to remove it.