Skip to content

fix(sentry): pass registerEsmLoaderHooks: false to skip Node 23+ DEP0205 (counter-PR to #800)#802

Merged
kelsonpw merged 1 commit into
mainfrom
fix/dep0205-better
May 22, 2026
Merged

fix(sentry): pass registerEsmLoaderHooks: false to skip Node 23+ DEP0205 (counter-PR to #800)#802
kelsonpw merged 1 commit into
mainfrom
fix/dep0205-better

Conversation

@kelsonpw
Copy link
Copy Markdown
Member

@kelsonpw kelsonpw commented May 16, 2026

Summary

Root-cause alternative to PR #800. Instead of monkey-patching process.emitWarning in bin.ts, pass Sentry's documented registerEsmLoaderHooks: false option to Sentry.init() so @sentry/node-core never calls the deprecated module.register() in the first place.

The DEP0205 warning surfaces on Node.js >= 23 when running npx @amplitude/wizard mcp serve. It originates from @sentry/node-core -> import-in-the-middle ESM loader-hook setup, called from initSentry(). The exact stack:

at Module.register (node:internal/modules/esm/loader:969:3)
at initializeEsmLoader (@sentry/node-core/build/esm/sdk/esmLoader.js:23:20)
at _init (@sentry/node-core/build/esm/sdk/index.js:90:5)
at initSentry (src/lib/observability/sentry.ts:174:18)

Sentry exposes a public, documented option to skip that call: registerEsmLoaderHooks. Verified against @sentry/node-core@10.47.0 (src/sdk/index.ts: if (options.registerEsmLoaderHooks !== false) { initializeEsmLoader() }) and still honored in the latest 10.53.1 — so this approach is forward-compatible.

Why this is better than PR #800

PR #800 (emitWarning patch) This PR (registerEsmLoaderHooks: false)
Approach Suppresses the warning after it fires Prevents the deprecated call from ever happening
Surface Patches a global Node API (process.emitWarning) One option in Sentry.init()
Forward compat Pattern-matches warning text — brittle if Sentry rewords or Node tightens DEP0205 Uses Sentry's public, documented API
Other DEP0205s Hides ALL DEP0205 warnings, masking future legit cases Untouched — they still reach process.on('warning') and stderr
Files touched bin.ts (24 lines, must run before any import) src/lib/observability/sentry.ts (20 lines, inside initSentry)

The wizard does NOT use any of the features that registerEsmLoaderHooks enables:

  • HTTP / fetch tracing patches Node's built-in http / https / undici modules directly — no ESM hook needed
  • MCP servers are wrapped explicitly via wrapMcpServerWithSentry
  • Error capture, breadcrumbs, structured logs, and manual spans all work without ESM loader hooks
  • We don't use any ESM-loaded third-party auto-instrumentation (DB drivers, framework integrations)

So disabling them has zero functional cost.

Test plan

  • pnpm lint clean (prettier + eslint)
  • pnpm test — 4471 / 4471 passing
  • mcp serve cold start emits no DEP0205 on Node 20.20.2, 22.14.0, 24.14.1, 26.0.0
  • Synthetic non-matching DEP0205 warning still emits to stderr AND reaches process.on('warning') listeners — covered by new regression test
  • wizard-abort.ts untouched

New tests (src/lib/observability/__tests__/sentry.test.ts)

  1. passes registerEsmLoaderHooks: false to Sentry.init — pins the option so a future refactor can't silently remove it
  2. does not patch process.emitWarning — other DEP0205 warnings still surface — fires a synthetic DEP0205 via process.emitWarning, asserts it reaches process.on('warning') listeners

Recommended next step

Close #800 in favor of this PR.

Checklist

  • Opened PR with gh pr create after push
  • pnpm lint and pnpm test pass
  • TUI / router / flows.ts / navigation store.ts changes: N/A — observability change only
  • Included /reflect checklist in this PR description or linked it — N/A, single-file root-cause fix
  • If this PR adds, removes, or changes an LLM call site, list the call-site IDs — N/A

Note

Medium Risk
Changes Sentry initialization behavior by disabling ESM loader hook registration, which could reduce auto-instrumentation for ESM-loaded libraries if any are used at runtime. Otherwise it’s a targeted observability change with regression tests covering the new option and ensuring Node warnings still propagate.

Overview
Prevents Node.js 23+ DEP0205 deprecation warnings by passing registerEsmLoaderHooks: false into Sentry.init() in src/lib/observability/sentry.ts, avoiding Sentry’s deprecated module.register()-based ESM loader setup.

Adds regression tests in src/lib/observability/__tests__/sentry.test.ts to pin the new init option and to verify the PR does not globally suppress warnings (a synthetic DEP0205 still reaches process.on('warning')).

Reviewed by Cursor Bugbot for commit db78c44. Bugbot is set up for automated code reviews on this repo. Configure here.

@kelsonpw kelsonpw requested a review from a team as a code owner May 16, 2026 00:11
Root-cause fix for the `[DEP0205] DeprecationWarning: module.register()
is deprecated` warning that surfaces on Node.js >= 23 when running
`npx @amplitude/wizard mcp serve`. The warning originates from
`@sentry/node-core`'s `initializeEsmLoader` (via the transitive
`import-in-the-middle`), which calls the deprecated `module.register()`
for ESM-loader-based auto-instrumentation.

Sentry exposes an official, documented option — `registerEsmLoaderHooks`
— that skips that call entirely. Passing `false` is preferred over
PR #800's `process.emitWarning` monkey-patch because:

- Root cause, not symptom: Sentry never calls the deprecated API, so
  there is no warning to suppress. Nothing in the global warning
  channel is patched, so unrelated DEP0205 warnings (or future
  legitimate ones from other dependencies) still surface to users
  and CI.
- Smaller, scoped surface: 1 option in `Sentry.init()` instead of
  patching `process.emitWarning` at the very top of `bin.ts`.
- Forward-compatible: still honored by latest `@sentry/node-core`
  (10.53.1) and not tied to the exact wording of Node's warning
  string, so it survives Sentry rewording or Node tightening
  deprecation messages on 24/25/26+.

The ESM loader hooks only power auto-instrumentation of *ESM-loaded*
third-party libraries (DB drivers, frameworks loaded as ESM). The
wizard's HTTP/`fetch` tracing patches Node's built-in modules
directly, MCP servers are wrapped explicitly via
`wrapMcpServerWithSentry`, and all error capture, breadcrumbs, logs,
and spans work without ESM loader hooks — so disabling them has no
functional cost.

Verified:
- `pnpm lint` clean
- `pnpm test` (4471 / 4471 passing)
- `mcp serve` cold start emits no DEP0205 on Node 20 / 22 / 24 / 26
- Regression test asserts `registerEsmLoaderHooks: false` is passed
- Companion test asserts `process.emitWarning` is NOT patched —
  unrelated DEP0205 warnings still reach `process.on('warning')`
@kelsonpw kelsonpw force-pushed the fix/dep0205-better branch from 6833669 to db78c44 Compare May 18, 2026 03:50
@kelsonpw kelsonpw removed the request for review from a team May 18, 2026 18:04
@kelsonpw kelsonpw merged commit 370905c into main May 22, 2026
10 checks passed
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