Skip to content

Ensure compatibility with Node.js 10 and 12#30

Merged
lovell merged 1 commit intolovell:mainfrom
fastman:main
Sep 24, 2025
Merged

Ensure compatibility with Node.js 10 and 12#30
lovell merged 1 commit intolovell:mainfrom
fastman:main

Conversation

@fastman
Copy link
Contributor

@fastman fastman commented Sep 24, 2025

In Node.js 10 and 12, callback to fs.close() is required:

Can be verified using npm run bench:

ubuntu@ubuntu:/tmp/detect-libc (main) $ node -v
v10.24.1
ubuntu@ubuntu:/tmp/detect-libc (main) $ npm run bench

> detect-libc@2.1.0 bench /tmp/detect-libc
> node benchmark/detect-libc

fs.js:136
    throw new ERR_INVALID_CALLBACK();
    ^

TypeError [ERR_INVALID_CALLBACK]: Callback must be a function
    at makeCallback (fs.js:136:11)
    at Object.close (fs.js:394:20)
    at fs.read (/tmp/detect-libc/lib/filesystem.js:40:12)
    at FSReqWrap.wrapper [as oncomplete] (fs.js:467:17)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! detect-libc@2.1.0 bench: `node benchmark/detect-libc`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the detect-libc@2.1.0 bench script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/ubuntu/.npm/_logs/2025-09-24T13_19_36_624Z-debug.log

@lovell
Copy link
Owner

lovell commented Sep 24, 2025

Thanks, the fs.close API has been on a bit of a journey with its optional then mandatory then optional callback. Luckily (well, intentionally) there's a try/catch around this so these errors are not propagated to users. However I agree it's best to include a no-op callback as this package still supports Node.js all the way back to version 8. Thanks for fixing.

@lovell lovell merged commit e64be76 into lovell:main Sep 24, 2025
35 checks passed
@lovell
Copy link
Owner

lovell commented Sep 24, 2025

v2.1.1 now available with this fix.

lambdalisue added a commit to lambdalisue/detect-libc that referenced this pull request Oct 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants