From d5887bdb73a1041f6c6b987e513f1534b6875ee4 Mon Sep 17 00:00:00 2001 From: Mike Lay Date: Mon, 24 Aug 2026 10:07:49 -0700 Subject: [PATCH] fix(server-utils): Stop orchestrion rollup plugin from crashing under Rolldown The plugin's `buildStart` hook assumed the bundler had normalized `external` into a predicate function by the time the hook runs. Rolldown deliberately omits function-typed options from its normalized options (rolldown/rolldown#1041), so `rollupOptions.external` is `undefined` there for every config shape and calling it crashed the build with "rollupOptions.external is not a function". The plugin now captures the raw `external` value in the `options` hook, which both bundlers invoke with the un-normalized input options. `buildStart` keeps using the normalized predicate when the bundler provides one and probes the captured raw value otherwise, so the externalized-modules warning still fires on Rolldown for string, array, RegExp and function configs. String entries match via the shared `externalEntryMatchesModule`, consistent with the esbuild and webpack plugins. Fixes #23450 --- .../src/orchestrion/bundler/rollup.ts | 42 +++++++++++++--- .../test/orchestrion/rollup-plugin.test.ts | 49 +++++++++++++++++++ 2 files changed, 85 insertions(+), 6 deletions(-) create mode 100644 packages/server-utils/test/orchestrion/rollup-plugin.test.ts diff --git a/packages/server-utils/src/orchestrion/bundler/rollup.ts b/packages/server-utils/src/orchestrion/bundler/rollup.ts index 4fd0dee7ab2e..055e3cb49a84 100644 --- a/packages/server-utils/src/orchestrion/bundler/rollup.ts +++ b/packages/server-utils/src/orchestrion/bundler/rollup.ts @@ -1,10 +1,26 @@ import codeTransformer from '@apm-js-collab/code-transformer-bundler-plugins/rollup'; -import type { NormalizedInputOptions, Plugin, PluginContext } from 'rollup'; +import type { ExternalOption, InputOptions, NormalizedInputOptions, Plugin, PluginContext } from 'rollup'; import { instrumentedModuleNames } from '../config'; import type { PluginOptions } from './options'; -import { externalizedModulesWarning, orchestrionTransformOptions } from './options'; +import { externalEntryMatchesModule, externalizedModulesWarning, orchestrionTransformOptions } from './options'; import { resolveOrchestrionRuntimeRequest, SNIPPET_IMPORT_SPECIFIER } from './resolve'; +/** + * Whether a raw (un-normalized) `external` input option marks `name` as + * external. String entries use the shared subpath-aware matching so a + * `'mysql/lib/...'` entry flags `mysql`, consistent with the esbuild and + * webpack plugins. + */ +function rawExternalMatchesModule(external: ExternalOption, name: string): boolean { + if (typeof external === 'function') { + return !!external(name, undefined, false); + } + const entries = Array.isArray(external) ? external : [external]; + return entries.some(entry => + typeof entry === 'string' ? externalEntryMatchesModule(entry, name) : entry.test(name), + ); +} + /** * Rollup plugin that runs the orchestrion code transform on the bundled output. * @@ -26,8 +42,17 @@ export function sentryOrchestrionPlugin(options: PluginOptions = {}): Plugin { const moduleNames = instrumentedModuleNames(options.instrumentations); + // Rolldown omits `external` from the normalized options passed to + // `buildStart` (function-typed options don't cross its Rust/JS boundary — + // rolldown/rolldown#1041), so capture the raw value for the probe below. + let rawExternal: ExternalOption | undefined; + return { ...codeTransformer(orchestrionTransformOptions(options)), + options(inputOptions: InputOptions): null { + rawExternal = inputOptions.external; + return null; + }, // The module-injected snippet imports `@sentry/server-utils` from INSIDE // transformed `node_modules` files. Under isolated installs (pnpm) that bare // specifier doesn't resolve from an instrumented package's location, so when @@ -45,10 +70,15 @@ export function sentryOrchestrionPlugin(options: PluginOptions = {}): Plugin { }, buildStart(this: PluginContext, rollupOptions: NormalizedInputOptions): void { // An externalized dependency never passes through the code transform, so - // its diagnostics_channel calls are silently never injected. By the time - // buildStart runs, Rollup has normalized `external` (string arrays, - // RegExps or user functions) into a single predicate we can probe. - const externalizedModules = moduleNames.filter(name => rollupOptions.external(name, undefined, false)); + // its diagnostics_channel calls are silently never injected. Rollup has + // normalized `external` into a single predicate by the time buildStart + // runs; Rolldown doesn't provide it here at all, so probe the raw value + // captured in the `options` hook instead. + const externalizedModules = moduleNames.filter(name => + typeof rollupOptions.external === 'function' + ? rollupOptions.external(name, undefined, false) + : rawExternal != null && rawExternalMatchesModule(rawExternal, name), + ); if (externalizedModules.length > 0) { this.warn(externalizedModulesWarning(externalizedModules)); } diff --git a/packages/server-utils/test/orchestrion/rollup-plugin.test.ts b/packages/server-utils/test/orchestrion/rollup-plugin.test.ts new file mode 100644 index 000000000000..6002e141a835 --- /dev/null +++ b/packages/server-utils/test/orchestrion/rollup-plugin.test.ts @@ -0,0 +1,49 @@ +import type { InputOptions, NormalizedInputOptions, PluginContext } from 'rollup'; +import { describe, expect, it, vi } from 'vitest'; +import { sentryOrchestrionPlugin } from '../../src/orchestrion/bundler/rollup'; + +type OptionsHook = (this: unknown, inputOptions: InputOptions) => null; +type BuildStartHook = (this: Pick, rollupOptions: NormalizedInputOptions) => void; + +function runBuildStart(inputOptions: InputOptions, normalizedExternal?: NormalizedInputOptions['external']): string[] { + const plugin = sentryOrchestrionPlugin(); + const warn = vi.fn(); + (plugin.options as OptionsHook).call({}, inputOptions); + (plugin.buildStart as BuildStartHook).call({ warn }, { external: normalizedExternal } as NormalizedInputOptions); + return warn.mock.calls.map(call => call[0] as string); +} + +describe('sentryOrchestrionPlugin (rollup) externalized-modules warning', () => { + it('warns via the normalized predicate when Rollup provides one', () => { + const warnings = runBuildStart({}, (source: string) => source === 'express'); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain('express'); + }); + + describe('without a normalized predicate (Rolldown — rolldown/rolldown#1041)', () => { + it('does not crash and stays silent when nothing is externalized', () => { + expect(runBuildStart({ external: ['react'] })).toEqual([]); + expect(runBuildStart({})).toEqual([]); + }); + + it('warns for a raw string entry', () => { + const warnings = runBuildStart({ external: 'express' }); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain('express'); + }); + + it('warns for raw array entries, including subpaths and RegExps', () => { + const warnings = runBuildStart({ external: ['react', 'mysql/lib/index.js', /^pg$/] }); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain('mysql'); + expect(warnings[0]).toContain('pg'); + expect(warnings[0]).not.toContain('react'); + }); + + it('warns via a raw user function', () => { + const warnings = runBuildStart({ external: source => source === 'express' }); + expect(warnings).toHaveLength(1); + expect(warnings[0]).toContain('express'); + }); + }); +});