Upgrade Node.js to v24 and all npm dependencies by jcheng5 · Pull Request #596 · rstudio/shiny-server

jcheng5 · 2026-04-14T00:23:14Z

Summary

Upgrades Node.js from v20.17.0 to v24.20.0 and brings every npm dependency
current, including the Express 4 → 5 jump.

This branch has been rebuilt on top of current master and split into five
commits, each independently runnable and independently verified. The original
bundled everything into one change, which meant the test suite could tell you
that something broke but not what. Now each step was landed against a passing
suite, so a bisect points at a specific upgrade:

#	Commit	What moves
1	`39be23e`	Node 20 → 24 (plus the `nan` bump it requires, and the dead `.npmrc`)
2	`d0be475`	`http-proxy` → `http-proxy-3`
3	`756c884`	express 4 → 5 and send 0.19 → 1.2 (inseparable — Express 5 depends on serve-static 2 / send 1)
4	`aa2d34b`	TypeScript 5 → 6, and an explicit `target` in tsconfig.json
5	`ee63175`	Everything else, plus the transitive security fixes
—	`83f0644`	Memory-bank docs updated to match (no code)

Step 1 is first on purpose: it holds Node constant for every later comparison,
rather than confounding a runtime change with a dependency change.

The pre-rebase head was 5592fd0.

What actually changed in `lib/`

Strikingly little, which is the headline result:

lib/router/directory-router.js — send 1.x dropped the send.mime
re-export and delegates to mime-types, so registering .R as text/R goes
through there now. mime-types becomes a direct dependency so that our copy
and send's copy are guaranteed to be the same instance.
lib/proxy/http.js — the require, and dropping proxySocket from
knownEvents (a diagnostic list; http-proxy-3 no longer emits it, and
test/proxy-events.js fails on names that don't exist).
lib/core/python.ts — resolvePython_p was annotated
Q.Promise<PythonEnv> while implemented async. TypeScript 6 rejects that,
and it was always a fiction; it's Promise<PythonEnv> now.

Express 5 needed no middleware changes. The stack uses only app.use with
bare functions — no route path strings, so path-to-regexp 8 doesn't reach us —
and none of the APIs Express 5 removed were in use.

The large lib/*.js diffs are regenerated TypeScript output, not hand edits.

Behaviour changes

One, caught by the integration tests: .R files are now served as
text/R; charset=utf-8 instead of text/R; charset=UTF-8. Charset names are
case-insensitive (RFC 7231 §3.1.1.2), so this should be invisible to clients.
test/integration/static-files.js pins the new spelling.

TypeScript 6 also moved its default target off ES5, which silently rewrote
every committed .js (downlevel helpers → native async/await and spread).
That output is right for Node 24, but it shouldn't be a side effect of a
dependency bump, so tsconfig.json now says "target": "es2022" out loud.

Security

npm audit goes from 25 findings to 5, and all 5 that remain are in
development-only tooling (mocha's and nodemon's trees). Closed along the way:

websocket-driver 0.7.4 → 0.7.5 — critical, resource-limit bypass via
message compression. Both parents already allowed it; only the shrinkwrap pin
was stale.
flatted 3.3.1 → 3.4.4 — high, likewise already in log4js's range.
sockjs's uuid 8 → 11 — needs an overrides entry, since sockjs pins
^8.3.2. sockjs calls require('uuid').v4 and nothing else.

Deliberately not moved

q — 2.x was abandoned in 2015, and Q is being removed from this codebase
separately.
typescript 7 and http-proxy-3 2 — both released since this branch was
opened; each deserves its own change.
@types/node — held at 24.x to match the runtime rather than following
latest to 26, which would type against APIs Node 24 doesn't have.

Also dropped from the original branch: two functions (isKeepalive,
stripConnectionHeaders) that it appended to lib/proxy/http.js with no
callers.

Verification

Run after every one of the five commits, not just at the end:

npm test — 310 unit + integration tests passing
npm run test:r — 8 tests against a real R Shiny worker
tools/check-licenses.js — clean

And after the final commit, from a clean npm ci:

npm run build leaves lib/ unchanged (the CI freshness gate)
external/node/install-node.sh fetches v24.20.0, and the node-gyp-built
posix.node loads under it — so the packaged build and Jenkins, which both
run the vendored ./bin/node, pick up Node 24 with no further changes
A real server booted from dev/shiny-server.conf serves the directory index,
an R app page, /ping, __assets__/, and a 404 page

🤖 Generated with Claude Code

Fifteen documents under memory-bank/ covering the subsystems: request lifecycle, router chain, proxy layer, scheduler, app workers, transport, config system, native/privilege model, core utilities, logging and errors, testing, build and packaging, plus a project brief and a code-conventions document. Each carries YAML frontmatter (title, description) that the discovery skill reads to decide relevance. Also adds the memory-bank skill under .agents/skills/ (with a .claude/skills symlink so Claude Code finds it), the .gitignore allowlist that tracks it, and a CLAUDE.md section pointing at the docs. These record the "why" — invariants, design decisions, and gotchas — rather than restating code, and they call out several verified findings that are expensive to rediscover: the missing error-handling middleware combined with NODE_ENV never being set (so unhandled throws return a stack trace), the out-of-scope `res` in the pre-config upgrade guard, client-sessions being handed a null `res` on the upgrade path, `server.listening = ...` being a silent no-op against Node's getter, and assorted dead code. The docs were originally drafted against the PR #596 branch and have been reworded here to describe master. Concretely: `http-proxy` rather than `http-proxy-3`, express v4, send 0.x, ip-address ^9.0.5, Node v20.17.0, and `.npmrc` still present. Content that is genuinely specific to the pending upgrade is kept but quarantined under explicit "Pending upgrade (PR #596)" headings in techContext.md and proxyLayer.md, so it stays useful without misdescribing the current tree. Two corrections made while rewording: * `fba8b83` was an ip-address 6 -> 7 upgrade, not "the v10 migration". The throwing `Address4`/`Address6` constructors that motivate the try/catch wrappers in lib/core/iputil.js arrived in v7. * The "63 passing" figure was measured on the #596 branch. On master a Mac is expected to show one failure in test/app-worker.js, since darwin returns EROFS where the assertion expects EACCES — #596 is what fixes it. The claim is now attributed rather than stated flatly. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Node 20 leaves maintenance in April 2026; v24 is the current LTS. `.nvmrc` is the single source of truth -- external/node/install-node.sh reads it to pick the runtime that gets vendored into the package, and CI's setup-node steps use `node-version-file`. Two things had to move with it: * `nan` 2.20 does not compile against Node 24's V8 headers, so build/Release/posix.node -- and therefore every `require` chain that reaches it -- fails to load. Bumped to ^2.28.0, which does. * `.npmrc` set `scripts-prepend-node-path`, an option npm removed in v9. It has been inert for two major versions; deleted. This commit deliberately moves nothing else. The Express 4 stack now runs on Node 24 unchanged, which makes Node a constant for the dependency upgrades that follow rather than a variable confounded with them. Verified on Node 24: 310 unit + integration tests pass, the 8 R integration tests pass, and the license check is clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

http-proxy's last release was 1.18.1 in May 2020. http-proxy-3 is a drop-in fork under active maintenance; the API ShinyProxy uses (createProxyServer, web, ws, and the proxyRes/error events) is unchanged. The one visible difference is that `proxySocket` is gone from the event set. That name only ever appeared in `knownEvents`, which is purely diagnostic -- lib/proxy/http.js wraps proxy.emit to log a warning for any event it doesn't recognise, and nothing subscribes to proxySocket. Leaving it in place would make test/proxy-events.js fail as a "fictional event", which is exactly what that test is for. Also drops the last caller of the deprecated `util._extend`, so the DEP0060 warning no longer appears in the R integration run. Verified on Node 24: 310 unit + integration tests, 8 R integration tests, license check clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

These cannot be separated: Express 5 depends on serve-static 2, which depends on send 1. lib/router/directory-router.js requires `send` directly as well, so pinning it keeps our copy and Express's copy the same. Only one thing in lib/ had to change. send 1.x dropped the `send.mime` re-export it inherited from the old `mime` package and now delegates to `mime-types`, so registering `.R` as text/R has to go through mime-types instead. That also makes mime-types a direct dependency: the mutation only takes effect if we and send end up on the same hoisted instance, and declaring it is what guarantees that rather than leaving it to hoisting luck. Nothing else needed adjusting. The middleware stack uses only app.use with bare functions -- no route path strings, so Express 5's path-to-regexp 8 rewrite doesn't reach us; and none of the APIs Express 5 removed (req.param, res.sendfile, res.json(status, body), app.del) were in use. The one behaviour change the harness caught: `.R` files come back as `text/R; charset=utf-8` where send 0.19 said `charset=UTF-8`. Charset names are case-insensitive per RFC 7231 3.1.1.2, so this is cosmetic, but it is a real difference on the wire and test/integration/static-files.js now pins the new spelling. Verified on Node 24: 310 unit + integration tests, 8 R integration tests, license check clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Two source changes and a large, entirely mechanical regeneration of the committed .js. lib/core/python.ts declared `resolvePython_p` as returning `Q.Promise<PythonEnv>` while implementing it with `async`. TypeScript 6 rejects that outright (TS1064: the return type of an async function must be the global Promise type) -- and it was always a fiction, since an async function returns a native promise no matter what the annotation says. The annotation is now `Promise<PythonEnv>`; the only caller already just `await`s it, so nothing depended on the Q surface. That leaves the `q` import unused, so it goes too. tsconfig.json now sets `target` explicitly. It never did, which meant the emit followed the compiler's default -- and TypeScript 6 moved that default off ES5. Upgrading the compiler therefore rewrote every committed .js file: `__awaiter`/`__generator`/`__spreadArray` downlevel helpers give way to native async/await and spread, and classes stop being IIFE-wrapped constructor functions. That output is correct and is what we want on Node 24, but it should be a decision in the config rather than a side effect of a dependency bump, so ES2022 is now written down. Everything in `engines.node` (>= 18) supports it. Verified on Node 24: 310 unit + integration tests, 8 R integration tests, license check clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Direct bumps: ip-address 9 -> 10 (the only major here; lib/core/iputil.js uses Address4/Address6, fromAddress4 and correctForm, all unchanged, and test/iputil.js covers them), compression 1.8.1, handlebars 4.7.9, morgan 1.12.0, underscore 1.13.8, mocha 11, rewire 9, sinon 22. Transitively, three advisories are closed: * websocket-driver 0.7.4 -> 0.7.5 (critical: resource-limit bypass via message compression). Both faye-websocket and sockjs already allowed 0.7.5; only the shrinkwrap pin was stale. * flatted 3.3.1 -> 3.4.4 (high), likewise already in log4js's range. * sockjs's uuid 8 -> 11, which needs an `overrides` entry because sockjs pins `^8.3.2`. sockjs calls `require('uuid').v4` and nothing else, and uuid 11 still ships that as CommonJS. `npm audit` goes from 25 findings to 5, and all 5 that remain are in development-only tooling (mocha's and nodemon's own trees). Not moved: `q`, whose 2.x line was abandoned in 2015 and which is being removed from this codebase separately; typescript 7 and http-proxy-3 2, both released since this branch was opened and both worth their own change; and @types/node, held at 24.x to match the runtime rather than following `latest` to 26. One test change was required. As of sinon 19 (fake-timers 13), useFakeTimers() fakes process.nextTick and queueMicrotask by default. Q drains its promise queue through nextTick, so its drain callback lands on the fake clock, and clock.restore() throws that queue away without running it -- leaving Q wedged for the rest of the process. The visible effect was 12 failures, only 2 of them in the file that installs the clock; the other 10 were in test/simple-scheduler.js and test/squash-run-as-router.js, which pass in isolation and don't use timers at all. Both call sites now pass `toNotFake: ['nextTick', 'queueMicrotask']`; they only ever wanted control of setTimeout. Verified on Node 24: 310 unit + integration tests, 8 R integration tests, license check clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Rewrites the "Pending upgrade (PR #596)" section of techContext.md as a description of what actually landed, and clears the now-wrong references scattered across five documents: `.nvmrc` v20.17.0 (buildAndPackaging, testingGuide), `express` v4 and `http-proxy` in the dependency table, ip-address "PR #596 takes it to ^10" (coreUtilities), the `proxySocket` note phrased as a prediction (proxyLayer), the "there is no overrides block" claim, the vestigial `engines` note, and a stale test count. Two things are recorded that were expensive to learn and are not visible in the diff: * `nan` and `.nvmrc` have to move in the same commit. `nan` 2.20 does not compile against Node 24's V8 headers, so bumping either alone fails in node-gyp -- which takes down everything that loads posix.node, which is essentially everything. * TypeScript 6 moved its default compile target off ES5, which is what regenerated every committed .js file. tsconfig.json now pins the target so it can't happen unannounced again, and techContext says to check there first if a compiler bump ever produces a huge lib/*.js diff. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`npm ci` emitted five warnings while compiling src/posix.cc, from two separate causes. Four were -Wvla-cxx-extension: `char buf[bufsize]` in GetPwNam, GetPwUid and GetGroupList, and `result_t groups[ngrp]` in GetGroupList. Variable length arrays are not standard C++, only a Clang/GCC extension. Use std::vector and pass .data(), which also moves the allocation off the stack -- _SC_GETPW_R_SIZE_MAX is a runtime value with a 16 KB fallback. The fifth was the known NAN cast on NODE_MODULE(). The file already had a DISABLE_WCAST_FUNCTION_TYPE wrapper for it, but gated on `__GNUC__ >= 8`; Clang reports __GNUC__ as 4, so the macro expanded to nothing, and Clang spells the diagnostic -Wcast-function-type-mismatch rather than -Wcast-function-type. Add a __clang__ branch covering both spellings, prefixed with -Wunknown-warning-option so it stays harmless on Clang versions that know neither. The GCC branch is unchanged. Also refresh the stale Node version in the CLAUDE.md ABI note: .nvmrc has been v24.20.0 since the dependency sweep, not v20.17.0. Verified with a clean `rm -rf build && node-gyp rebuild` (no warnings), a smoke test of all five exports plus the F_* / SEEK_SET constants against the rebuilt .node, and `npm test` (310 passing). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Audit: all five findings were dev-only, and in every case the direct dev dependency was already at its latest release with the advisory in a transitive dep it pins below the fix -- so there was nothing to upgrade to and each needed an override. `npm audit fix` was no help; for the mocha chain it proposed downgrading mocha to 11.3.0, which fixes nothing. - mocha -> diff ^9, serialize-javascript ^7. mocha 11.8.0 is latest (12 is still RC) and pins diff ^7 / serialize-javascript ^6. Its API surface on both is tiny -- diff.createPatch in reporters/base.js, and serializeJavascript(opts, {unsafe, ignoreFunction}) in the parallel-mode worker pool, which this project does not use. Checked the reporter still renders a diff by forcing a string-mismatch assertion. - picomatch ^4, top level. nodemon 3.1.14 is latest and drags in chokidar 3, whose anymatch/readdirp pin picomatch ^2; the last 2.x is the vulnerable one. This has to be top-level: scoping it under nodemon reaches readdirp's edge but not anymatch's, leaving 2.3.1 hoisted and the advisory open. Nothing else in the tree wants picomatch. Verified chokidar still fires add events and honours `ignored`, and that nodemon still restarts on a file change. - nodemon -> brace-expansion ^5.0.9, scoped deliberately: the tree also holds 1.1.18 (eslint, via rewire) and 2.1.4 (mocha), both outside the advisory range, and a global override would move them two majors for no reason. Outdated: qs 6.15.3 -> 6.16.0 was just a stale shrinkwrap pin. TypeScript 6 -> 7 compiles clean and regenerates the committed lib/*.js byte-identically, which was the thing to watch after the 5 -> 6 bump rewrote all of it. http-proxy-3 1 -> 2 is purely additive (new xfwdWsProtoAsHttp and connectTimeout options, function-valued target); its only breaking change is engines node >=18 -> >=22, so raise our own floor to match -- it was >=18 for Express 5, and .nvmrc has been 24.20.0 for a while. Two rows are left and neither is actionable, so they are now documented rather than silently tolerated: - q 1.5.1 -> "2.0.3" is a false positive. q's `latest` dist-tag is 1.5.1; 2.0.3 sits under a `future` tag, and npm 11's outdated reports the highest semver rather than the latest tag. q 2.0.3 was published in January 2015, nearly three years before 1.5.1, and is an abandoned rewrite with different deps and a different API -- "upgrading" would be a functional downgrade. Removing Q is tracked separately. - @types/node is held at 24.x to match the pinned runtime. 26.x compiles clean, but it would type against APIs Node 24 does not have. Verified from a clean tree: rm -rf node_modules build && npm ci (no C++ warnings, no audit findings), npm run build (no drift in committed .js), npm test (310 passing), tools/preflight.sh (exit 0). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

`npm run dev` is broken on a fresh clone: dev/apps/py-hello is a uv project, dev/.gitignore ignores **/.venv/ (correctly -- a venv is a platform-specific build artifact), and nothing creates it. /py-hello/ 500s and the log says only: Error getting worker: Error: ENOENT: no such file or directory, stat '.../dev/apps/py-hello/.venv' Two things were wrong, so fix both. resolvePython_p explains every failure mode it has -- "does not contain bin/python", "Can't execute the specified python binary", "is not a file, directory, or symbolic link" -- except the one people actually hit, where the path simply isn't there and the raw fs error escapes. Catch it and say what the setting expects, including that it will not create a venv for you. This is not dev-only: any deployment whose `python <path>;` points at a venv that was never built, or that moved, gets the bare ENOENT today. Add test/python.js, which resolvePython_p had none of. The two tests for the new message fail against the old build; the other three pin existing behaviour (venv directory, missing bin/python, bare name deferred to PATH). Document the one-time `uv sync --project dev/apps/py-hello` in CLAUDE.md, techContext.md and NEWS, whose dev-mode entry promised "sample R and Python apps, no root required" without mentioning it. Deliberately not automated. A `predev` hook running uv sync was considered and rejected, as was teaching the server to detect a uv project and launch through `uv run`. `uv run` silently creates and populates the venv when it is missing -- verified: deleting .venv and running it reinstalled 41 packages -- which would move dependency resolution, network access, and arbitrary package execution as the `run_as` user into the request path of a running server. Provisioning stays the operator's job; the error message just has to say so. Verified: npm test 315 passing (310 + 5 new), and `npm run dev` serves both /py-hello/ and /r-hello/ at 200 with the venv present, while a moved-aside venv now produces the readable error. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The coverage map listed user-dirs-router.js alongside directory-router.js and local-config-router.js as "covered end-to-end by test/integration/, but not unit-tested." That is true of the other two but not of this one. createRouter_p throws 'shiny-server must be run as root to use the user_dirs directive' (lib/router/config-router.js:99-102) unless the process is root, and the integration harness runs non-root, so user_dirs is unreachable from that tier -- it has no coverage at all. Nothing under test/ mentions the directive; it appears only in the shipped samples and the schema. Split it into its own entry, record why the integration tier cannot reach it, and note that a unit test is still feasible: test/nested-locations.js:25 already rewires checkPermissions to a no-op for exactly this reason. Also drop python.ts from the zero-coverage list and give it a table row, since test/python.js was added on this branch, and note the branch's 315-test count next to master's 310. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

jcheng5 changed the title ~~Replace http-proxy with http-proxy-3 and upgrade all deps~~ Upgrade Node.js to v24 and all npm dependencies Apr 14, 2026

This was referenced Aug 29, 2026

Make the server startable in-process (extract lib/server-init.js) #598

Merged

Test harness: integration tier, refactor-protection unit tests, and CI #599

Merged

jcheng5 and others added 5 commits August 29, 2026 15:19

jcheng5 force-pushed the replace-http-proxy-with-http-proxy-3 branch from 5592fd0 to ee63175 Compare August 29, 2026 22:42

jcheng5 and others added 5 commits August 29, 2026 15:46

jcheng5 merged commit 46f27df into master Aug 30, 2026
4 checks passed

jcheng5 deleted the replace-http-proxy-with-http-proxy-3 branch August 30, 2026 00:26

Navigation Menu

Repository navigation

Upgrade Node.js to v24 and all npm dependencies - #596

Upgrade Node.js to v24 and all npm dependencies#596
jcheng5 merged 10 commits into
masterfrom
replace-http-proxy-with-http-proxy-3

jcheng5 commented Apr 14, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

jcheng5 commented Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

What actually changed in lib/

Behaviour changes

Security

Deliberately not moved

Verification

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jcheng5 commented Apr 14, 2026 •

edited

Loading

What actually changed in `lib/`