util: fix crashing when emitting new Buffer() deprecation warning

[Uzlopak]

Closes #53075

[anonrig]

Can you add a test?

[Uzlopak]

@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.

[naugtur]

I suggest leaving a line of comment explaining it's meant to recognize that we're dealing with a Node.js core module.

[Uzlopak]

Well, tbh a comment is superficial. getFileName can return undefined

Do you insist on the comment?

[naugtur]

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 :)

[Uzlopak]

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?

[naugtur]

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.

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/60001/

[Uzlopak]

what now?

@anonrig

[anonrig]

what now?

@anonrig

There is a CI lockdown. We need to wait couple of days to land this.

[Uzlopak]

@anonrig
Can we merge it now?

[Uzlopak]

@anonrig

Can we now merge it?

[Uzlopak]

@jasnell
@anonrig
@mcollina

is it mergable?

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/60239/

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/60240/

[mcollina]

lgtm

[nodejs-github-bot]

Landed in 096e44a

[legobeat]

Is there an ETA for this getting backported to v20?

It seems that it's not part of just released v20.16.0? (#53945)

[avivkeller]

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.