ref(nuxt): Extract handler patching to extra plugin for Nitro v2/v3#19915
Merged
ref(nuxt): Extract handler patching to extra plugin for Nitro v2/v3#19915
Conversation
Contributor
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. New Features ✨Deps
Bug Fixes 🐛Cloudflare
Core
Deps
Other
Internal Changes 🔧Deps Dev
Other
🤖 This preview updates automatically when you update the PR. |
s1gr1d
force-pushed
the
sig/nuxt-v5-handler
branch
from
3844373 to
9c9528e
Compare
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
+83
to
84
| addServerPlugin(moduleDirResolver.resolve('./runtime/plugins/handler-legacy.server')); | ||
| addServerPlugin(moduleDirResolver.resolve('./runtime/plugins/sentry.server')); |
There was a problem hiding this comment.
Bug: The new handler-legacy.server.ts plugin and the existing sentry.server.ts plugin both patch nitroApp.h3App.handler, leading to the handler being wrapped twice.
Severity: MEDIUM
Suggested Fix
The refactoring appears to be incomplete. To avoid double patching, the patching logic within sentry.server.ts should be removed, as the intent was to centralize this functionality in the new handler-legacy.server.ts plugin.
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/nuxt/src/module.ts#L83-L84
Potential issue: The pull request introduces a new server plugin,
`handler-legacy.server.ts`, which patches the Nitro event handler
`nitroApp.h3App.handler`. However, the existing `sentry.server.ts` plugin, which also
performs the same patching, was not modified to remove its patching logic. Since both
plugins are registered unconditionally, the `handler-legacy.server.ts` plugin will run
first (due to alphabetical sorting), followed by `sentry.server.ts`, resulting in the
event handler being patched twice. This double-wrapping could lead to redundant
operations and potential unexpected side effects or performance degradation.
Did we get this right? 👍 / 👎 to inform future reviews.
s1gr1d
commented
Mar 20, 2026
| "devDependencies": { | ||
| "@nuxt/module-builder": "^0.8.4", | ||
| "@nuxt/nitro-server": "^3.21.1", | ||
| "nitro": "^3.0.260311-beta", |
Member
Author
There was a problem hiding this comment.
I installed the beta version as the 3.0.0 version is using different types 😬
![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 prepared a fix for the issue found in the latest run.
- ✅ Fixed: Handler gets patched twice causing double Proxy wrapping
- Removed the duplicate handler patching logic from
sentry.server.tsso only the dedicatedhandler-legacy.server.tsplugin wrapsnitroApp.h3App.handleronce.
- Removed the duplicate handler patching logic from
Or push these changes by commenting:
@cursor push b1f2cae2f5
Preview (b1f2cae2f5)
diff --git a/packages/nuxt/src/runtime/plugins/sentry.server.ts b/packages/nuxt/src/runtime/plugins/sentry.server.ts
--- a/packages/nuxt/src/runtime/plugins/sentry.server.ts
+++ b/packages/nuxt/src/runtime/plugins/sentry.server.ts
@@ -1,11 +1,5 @@
-import {
- debug,
- flushIfServerless,
- getDefaultIsolationScope,
- getIsolationScope,
- withIsolationScope,
-} from '@sentry/core';
-import type { EventHandler, H3Event } from 'h3';
+import { debug } from '@sentry/core';
+import type { H3Event } from 'h3';
import type { NitroAppPlugin } from 'nitropack';
import type { NuxtRenderHTMLContext } from 'nuxt/app';
import { sentryCaptureErrorHook } from '../hooks/captureErrorHook';
@@ -13,8 +7,6 @@
import { addSentryTracingMetaTags } from '../utils';
export default (nitroApp => {
- nitroApp.h3App.handler = patchEventHandler(nitroApp.h3App.handler);
-
nitroApp.hooks.hook('beforeResponse', updateRouteBeforeResponse);
nitroApp.hooks.hook('error', sentryCaptureErrorHook);
@@ -37,26 +29,3 @@
}
});
}) satisfies NitroAppPlugin;
-
-function patchEventHandler(handler: EventHandler): EventHandler {
- return new Proxy(handler, {
- async apply(handlerTarget, handlerThisArg, handlerArgs: Parameters<EventHandler>) {
- const isolationScope = getIsolationScope();
- const newIsolationScope = isolationScope === getDefaultIsolationScope() ? isolationScope.clone() : isolationScope;
-
- debug.log(
- `Patched h3 event handler. ${
- isolationScope === newIsolationScope ? 'Using existing' : 'Created new'
- } isolation scope.`,
- );
-
- return withIsolationScope(newIsolationScope, async () => {
- try {
- return await handlerTarget.apply(handlerThisArg, handlerArgs);
- } finally {
- await flushIfServerless();
- }
- });
- },
- });
-}This Bugbot Autofix run was free. To enable autofix for future PRs, go to the Cursor dashboard.
Contributor
size-limit report 📦
|
chargome
reviewed
Mar 20, 2026
Contributor
node-overhead report 🧳Note: This is a synthetic benchmark with a minimal express app and does not necessarily reflect the real-world performance impact in an application.
|
logaretm
approved these changes
Mar 20, 2026
Member
logaretm
left a comment
logaretm
left a comment
There was a problem hiding this comment.
LGTM, if we need to check Nitro's version we can probably do something like this and read it from the package json.
is there a possibility that Nuxt 4 uses Nitro v3?
Member
Author
Nuxt 4 can enable a compatibility mode for v5, but there's still Nuxt v4 and Nitro v2 packages installed. It's just using the new APIs. The idea with reading it from the package json sounds good - I could add this (along with an E2E test) in another PR. |
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
3 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.
h3Appis calledh3in Nitro v3. Additionally, the type imports have changed.This PR is just moving some code around (not changing any logic). Those are the made changes:
nitroas adevDependency-> the new import in Nitro v3 (instead ofnitropack)Currently, the
handler.server.tsfile (for Nuxt v5) is unused as we don't have a reliable way yet to tell whether Nitro v2 or v3 is running. In theory we could do something like this below but this only checks for Nuxt and not Nitro.Closes #19913