Repository navigation
Conversation
`@cloudflare/vite-plugin` pre-bundles the worker dependencies in `vite dev`, and the Vite dependency optimizer runs no Vite plugins. So the orchestrion transform never ran on instrumented packages, and their spans were missing. The orchestrion Vite plugin now adds the transform to `optimizeDeps` of server environments in `serve`: a Rolldown plugin on Vite 8 and an esbuild plugin on Vite 6 and 7. Both resolve the snippet import to the ESM entry of `@sentry/server-utils`, like the build does. The cloudflare variant of the `hono-4` e2e app now also runs its tests against `vite dev`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
size-limit report 📦Compared: b11dd91 (PR test merge using develop at 609a0cc) Against: a6c28d0 (develop) · base run #63865
|
Optional E2E testsSome optional checks were not successful. They do not block the merge. 4 not successful, 61 successful. See the run.
Successful checks
|
isaacs
left a comment
There was a problem hiding this comment.
The design is sound for its target. The main problem is scope. It applies to every server environment of every SDK that uses the shared Vite plugin, not only cloudflare.
| if (isDuplicate()) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
This is function is gonna cause a crash if it's used in a node bundle with vite.
On Vite 6 and 7, the dependency optimizer uses platform: 'node' for any server environment that is not webworker (the default ssr environment of every Node SSR framework). For that platform, Vite adds this banner to every optimized file:
import { createRequire } from 'module';const require = createRequire(import.meta.url);The new esbuild plugin makes the optimizer bundle @sentry/server-utils (and @sentry/core) into the pre-bundled dependency.
Two lines in depOptimizerEsbuildPlugin cause this. Line 112 (upstream.setup(build)) adds the transform, which puts an import of @sentry/server-utils into each instrumented module. Line 107 then resolves that import to a file path (toEsmEntry(resolved.path)), so esbuild bundles the file instead of leaving the import external. The @sentry/server-utils ESM build has its own top-level import { createRequire } from "module", and esbuild cannot rename around a text banner. The result is a file that doesn't parse:
vite 6.4.3 esm load error: Identifier 'createRequire' has already been declared
vite 6.4.3 cjs load error: Identifier 'createRequire' has already been declared
vite 7.3.6 esm load error: Identifier 'createRequire' has already been declared
vite 7.3.6 cjs load error: Identifier 'createRequire' has already been declared
vite 8.3.1 esm runs: x channel fired: true
vite 8.3.1 cjs runs: x channel fired: true
Cloudflare is not affected, because @cloudflare/vite-plugin sets platform: 'neutral', which has no banner. With platform: 'neutral' the Vite 6/7 ESM case loads and fires the channel.
Suggestion: scope the dev optimizer plugins to the case this PR targets. Two options:
- Minimal: don't register the resolver or the transform when the optimizer bundles for Node. Vite passes the platform it picked to esbuild as
build.initialOptions.platform, sosetup(line 91) can return before it reaches lines 96-112. The existing duplicate-instance check here already returns early in the same place, so the new check can go next to it:
| if (isDuplicate()) { | |
| return; | |
| } | |
| // Vite 6/7 add a `createRequire` banner for `platform: 'node'`, which | |
| // collides with the `createRequire` import in the bundled SDK. | |
| if (build.initialOptions.platform === 'node') { | |
| return; | |
| } | |
| if (isDuplicate()) { | |
| return; | |
| } |
- Cleaner: add an opt-in option to
PluginOptionsthatsentryCloudflareVitePluginsets, so the other SDKs keep today'sservebehavior. The PR title saysfix(cloudflare), and the comment at lines 205-207 names@cloudflare/vite-pluginas the reason. An opt-in would make the change match that scope. It would also avoid the side effects on Node SSR envs. This option touches three places: the new field inPluginOptions(packages/server-utils/src/orchestrion/bundler/options.ts), theservebranch at line 208 ofvite.ts(returnnullwhen the option is off), and thesentryOrchestrionPlugin({ ... })call atpackages/cloudflare/src/vite/index.tsline 88.
There was a problem hiding this comment.
I'm gonna check. I like the opt-in method.
| const plugin = vitePlugin(); | ||
| // Vite 6 passes no context, Vite 8 passes `rolldownVersion`. | ||
| context?: { meta: { rolldownVersion?: string } }, | ||
| plugin = vitePlugin(), |
There was a problem hiding this comment.
nit: this now takes 5 positional parameters. Might be time for an options object?
There was a problem hiding this comment.
Yeah that's actually better! I'll change it
Good that you brought that up. I actually assumed there might be cases where it is applicable in Node as well. Since on plain Vite+Node it's already working we can do the opt-in as you suggested. |
part of #25765
part of JS-4010
Description
This runs the orchestrion transform in the Vite dependency optimizer, so integrations that are added at build time (for example
honoandgraphql) also send spans invite devon Cloudflare.@cloudflare/vite-pluginpre-bundles the Worker dependencies intonode_modules/.vite/deps_*, and before, the transform did not run on these files. The plugin adds a Rolldown plugin for Vite 8 and an esbuild plugin for Vite 6 and 7.The new
transformOptimizedDepsplugin option turns this on, andsentryCloudflareVitePluginsets it. Node servers do not need it, because the runtime hook instruments the dependencies that Node loads.The Cloudflare variant of the
hono-4e2e app now also runs its tests invite dev.Known issues
optimizeDepsis still not instrumented invite dev: its module id ends with?v=<hash>, and the file matcher of@apm-js-collab/code-transformer-bundler-pluginsdoes not remove the query (Vite plugin does not transform modules whose id has a query (?v=<hash> in vite dev) apm-js-collab/code-transformer-bundler-plugins#52).Sentry traces
Before (11.9.0),
/graphqlinvite dev, nographqlspans: https://sentry-sdks.sentry.io/explore/traces/trace/9221590d886f4648b401eded8a996046/After,
/graphqlinvite dev, withgraphqlspans: https://sentry-sdks.sentry.io/explore/traces/trace/2aea20834d3d4fa38e39d37166273e14/Before (11.9.0),
/hono/users/1invite dev, span nameGET /hono/users/1: https://sentry-sdks.sentry.io/explore/traces/trace/b8e5b8dba83b4b5aba45bdfdf40b307c/After,
/hono/users/1invite dev, span nameGET /hono/users/:idfrom the Hono route: https://sentry-sdks.sentry.io/explore/traces/trace/20b2a5dcfe6a4dbfb7e68eb3037bf3b2/