fix(node): Skip registration-only instrumentations in the runtime loader - #24240
Merged
Conversation
Contributor
size-limit report 📦
|
mydea
marked this pull request as ready for review
September 9, 2026 11:41
mydea
requested review from
JPeer264 and
isaacs
and removed request for
a team
September 9, 2026 11:41
mydea
added a commit
that referenced
this pull request
Sep 10, 2026
…he runtime warning (#24242) The runtime loader emitted the always-on `[Sentry] @sentry/server-runtime-injection was bundled ... loads uninstrumented` warning for **any** `TypeError` thrown while transforming a module. That equated "a transform threw a TypeError" with "the transformer was stripped by a bundler", so an unrelated per-module failure was reported as a bundling problem — pointing users at externalizing the package, which can itself break other setups. This makes the warning honest and self-diagnosing: - **Include the underlying error** in the message, so the reader can see the actual cause instead of a hardcoded diagnosis they can't verify (`debug: true` still logs the full error/stack). - **Only claim "bundled" for the transform pipeline itself going missing** — `parse`/`generate is not a function`, the fingerprint of a bundler tree-shaking the vendored meriyah/astring parser, which fails every module the same way — guarded by nothing having been instrumented yet (a transformer that already instrumented something is provably not stripped). Any other transform `TypeError` (e.g. `transform is not a function`) now gets a scoped `Could not instrument <module> (...)` message that points at reporting it, not changing the build. _Root cause of the misclassification_: the callback branched on `error instanceof TypeError` alone. The systemic (stripped-transformer) case has a distinct signature (`parse`/`generate is not a function`), so classifying on that — with the success marker as a secondary guard — separates it from isolated per-module failures with a reasonable success rate. A message-based heuristic is the pragmatic fix here; a follow-up upstream change to the transformer (`nodejs/orchestrion-js`) to throw typed/coded errors would let this classification be exact rather than string-matched. Related to #24240, which removes the specific registration-only cause that surfaced this; this hardens the warning for any remaining/future cause. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
isaacs
approved these changes
Sep 11, 2026
isaacs
left a comment
isaacs
left a comment
Member
There was a problem hiding this comment.
One suggestion to consider, but this LGTM. Good find, correctly fixed. 👍
| * integrations statically, so it does not need it. | ||
| */ | ||
| export const SENTRY_RUNTIME_INSTRUMENTATIONS: InstrumentationConfig[] = SENTRY_INSTRUMENTATIONS.filter( | ||
| config => config.transform !== MODULE_REGISTRATION_TRANSFORM, |
Member
There was a problem hiding this comment.
As this is written, it filters out anything that isn't using the specific MODULE_REGISTRATION_TRANSFORM, but I think in practice, it should be just config.transform === undefined, because any named transform will have the same effect, right? Otherwise, if we add some other named transform someday, we're back to the same false alarm warning.
Suggested change
| config => config.transform !== MODULE_REGISTRATION_TRANSFORM, | |
| config => config.transform === undefined, |
Member
Author
There was a problem hiding this comment.
right, good call, adjusted!
|
|
||
| // ...while every other config is preserved, unchanged and in order. | ||
| expect(SENTRY_RUNTIME_INSTRUMENTATIONS).toEqual( | ||
| SENTRY_INSTRUMENTATIONS.filter(c => c.transform !== MODULE_REGISTRATION_TRANSFORM), |
Member
There was a problem hiding this comment.
This test is just a copy of the same implementation, so it's not really doing a lot. Can we test that the properties we actually care about are set the way we need them to be?
Registration-only orchestrion configs (native-channel libraries — ai v7, ioredis, @redis/client, mysql2, mongoose) carry a custom transform wired into the bundler plugins only. The runtime loader (`@sentry/server-runtime-injection` `register`) has no custom transforms, so transforming these modules threw `TypeError: transform is not a function`, which the diagnostics callback misreported as the always-on "`@sentry/server-runtime-injection` was bundled ... loads uninstrumented" warning — even though the libraries are correctly instrumented via their native channel (`setupOnce` / `waitForTracingChannelBinding`). Exclude registration-only configs from the runtime instrumentation set (`SENTRY_RUNTIME_INSTRUMENTATIONS`). This is lossless: at runtime the snippet would only trigger a no-op subscription to `orchestrion:*` channels these versions never publish. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- server-utils unit test: `SENTRY_RUNTIME_INSTRUMENTATIONS` drops every registration-only config and keeps the rest in order, and documents the affected native-channel modules (`@redis/client`, `ai`, `ioredis`, `mongoose`, `mysql2`), asserting the exclusion is per-config not per-module. - node-integration-test: with the runtime loader active, loading a native-channel library (`mysql2` >= 3.20) emits no "transform is not a function" / "server-runtime-injection was bundled" warning (`ensureNoErrorOutput`). This fails against the unfiltered set and passes with the fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`.toContain([...])` checks for the whole array as a single member, so it never matched. Use `arrayContaining` so the assertion documents the known native-channel modules without breaking when another such library is added. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
mydea
force-pushed
the
fn/fix-orchestrion-register
branch
from
September 11, 2026 07:42
4822bef to
4e66dcb
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Registration-only orchestrion configs — native-channel libraries such as
aiv7,ioredis,@redis/client,mysql2andmongoose— carry the customMODULE_REGISTRATION_TRANSFORM. That transform is wired into the bundler plugins only (orchestrion/bundler/moduleInjectedTransform.ts, viabundler/options.ts'scustomTransforms); the runtime loader (@sentry/server-runtime-injection'sregister) passes no custom transforms toinitialize(). So when one of these modules loads through the runtime hook,getTransformerreturns a transformer whose.transformis undefined and the loader throwsTypeError: transform is not a function. The diagnostics callback treats anyTypeErroras a stripped/bundled transformer and emits the always-on warning:…even though nothing is wrong: these libraries publish their own tracing channels and are instrumented via
setupOnce()/waitForTracingChannelBinding, independently of the module-registration snippet.This excludes registration-only configs from a new
SENTRY_RUNTIME_INSTRUMENTATIONSset used by the runtime loader; the bundler keeps the fullSENTRY_INSTRUMENTATIONS. Skipping them at runtime is lossless rather than a workaround: the registration snippet only firesorchestrion.module-injected, which drives theorchestrion:*subscription these native versions never publish (a no-op at runtime). The snippet earns its keep only on the bundler path (e.g. bundler-only SDKs like@sentry/cloudflarethat discover a loaded module via that event);@sentry/noderegisters its integrations statically.Root cause: the
sentryModuleRegistrationcustom transform used by registration-only configs is registered inorchestrion/bundler/*but never in the runtime path, so the runtime code transformer cannot apply it.Noticed this here: #24228
Tests:
server-utilsunit test —SENTRY_RUNTIME_INSTRUMENTATIONSdrops every registration-only config and keeps the rest, and asserts the exclusion is per-config, not per-module.mysql2≥ 3.20) with the runtime loader active emits no transformer-unavailable warning (viaensureNoErrorOutput); it fails against the unfiltered set and passes with this change.🤖 Generated with Claude Code