-
Notifications
You must be signed in to change notification settings - Fork 512
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Upgrade @vercel/nft
to 0.27.4 to include ESM module.register
hooks
#2703
Comments
We have already upgraded to In the meantime, it is possible to use nightly release channel to receive the update. |
Thanks for this information :) Would it be possible to create a patch release (nitro |
We can... Can you please help to make a minimal nitro reproduction for the issue with open telemetry dependency? I remember last time there were more issues specially on nodeless (edge) platforms this way at least we can make sure this releases fully solves the problem. |
I updated the issue and created a minimal reproduction with Sentry as Sentry uses |
Hi @pi0, any decisions on this? This also holds up otel instrumentation for vinxi for example. |
Thanks for reproduction @s1gr1d it helped me to understand the better of usage. @andreiborza We are working on the v2 branch to be stable (we also already asked both Nuxt and solid/vinxi teams since last week to review the nightly channel) as soon as we are sure of stability will issue a new release, I hope (but don't promise) by early next week at latest for it to happen. My understanding is that On a relevant topic, if you are interested I would love to have better support of Sentry for Nitro (both Node.js and Edge) (long-time user!), i will contact you if you are intrested. |
Hy, yeah adding a resolution is the only current workaround. And it is good to hear that another release is coming soon! Currently, the Nuxt and SolidStart SDKs are in alpha/beta and it would be nice for developers to use it without the resolution once the proper release of the SDK comes out. And we are about to start working on a Nitro instrumentation: getsentry/sentry-javascript#13670 |
Sounds exciting. I could not find any contact in X. Would you mind to drop me a DM in discord ( |
We've been using a manual OpenTelemetry instrumentation for our projects and the idea of having an officially supported version is exciting! Have you given any thought to supporting Native Instrumentation within the Nitro package itself? (Similar to NextJS) With an external package it can be difficult to inject the necessary code at the right points. For example, in our instrumentation package we have to use a nitro plugin where we get a handle on the h3 app and patch the nitro router to wrap each request in a handle. It works, but as with all code that depends on internals there is the risk of future breakage. https://opentelemetry.io/docs/concepts/instrumentation/libraries/ |
@cjpearson Actually yes one of the reasons I am thinking of nitro-native support is to have a cross-platform instrument system. I don't prefer the complexity of open telemetry Node.js SDK either. |
Oh cool. Are you open to PRs in this direction? |
Hi, I'm using Nuxt 3 and already add resolutions for @vercel/nft but still didn't work. Here's my package version:
|
2.10 is released with vercel/nft dependency update. otel support requires more works. |
Environment
ESM
Reproduction
Link: https://github.com/s1gr1d/nitro-sentry-iitm
This is how import-in-the-middle (IITM) looks like in
.output/server/node_modules
:Describe the bug
Previous versions of @vercel/nft do not include ESM
module.register
hooks so the@opentelemetry/instrumentation/hook.mjs
is not included, which leads to errors when starting the server if opentelemetry or packages that depend on it (like Sentry) are used.@vercel/nft
released a new version with the fix, but as it is still a0
major version, the caret (^
) does not work like usual. With0
major versions, only the patch is automatically updated, but not the minor (npm source).The current release of nitro depends on
^0.26.5
and the fix is included in0.27.4
(another minor version).It would be awesome to release a new nitro version (minor or patch), which includes this
@vercel/nft
minor version upgrade 🙌Additional context
Issue Reference:
@sentry/astro
on Netlify getsentry/sentry-javascript#12603 (netlify-related, but this uses nft as well) - includes a reproductionLogs
No response
The text was updated successfully, but these errors were encountered: