fix: resolve uWebSockets.js via a tarball URL, not a github: git spec#1756
Conversation
Same issue as harper-pro#561: the github: shorthand (github:uNetworking/uWebSockets.js#v20.68.0) gets re-resolved by npm as git+ssh://github.com/... on any npm install. Our Docker build stage has no SSH credentials for github.com, so npm silently skips the (optional) dependency and the shipped image never bundles the native addon — HARPER_UWS_UDS / HARPER_UWS_HTTP are inert even when set. An explicit git+https:// spec doesn't fix this either — confirmed with a clean npm cache that npm/hosted-git-info canonicalizes ANY github.com git dependency back to git+ssh:// regardless of requested protocol. Switching to a plain tarball URL (https://.../archive/<sha>.tar.gz) sidesteps hosted-git-info entirely: npm treats it as a remote-tarball dependency, resolved stays a plain https URL with a pinned integrity hash, and it can't regress on a future npm install. Verified npm ci installs all 15 native .node binaries in a HOME-stripped, credential-less environment (matching the Docker build stage) both before and after a full npm install regenerates the lockfile from package.json.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the uWebSockets.js optional dependency in package.json to point to a specific tarball archive URL instead of a GitHub tag. The reviewer pointed out that the corresponding npm-shrinkwrap.json or package-lock.json file needs to be updated and committed as well, otherwise CI/CD pipelines using npm ci will continue to use the old resolved URL, rendering this change ineffective.
| "bufferutil": "^4.0.9", | ||
| "segfault-handler": "^1.3.0", | ||
| "uWebSockets.js": "github:uNetworking/uWebSockets.js#v20.68.0", | ||
| "uWebSockets.js": "https://github.com/uNetworking/uWebSockets.js/archive/624987739d4da0acb628aaf2a10fc43e7ce27c5c.tar.gz", |
There was a problem hiding this comment.
The repository tracks npm-shrinkwrap.json (as listed in the files array of package.json). However, the updated npm-shrinkwrap.json (or package-lock.json) is missing from this pull request. Without committing the updated lockfile/shrinkwrap, npm ci in CI/CD pipelines will continue to use the old resolved git URL, making this change ineffective.
|
Reviewed; no blockers found. |
1 participant
Summary
Same root cause as harper-pro#561, filed against this repo since
uWebSockets.js(added for #1096) is declared here too:package.jsondeclareduWebSockets.jsvia thegithub:shorthand (github:uNetworking/uWebSockets.js#v20.68.0).hosted-git-inforesolves that togit+ssh://git@github.com/...inpackage-lock.json.github.com. Since it's an optional dependency,npm cisilently skips the failed clone — no error, no warning, build succeeds without it.uWebSockets.jsaddon, soHARPER_UWS_UDS/HARPER_UWS_HTTPare inert even when explicitly set.An explicit
git+https://spec does not fix this — verified with a clean npm cache thathosted-git-infocanonicalizes any github.com git dependency back togit+ssh://regardless of the requested protocol.Fix
Point
package.jsonat a plain tarball URL (https://github.com/uNetworking/uWebSockets.js/archive/<sha>.tar.gz) instead of agithub:git spec. npm treats this as a remote-tarball dependency, not hosted-git, so the ssh-canonicalization path never runs.resolvedstays a plain https URL with a pinned integrity hash — immune to regressing on a futurenpm install.Verification
Ran
npm ciin a HOME-stripped, credential-less environment (no~/.ssh, no SSH agent) matching the Docker build stage, both before and after a fullnpm installregenerates the lockfile frompackage.json:uWebSockets.jssilently absent fromnode_modules(0 native.nodebinaries).Test plan
coresubmodule is bumped to include this) bundlesnode_modules/uWebSockets.js/*.nodeHARPER_UWS_UDSon a dev cluster and confirm the uWS backend actually engages