Skip to content

node: withhold --enable-source-maps across the whole Node 26.x assert regression band #784

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
colinhacks merged 1 commit into from
Aug 28, 2026
Merged

node: withhold --enable-source-maps across the whole Node 26.x assert regression band #784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 35 additions & 22 deletions crates/nub-core/src/node/flags.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,27 +15,38 @@ use super::version::NodeVersion;
/// Flags Nub injects on EVERY supported Node version where they are safe.
/// `--enable-source-maps` has existed since Node 12.12, so it is structurally
/// available across the whole 18.19+ range — BUT it is gated out of the
/// `source_maps_safe`-false band (Node 26.2.x; see that predicate).
/// `source_maps_safe`-false band (Node 26.0.0–26.7.x; see that predicate).
/// (`--disable-warning` is NOT here — it doesn't exist on Node 18.x / 20.0–20.10
/// and is gated below; injecting it there is a hard "bad option" / "not allowed
/// in NODE_OPTIONS" error, which broke the compat tier on those versions.)
const ALWAYS_INJECT: &[&str] = &["--enable-source-maps"];

/// Whether nub may inject `--enable-source-maps` on this Node version.
///
/// Node **26.2.x specifically** has a regression where, with source maps enabled,
/// a no-message `assert.ok(false)` / `assert(false)` rethrows as a `TypeError`
/// instead of the expected `AssertionError` (the source-map remapping path
/// mis-constructs the error for the no-message form). Empirically isolated to the
/// 26.2 patch band: Node 18.19 / 20 / 22 / 24 / 25 and 26.1 are all clean, and a
/// future 26.3 is expected clean too. So nub withholds the injection ONLY on
/// 26.2.x — source maps are unavailable there (a cosmetic loss: stack traces are
/// not remapped), which is far better than corrupting the type of a thrown
/// AssertionError. Verified on real Node 26.2.0:
/// The whole **Node 26.x** line released so far regresses (nodejs/node#63169):
/// with source maps enabled and no source map for the file, the remapping path's
/// `getErrorSourceLocation` returns `undefined`, so a no-message
/// `assert(false)` / `assert.ok(false)` / `assert.strict(false)` throws
/// `TypeError [ERR_INVALID_ARG_TYPE]` ("The \"message\" argument …") instead of
/// the expected `AssertionError`. Verified on real 26.0.0 / 26.3.0 / 26.5.1 /
/// 26.7.0; 25.x is clean (25.9.0 yields an `AssertionError` with a degraded
/// message, which is the right TYPE). This band was previously — and wrongly —
/// documented as 26.2-only.
///
/// Fixed on Node `main` by b5d37cd4 (nodejs/node#63215, 2026-08-20) in
/// `lib/internal/errors/error_source.js`, which is in NO release yet (latest is
/// v26.7.0, 2026-08-05). 26.8.0 is therefore the first release that can carry
/// the fix, so the band is closed there — a **stopgap pending that release**.
/// RE-VERIFY when 26.8.0 ships, and move the boundary up if it does not carry
/// the fix:
/// `node --enable-source-maps -e 'try{require("assert").ok(false)}catch(e){console.log(e.constructor.name)}'`
/// prints `TypeError`; without the flag it prints `AssertionError`.
/// must print `AssertionError`, not `TypeError`.
///
/// The trade-off is unchanged: withholding costs only stack-trace remapping (a
/// cosmetic loss), while injecting corrupts the TYPE of a thrown AssertionError,
/// which breaks `node:test` assertions outright.
fn source_maps_safe(node_version: &NodeVersion) -> bool {
!(node_version.major() == 26 && node_version.minor() == 2)
!(node_version.major() == 26 && *node_version < NodeVersion::new(26, 8, 0))

@pullfrog pullfrog Bot Aug 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The 26.8.0 upper bound is unconfirmed upstream, and it fails in the unsafe direction. b5d37cd4 merged to Node main on 2026-08-20, but v26.x-staging was at 2026-08-18 and nodejs/node#63215 carries no backport marker or milestone — so 26.8.0 is verifiably the earliest release that could carry the fix, not a confirmed one. If it ships without it, nub silently re-injects and the type corruption returns, with no test and no CI leg (CI pins Node 24) to notice.

Technical details 
# The stopgap's upper bound assumes the fix rather than verifying it

## Affected sites
- `crates/nub-core/src/node/flags.rs:49` — `!(node_version.major() == 26 && *node_version < NodeVersion::new(26, 8, 0))` opens the band at 26.8.0 unconditionally.
- `crates/nub-core/src/node/flags.rs:40-43` — "RE-VERIFY when 26.8.0 ships, and move the boundary up if it does not carry the fix" is a note to a human, not a mechanism. Nothing fires when 26.8.0 lands.

## Required outcome
- If 26.8.0 ships without `b5d37cd4`, nub must not silently re-enable `--enable-source-maps` on it.

## Suggested approach (optional)
- The PR's own asymmetry argument points one way: withholding costs stack-trace remapping (cosmetic), injecting corrupts the TYPE of a thrown `AssertionError` (breaks `node:test`). Gating on the *verified-good* set rather than the *assumed-good* set means withholding across all `26.x` until a release is checked, then narrowing. Both shapes need a follow-up nub release, but only one of them fails safe.
- If you'd rather keep the optimistic bound, a tracking issue is the minimum forcing function, since neither the unit test nor CI will ever exercise a real Node 26.

## Open questions for the human
- Is 26.x the Current line? If so, commits on `main` typically sweep into the next minor by default and the optimistic bound is probably right in practice — this is a risk-posture call, not a proven defect.

}

/// `--disable-warning=ExperimentalWarning` (suppresses Node's experimental-feature
Expand Down Expand Up @@ -353,8 +364,9 @@ pub fn compute_inject_flags(
let mut flags: Vec<&str> = Vec::new();

for &flag in ALWAYS_INJECT {
// --enable-source-maps is withheld on Node 26.2.x (see `source_maps_safe`):
// there it turns a no-message AssertionError into a TypeError.
// --enable-source-maps is withheld below Node 26.8 on the 26.x line (see
// `source_maps_safe`): there it turns a no-message AssertionError into a
// TypeError.
if flag == "--enable-source-maps" && !source_maps_safe(&node_version) {
continue;
}
Expand Down Expand Up @@ -1124,12 +1136,13 @@ mod tests {
}

#[test]
fn source_maps_withheld_only_on_26_2_band() {
// Node 26.2.x regresses: with --enable-source-maps, a no-message
// assert.ok(false) rethrows as TypeError instead of AssertionError. nub
// withholds the injection there and ONLY there — 24 / 25 / 26.1 and a
// future 26.3 are clean. (Verified empirically on real Node 26.2.0.)
for ver in [v(24, 0, 0), v(25, 8, 0), v(26, 1, 0), v(26, 3, 0)] {
fn source_maps_withheld_across_the_whole_26_x_regression_band() {
// Every Node 26.x below 26.8 regresses (nodejs/node#63169): with
// --enable-source-maps, a no-message assert.ok(false) throws TypeError
// [ERR_INVALID_ARG_TYPE] instead of AssertionError. The band's EDGES are
// what this pins — 25.9.0 below it, 26.8.0 (first release that can carry
// the upstream fix b5d37cd4) and 27.0.0 above it.
for ver in [v(24, 0, 0), v(25, 9, 0), v(26, 8, 0), v(27, 0, 0)] {
assert!(
source_maps_safe(&ver),
"source maps must be safe to inject on {ver:?}"
Expand All @@ -1140,8 +1153,8 @@ mod tests {
"--enable-source-maps must inject on {ver:?}"
);
}
// The affected band: every 26.2.x patch is gated out.
for ver in [v(26, 2, 0), v(26, 2, 5)] {
// The affected band: 26.0.0 through 26.7.x, inclusive.
for ver in [v(26, 0, 0), v(26, 2, 0), v(26, 7, 0)] {
assert!(
!source_maps_safe(&ver),
"source maps must be withheld on {ver:?}"
Expand Down
3 changes: 0 additions & 3 deletions crates/nub-core/src/node/version.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,6 @@ impl NodeVersion {
pub(crate) fn major(&self) -> u64 {
self.0.major
}
pub(crate) fn minor(&self) -> u64 {
self.0.minor
}

/// The minimum Node version Nub supports at all. Below this, Nub
/// hard-errors before spawning — the user must upgrade Node or run
Expand Down
2 changes: 1 addition & 1 deletion site/content/docs/runtime/typescript.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,4 +130,4 @@ Error: kaboom

The stack frame reports `app.ts:2:8` — the line in your source, not the transpiled output. To turn it off, pass `--no-enable-source-maps`.

Source maps are injected on every supported Node version except the **26.2.x** patch band, where a Node regression makes a no-message `assert(false)` rethrow as a `TypeError` instead of an `AssertionError` when source maps are on. On 26.2.x Nub withholds the flag, so stack traces there are not remapped. Every other version (18.19 through 26.1, and 26.3+) gets source maps.
Source maps are injected on every supported Node version except **26.0 through 26.7**, where a Node regression ([nodejs/node#63169](https://github.com/nodejs/node/issues/63169)) makes a no-message `assert(false)` throw a `TypeError` instead of an `AssertionError` when source maps are on. Nub withholds the flag across that band, so stack traces are not remapped there. Every other version gets source maps: 18.19 through 25.x, and 26.8 onward, where the upstream fix ships.

@pullfrog pullfrog Bot Aug 22, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

"26.8 onward, where the upstream fix ships" states as settled fact something the code comment two files over hedges as a stopgap pending re-verification. The fix is on Node main and not confirmed in any release line yet, so this reads as a promise the docs may have to walk back. Something closer to "26.8 onward, once the upstream fix reaches a release" matches the evidence.

8 changes: 5 additions & 3 deletions wiki/research/nub-v0.5-augmentation-regressions.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,13 @@ One `typeof` probe against a lazy undici-backed global pulls 227 modules into ev

### Cause 2 — `--enable-source-maps` always injected

Injecting the flag on every run puts each invocation on Node's source-map error path, which changes what a message-less assertion throws. Accepted as-is.
Injecting the flag on every run puts each invocation on Node's source-map error path, which changes what a message-less assertion throws. Gated off across the affected Node 26 band (see the reversed verdict below).

- **Tests:** ~5 (assert ×2, source-map-enable, es-module-cjs-named-error; also the 26.x TypeError).
- **What breaks:** nub injects `--enable-source-maps` on every run (`flags.rs`, `ALWAYS_INJECT`). Node's error path bails when a file has no source map, so (a) on Node 26 a no-message `assert.ok(false)` throws **`TypeError` instead of `AssertionError`** — breaks `catch (e) { e instanceof assert.AssertionError }`; (b) on Node 24.9–25.x the assert *message* degrades from the source expression to `false == true`; (c) a child spawned without the flag still gets remapped stacks.
- **Landed:** source-maps injection predates the June benchmark — the 26.2 gate for it is from **2026-06-11**, so the behavior is **old / pre-existing**, not part of recent growth. The 26.x TypeError only became visible as Node 26 shipped and became the recommended line.
- **VERDICT — accept, no fix (2026-07-24).** The bug is narrow: only a bare `assert.ok(false)`/`assert(false)` with **no message** (asserts with a message, `strictEqual`, and plain throws are all fine). A 26.x-wide gate was briefly landed then **dropped** — disabling TS stack-trace remapping for every Node-26 user to fix that one edge case, whose workaround is passing a message, was the wrong trade. Not documented, not filed upstream, by decision. The `source_maps_safe` gate stays as-is (26.2-only, unrelated).
- **Landed:** source-maps injection predates the June benchmark — the original 26.2-only gate for it is from **2026-06-11**, so the behavior is **old / pre-existing**, not part of recent growth. The 26.x TypeError only became visible as Node 26 shipped and became the recommended line.
- **VERDICT — REVERSED 2026-08-21: gate the whole released 26.x band.** The 2026-07-24 call below accepted the divergence and left `source_maps_safe` at 26.2-only. Two facts overturned it. First, the band was mis-scoped: the regression is [nodejs/node#63169](https://github.com/nodejs/node/issues/63169) and it affects **every released 26.x** — reproduced on 26.0.0, 26.3.0, 26.5.1 and 26.7.0 — so a 26.2-only gate protected almost nobody while the docs claimed 26.1 and 26.3+ were clean. Second, the failure is a wrong error TYPE, not a degraded message, so it breaks `node:test` and any `instanceof assert.AssertionError` check from outside the assertion, where "pass a message" is not a workaround the caller owns. Upstream fixed it on `main` in b5d37cd4 ([nodejs/node#63215](https://github.com/nodejs/node/pull/63215), 2026-08-20), unreleased as of v26.7.0. `source_maps_safe` now withholds the flag for `26.0.0 <= v < 26.8.0` as a stopgap; re-verify when 26.8.0 ships. 25.x is clean (25.9.0 throws an `AssertionError` with the degraded `false == true` message — right type, worse text).
- **Superseded verdict — accept, no fix (2026-07-24).** The bug was judged narrow: only a bare `assert.ok(false)`/`assert(false)` with **no message** (asserts with a message, `strictEqual`, and plain throws are all fine). A 26.x-wide gate was briefly landed then **dropped**, on the reasoning that disabling TS stack-trace remapping for every Node-26 user to fix one edge case was the wrong trade. That reasoning rested on the mis-scoped band and on treating the type corruption as cosmetic.

### Cause 3 — Default `NODE_COMPILE_CACHE` lacks provenance

Expand Down Expand Up @@ -131,3 +132,4 @@ That left zero automated compat signal for the whole June→July window in which
Every revision to this document, with the date and what changed.

- 2026-07-24 — Initial write-up. 70 regressions → 9 root causes with git provenance. Key finding: none introduced by/since the benchmark; all landed 2026-06-03…07-09 during v0.1→v0.5 development; causes 2/3/4/9b predate the June benchmark, the rest are the 53→70 growth. Disambiguated the 07-23 fix "revert" (an editor discard) from code regressions.
- 2026-08-21 — **REVERSAL** on Cause 2. The `--enable-source-maps` regression band was mis-scoped as 26.2-only; it is [nodejs/node#63169](https://github.com/nodejs/node/issues/63169) and covers every released 26.x (reproduced on 26.0.0 / 26.3.0 / 26.5.1 / 26.7.0). The 2026-07-24 "accept, no fix" verdict is superseded: `source_maps_safe` now withholds the flag for `26.0.0 <= v < 26.8.0`, pending the upstream fix b5d37cd4 ([nodejs/node#63215](https://github.com/nodejs/node/pull/63215)) reaching a release.
Loading