Conversation
size-limit report 📦
|
|
This still fails, but because of another issue. That's probably an upstream issue. Created a PR to add |
Related to this comment: #24283 (comment)
| const { method, path } = { | ||
| method: errorContext.event?._method ? errorContext.event._method : '', | ||
| path: errorContext.event?._path ? errorContext.event._path : null, | ||
| }; | ||
|
|
||
| if (path) { | ||
| getCurrentScope().setTransactionName(`${method} ${path}`); | ||
| } |
There was a problem hiding this comment.
Bug: The createCaptureErrorHook utility accesses event._method and event._path, which are internal to h3 v1. These fields do not exist in h3 v2 (used by Nitro v3/Nuxt 5+).
Severity: MEDIUM
Suggested Fix
Update the shared utility to handle both h3 v1 and h3 v2 event shapes. Check for the standard event.path before falling back to event._path, and use event.req.method or event.method to get the request method, ensuring compatibility with both Nitro v2 and v3.
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/runtime/utils/captureError.ts#L51-L58
Potential issue: The shared utility `createCaptureErrorHook` attempts to set the Sentry
transaction name by accessing `errorContext.event._method` and
`errorContext.event._path`. These properties are internal to the `H3Event` from `h3 v1`
(used in Nuxt 3/4). However, in Nuxt 5+ which uses Nitro v3 and `h3 v2`, the event
object is an `HTTPEvent` and lacks these underscore-prefixed fields. Consequently, for
Nuxt 5+ applications, the method and path will be `undefined`, the
`setTransactionName()` function will not be called, and transaction names for captured
server errors will be missing, degrading observability.
Did we get this right? 👍 / 👎 to inform future reviews.
| */ | ||
| export default (nitroApp => { | ||
| // @ts-expect-error Nitro v3 hands the `error` hook an `HTTPEvent`, Nitro v2 an `H3Event` | ||
| nitroApp.hooks.hook('error', sentryCaptureErrorHook); |
There was a problem hiding this comment.
l/q: Can't we do a function overload then?
| @@ -0,0 +1,12 @@ | |||
| // eslint-disable-next-line import/no-extraneous-dependencies | |||
| import { H3Error } from 'h3'; | |||
There was a problem hiding this comment.
q: Why can't we simply move it to nitro/h3 entirely? Then we could keep everything in one function
Nuxt 5 switched on its Nitro v2 compatibility layer for any module whose server code imports
h3: nuxt/nuxt#36318Split it the way we already split storage and database:
captureErrorHookusesnitro/h3,captureErrorHook-legacykeepsh3, andmodule.tspicks by Nitro major.Our CI tests currently show this warning: