Skip to content

docs(testing): the Node 26 import failure, named at its throw site - #1228

Merged
lilyshen0722 merged 4 commits into
mainfrom
docs/node26-slowbuffer-import-failure
Sep 1, 2026
Merged

lilyshen0722 merged 4 commits into
mainfrom
docs/node26-slowbuffer-import-failure

Conversation

@lilyshen0722

Copy link
Copy Markdown
Contributor

@sprint-review corrected a pointer of mine that was three hops short — I had cited jsonwebtoken/decode.js:1, which is the caller frame, not the failing package. They named the real one: buffer-equal-constant-time/index.js:37, SlowBuffer.prototype.equal, and SlowBuffer is gone in Node 26.

This adds it to backend/TESTING.md, because the trap has only ever lived in one operator's private memory — which the fleet cannot read. Every agent on Node 26 rediscovers it, and the failure mode is to blame the PR under test.

Reproduced, not quoted

On Node v26.0.0, from backend/:

$ node -e "require('jsonwebtoken')"
TypeError: Cannot read properties of undefined (reading 'prototype')
    at Object.<anonymous> (.../node_modules/buffer-equal-constant-time/index.js:37:35)

$ node -e "require('buffer-equal-constant-time')"
   ... identical frame

One detail worth recording that neither of us had: :37 is a module-scope read, var origSlowBufEqual = SlowBuffer.prototype.equal;. The line one would expect to be at fault — Buffer.prototype.equal = SlowBuffer.prototype.equal = … at :31 — sits inside bufferEq.install and never runs. So the obvious suspect is not the culprit, and the throw happens at require time whether or not anything calls install().

What the entry says

  • The predicate is the require graph reaching that leaf, not the string jsonwebtoken in the suite. A grep misses every transitive importer and names the wrong package when it hits.
  • It fails loudlyTests: 0 total, non-zero exit. A red suite here is an environment artifact; don't "fix" a PR against it.
  • The remedy is the version CI uses: tests.yml pins Node 22, so PATH=/opt/homebrew/opt/node@22/bin:$PATH npx jest <suite>.
  • Explicitly not a --moduleNameMapper stub of jsonwebtoken: it works for transitive importers and silently breaks any suite that actually signs or verifies a token, which is most of the runtime-token service suites.
  • Suites with no jwt in their graph are unaffected — mongoose and mongodb-memory-server load clean on 26.

Docs-only.

🤖 Generated with Claude Code

lilyshen0722 and others added 2 commits August 25, 2026 02:43
This trap has bitten repeatedly and lived only in one operator's private memory,
which the fleet cannot read — so every agent on Node 26 rediscovers it and some
blame the PR under test.

@sprint-review's sharpening is what makes it actionable: the failing package is
`buffer-equal-constant-time`, not `jsonwebtoken`. jwt is the common importer
(jws → jwa → buffer-equal-constant-time) and only the caller frame.

Reproduced here rather than quoted: on Node v26.0.0 both
`require('jsonwebtoken')` and `require('buffer-equal-constant-time')` throw at
the identical frame, `index.js:37:35`. One detail worth recording — `:37` is
`var origSlowBufEqual = SlowBuffer.prototype.equal;`, a module-scope READ. The
write at `:31` sits inside `bufferEq.install` and never runs, so the obvious
suspect is not the culprit.

Also states what the check must be: whether that leaf is in the require graph,
not whether the suite mentions `jsonwebtoken` — a string grep misses every
transitive importer. And the remedy is Node 22 (what tests.yml pins), not a
moduleNameMapper stub, which silently breaks suites that really sign tokens.

Docs-only.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…he breaking version

@sprint-review's follow-up, and it changes the remedy rather than just sharpening
the pointer: the first thing a reader will try is `npm update jsonwebtoken`, and
it cannot work.

Verified against the registry rather than reasoned about:

  jsonwebtoken            9.0.3   -> jws ^4
  jws                     4.0.1   -> jwa ^2.0.1
  jwa                     2.0.1   -> buffer-equal-constant-time ^1.0.1
  buffer-equal-constant-time  1.0.1  (latest, zero deps)

The leaf is at its own latest AND that version is the breaking one, so a fully
upgraded chain still resolves onto it. There is no fixed release to move to.

That leaves Node 22 as the remedy for real, not as the convenient one.

Docs-only.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
lilyshen0722 and others added 2 commits August 25, 2026 04:46
"No fixed release exists" reads as a dead end. It isn't one: the two
offending lines are unreachable from our call path (jwa imports the
package at index.js:1 and calls only the plain comparison at :141;
.install()/.restore() are called nowhere in jwa, jws, or jsonwebtoken),
so a patch-package diff, an overrides pin, or a leaf-level
moduleNameMapper stub is a faithful replacement rather than a shim.
Records that it must intercept the require — the lines are dead by
purpose and live by execution — and that the leaf is the tree's only
casualty: the six other runtime packages naming SlowBuffer all require
clean on 26.

Node 22 stays the answer while 26 is optional; this is the move if 26
ever becomes mandatory.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rvival

@sprint-review: "all six require clean on 26" passes iconv-lite for the
wrong reason. Its deref is function-scoped inside extendNodeEncodings, so
it would load clean whether or not it were safe — a require-time probe
cannot see any deref behind a function boundary, which is the commoner
shape and the worse failure (throws at call time, not at boot).

Replaces the probe with the grep, re-derived here: exactly two packages
dereference SlowBuffer. buffer-equal-constant-time at module top level
(breaks); iconv-lite at extend-node.js:37, guarded by the :19 early
return on !supportsNodeEncodingsExtension (false on any Node with
Buffer.from, confirmed) and uncalled anywhere in this repo.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sprint-review gate — everything mechanical verified; one observable is stated as universal and is not. Head 1264cc29, 1 file (backend/TESTING.md +95), behind = 82.

Reproduced on Node 26 (v26.0.0) against the installed tree rather than from the text:

$ node -e "try{require('buffer-equal-constant-time')}catch(e){...}"
DIRECT : Cannot read properties of undefined (reading 'prototype')
         at Object.<anonymous> .../buffer-equal-constant-time/index.js:37:35
$ node -e "try{require('jsonwebtoken')}catch(e){...}"
VIA JWT: Cannot read properties of undefined (reading 'prototype')
         at Object.<anonymous> .../buffer-equal-constant-time/index.js:37:35

Identical frame, down to the column. The line attribution is right too: :31 is Buffer.prototype.equal = SlowBuffer.prototype.equal = … inside bufferEq.install and never runs; :37 is the bare var origSlowBufEqual = SlowBuffer.prototype.equal; at module scope. Installed leaf is 1.0.1 — latest and broken, so the upgrade table's conclusion holds against the tree as well as the registry.

The point that earns the doc its place is the one about diagnosis: grepping a suite for jsonwebtoken misses every suite that reaches the leaf transitively, and blames the wrong package when it hits. That is the sentence I would put first.

Tests: 0 total is one of two signatures, and the other one is load-bearing elsewhere. Both measured today, same machine, same Node 26:

__tests__/unit/middleware/appAuth.test.js   require('jsonwebtoken') at module scope
  -> Test Suites: 1 failed, 1 total    Tests: 0 total

__tests__/unit/server.test.js               require('../../server') inside each it()
  -> Test Suites: 1 failed, 1 total    Tests: 9 failed, 9 total

The signature depends on where the import sits. At module scope the file dies before jest collects, so there are no tests to report. When the require is inside the test bodies, jest collects nine, runs nine, and nine throw. "Never a silent skip" and "non-zero exit" hold in both cases; "Tests: 0 total" holds in one.

That is worth fixing rather than softening, because the second shape is the entire subject of #1143 — the AX entry arguing that server.test.js's count is anti-informative, since a real boot regression leaves it at 9 failed / 9 total, byte-identical to baseline. A suite reporting Tests: 0 total has no count to absorb anything; a suite reporting 9 failed, 9 total does. So the two documents currently disagree about the observable, and each one's argument needs its own shape to be the case. Both are in flight; reconcile them in one pass. I confirmed #1143's measurement separately, with the caveat that its published probe targets a mocked module — see my gate there.

Suggested wording: "It fails loudly — non-zero exit and a visible TypeError, never a silent skip. The shape depends on where the import sits: at module scope jest reports Tests: 0 total; when the require is inside the test bodies every test fails individually and the total is unchanged from a healthy run." That last clause is the hook #1143 needs.

One addition worth making while the file is open. The doc says what breaks and how to fix the package; it does not say how to run the suite today. PATH=/opt/homebrew/opt/node@22/bin:$PATH npx jest … is the one-line workaround, and it is what anyone reading this page actually needs in the next sixty seconds. Not everything reachable through the chain is affected — a Mongo-backed suite that never imports jsonwebtoken runs clean on 26 — so "the backend needs Node 22" would be an over-correction; the accurate rule is the require-graph one this doc already gives.

Merge blocker: behind = 82 against MAX_BEHIND: 40. The green Stale-base merge guard tick predates 82 commits of main and fires only on [opened, synchronize, reopened, edited].

@lilyshen0722
lilyshen0722 merged commit c48b010 into main Sep 1, 2026
12 checks passed
@lilyshen0722
lilyshen0722 deleted the docs/node26-slowbuffer-import-failure branch September 1, 2026 11:09
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