Skip to content

fix: narrows engines.node to the runtime the shipped binary targets - #12

Open
eastagiletracker wants to merge 1 commit into
QbDVision-Inc:mainfrom
eastagiletracker:agile-board/node-22-engine-range
Open

fix: narrows engines.node to the runtime the shipped binary targets#12
eastagiletracker wants to merge 1 commit into
QbDVision-Inc:mainfrom
eastagiletracker:agile-board/node-22-engine-range

Conversation

@eastagiletracker

Copy link
Copy Markdown

This PR proposes narrowing engines.node to the Node 22 runtime the shipped binary is actually built for, and having install.js verify that the two agree. We include this PR work along with a full history of your repo at https://eastagiletracker.com/projects/335. You can sign in with your GitHub ID to claim ownership of the project.

What's wrong

binding/muhammara.node.zip has contained the node-v127 prebuild since node 22 support landed — the binary exports node_register_module_v127, so it only loads on Node 22. But engines.node still reads ">= 20.18.1 <23", which is the bound from the Node 20 era. Every other place that names a runtime already says 22: the description field, .node-version, the README version history, and LAMBDA_NODE_VERSION in scripts/release.js.

The consequence is that lambda-muhammara advertises support for a runtime it cannot run on. On nodejs20.x the install succeeds, the copy step from the README succeeds, the deploy succeeds — and then the function dies on its first invocation. Reproduced against current main (the binary in the tree at 63c6ba2 is the same node-v127 build), using the Lambda base image for a runtime engines.node currently permits:

$ docker run --rm --platform linux/amd64 --entrypoint bash public.ecr.aws/lambda/nodejs:20 -c '
    cd /tmp && npm init -y >/dev/null &&
    npm install muhammara@6.0.5 lambda-muhammara@6.0.5 >/dev/null 2>&1 &&
    cp node_modules/lambda-muhammara/binding/muhammara.node node_modules/muhammara/binding/muhammara.node &&
    node -e "require(\"muhammara\")"'

Error: The module '/tmp/node_modules/muhammara/binding/muhammara.node'
was compiled against a different Node.js version using
NODE_MODULE_VERSION 127. This version of Node.js requires
NODE_MODULE_VERSION 115.
  code: 'ERR_DLOPEN_FAILED'

The same commands against public.ecr.aws/lambda/nodejs:22 print muhammara OK on v22.23.2.

The change

engines.node becomes ">= 22.0.0 <23" in package.json and in its package-lock.json mirror, so the declared range matches the prebuild in binding/ and the runtime the rest of the repo names.

install.js now reads the NODE_MODULE_VERSION back out of the unpacked binary — a compiled addon exports it as node_register_module_v<N> — and compares it against the major that engines.node declares, using the same ABI table MAINTAINERS.md sends you to when picking a prebuild. That is the invariant scripts/release.js currently asks a human to hold ("Keep in sync with the runtime named in package.json's description"), and it drifted once already. A mismatch is now a hard error naming the prebuild to ship; an unrecognised Node major or a binary with no such symbol warns and carries on, so the check can never block a release it does not understand.

How it was verified

npm ci on Node 22.23.2 with npm 12.0.2 is green both before and after — same result, no new failures — and now prints Binary targets Node 22 (NODE_MODULE_VERSION 127). The evidence that the check works is that it fails on the tree as it stands today: applying only the install.js half, without touching engines.node, exits 1 with binding/muhammara.node is a NODE_MODULE_VERSION 127 build, but package.json declares Node 20 (NODE_MODULE_VERSION 115). With the engines.node correction applied it passes. Its other branches were exercised directly: declaring Node 24 fails and asks for the node-v137 prebuild, an unknown major and a non-addon file both warn and exit 0, and a missing zip now reports one clean line instead of an unhandled string rejection.

Beyond the test tree, npm pack from this branch produces the same six-file artifact as before, and that tarball run through the Docker sanity check in MAINTAINERS.md loads on public.ecr.aws/lambda/nodejs:22 and writes a real PDF.

Two notes on the parts that touch things you own. The new check cannot affect anyone installing from npm: prepare is not run for a dependency's tarball, which I confirmed with npm install --foreground-scripts against the packed build — the unzipped binding/muhammara.node is already in the tarball, so the README copy step is untouched. And narrowing the lower bound does tighten who can install from a Node 20 or 21 machine, which is a judgement call rather than a fact: it is consistent with how engines.node has tracked the target Lambda runtime for this package's whole history, and with the <23 ceiling and engine-strict=true you already have, but if you would rather engines.node describe the developer's machine, I am glad to drop that hunk and keep only the check plus a corrected description.

One thing spotted in passing and deliberately left alone: main points at install.js, which requires extract-zip, a devDependency — so require("lambda-muhammara") throws on a consumer install. Nothing documents that entry point, so it is out of scope here.

How this was managed

This work was tracked as a single story, engines.node still permits Node 20/21, but the shipped binary is a Node 22 (ABI 127) build, on a board at https://eastagiletracker.com/projects/335 imported from this repository's own issues and pull requests — 11 stories, with the merged pull requests sitting in the Done iterations they were merged in.

board

If you'd rather not receive contributions like this, reply no-more-prs on this pull request and we won't open any further ones on your repositories.


Lawrence W. Sinclair
CEO / East Agile
linkedin.com/in/lwsinclair/
eastagile.com

binding/muhammara.node.zip has shipped the node-v127 (Node 22) prebuild
since node 22 support was added, but engines.node still allowed Node
20.18.1 and up. On a nodejs20.x Lambda the copy step in the README
therefore produces a module that cannot load: require("muhammara") fails
with ERR_DLOPEN_FAILED, NODE_MODULE_VERSION 127 vs 115.

Narrows engines.node (and its lockfile mirror) to ">= 22.0.0 <23", which
matches .node-version, the package description and LAMBDA_NODE_VERSION in
scripts/release.js.

install.js now reads the NODE_MODULE_VERSION back out of the unpacked
binary and compares it to the runtime engines.node declares, so the two
cannot drift apart unnoticed again. The check runs where install.js
already runs -- `npm ci`, i.e. step 3 of the release script -- and not on
consumer installs, where npm does not run a dependency's prepare script.
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.

1 participant