fix(nextjs): Do not bundle orchestrion on CF WIP - #22851
Conversation
|
bugbot run |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| if (runtime === 'server' && userSentryOptions._experimental?.useDiagnosticsChannelInjection) { | ||
| newConfig.plugins.push(sentryOrchestrionWebpackPlugin() as unknown as WebpackPluginInstance); | ||
| prependOrchestrionRuntimeExternals(newConfig); | ||
| const orchestrion = loadOrchestrionBundlerPlugin(); |
There was a problem hiding this comment.
Bug: The dynamic import for the orchestrion bundler plugin can fail silently, deactivating the useDiagnosticsChannelInjection feature without any warning to the user.
Severity: LOW
Suggested Fix
Add a warning or log message when loadOrchestrionBundlerPlugin() returns undefined inside the webpack configuration. This would alert the user that the useDiagnosticsChannelInjection feature they enabled could not be activated due to a module loading failure.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/nextjs/src/config/webpack.ts#L433
Potential issue: The orchestrion bundler plugin is loaded dynamically via
`loadModule()`. If this module fails to load for any reason, such as a corrupted
installation, the function returns `undefined`. This causes the
`useDiagnosticsChannelInjection` feature to be silently disabled without any warning or
log being emitted. A user who has explicitly enabled this feature will not be aware that
it is not active, which can hinder debugging and monitoring efforts.
Also affects:
packages/nextjs/src/config/turbopack/constructTurbopackConfig.ts:140~140packages/nextjs/src/config/loadOrchestrionBundlerPlugin.ts:28~34
Did we get this right? 👍 / 👎 to inform future reviews.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 1e6f94b. Configure here.
| new RegExp(String.raw`loadModule\(\s*['"]${escapeRegExp(SUBPATH)}['"]\s*,\s*module\s*\)`), | ||
| ); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Test loop should use it.each
Low Severity
The second regression case loops over the CJS and ESM builds inside one it, while the sibling case already uses it.each. Per the Testing Conventions review rule, multi-scenario coverage should use it.each / test.each so failures are attributed per build.
Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 1e6f94b. Configure here.
1 participant
closes #22794