Skip to content

ref(node): Remove two Node <20 leftovers (hrtime.bigint, undici channel refs) - #24183

Merged
chargome merged 1 commit into
getsentry:developfrom
wittachai-as:ref/node-20-leftovers
Sep 7, 2026
Merged

chargome merged 1 commit into
getsentry:developfrom
wittachai-as:ref/node-20-leftovers

Conversation

@wittachai-as

Copy link
Copy Markdown
Contributor

Both notes named the exact condition for their own removal, and both conditions passed when the engine floor moved to Node >=20.19.0 in 4f03438.

createHrTimer()'s TODO was conditioned on dropping Node 8; process.hrtime.bigint() has been available since Node 10.7. getTimeMs() still returns an integer number of milliseconds — BigInt division truncates toward zero and the elapsed delta is never negative, so it yields the same value the previous Math.floor(seconds * 1e3 + nanoSeconds / 1e6) did.

_channelSubs existed only to hold a reference around nodejs/node#42170, which mattered while Node 18.18.0 was supported. It is replaced by the _isInstrumented boolean its own comment proposed. Nothing else in the repo read the array — the only uses were the .length guard and the .push(). The flag is now set before subscribing rather than after, so a re-entrant call cannot double-subscribe; the old .length check only flipped after the first push, making this equal-or-stricter.

Fixes #24182

🤖 Generated with Claude Code

https://claude.ai/code/session_01JGK2ap3HHAy2WuDwoS8yTW

…el refs)

Both were guarded by a condition that has since passed: the engine floor
moved to Node >=20.19.0 in 4f03438.

- anr/worker.ts: `createHrTimer()` now uses `process.hrtime.bigint()`; the
  TODO asking for it was conditioned on dropping Node 8.
- node-fetch/undici-instrumentation.ts: `_channelSubs` only existed to keep
  a reference alive around nodejs/node#42170, which needed Node 18.18.0
  support. Replaced with the `_isInstrumented` boolean its own comment
  proposed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01JGK2ap3HHAy2WuDwoS8yTW
@chargome chargome self-assigned this Sep 7, 2026
@chargome
chargome marked this pull request as ready for review September 7, 2026 14:53
@chargome
chargome requested a review from a team as a code owner September 7, 2026 14:53
@chargome
chargome requested review from logaretm and stephanie-anderson and removed request for a team September 7, 2026 14:53
@chargome

chargome commented Sep 7, 2026

Copy link
Copy Markdown
Member

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 43ea6f7. Configure here.

@chargome
chargome merged commit 9596f42 into getsentry:develop Sep 7, 2026
187 checks passed
@wittachai-as
wittachai-as deleted the ref/node-20-leftovers branch September 7, 2026 15:11
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.

@sentry/node: two notes whose "once we drop Node X" condition has passed — remove the workarounds, or refresh the notes?

3 participants