-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
@sentry/astro
breaks drizzle-orm
in Astro
#12912
Comments
Hello, thank you for writing in. I've tried reproducing this locally with an astro starter project and the steps outlined here but did not encounter the issue.
|
This looks to be some kind of issue with import in the middle and ESM. We are still investigating, but for now to get you unstuck you could create an empty |
I suspect this is the same issue as seen here: I've got an open PR to fix this: |
@timfish lol I just opened another issue in nodejs/import-in-the-middle#141 b/c I missed the connection to nodejs/import-in-the-middle#139 😅 |
Well, I do still need to check that they're caused by the same issue! |
General comment to what we found out while debugging this issue: It looks like there are several problems with Astro and OpenTelemetry in in general and even if we get this to run with the current SDK initialization procedure, auto instrumentation is likely not gonna work. I couldn't verify this yet though, so it's just an educated guess. The core problem is that we need to register the opentelemetry hook as early as possible. In Astro we can't pass the > npm run dev
Fragment is not defined
Stack trace:
at Function.assign (<anonymous>)
at ModuleJob.run (node:internal/modules/esm/module_job:218:25)
at async runCommand (file:///Users/lukas/code/test-projects/gh-sentry-javascript-12912-astro-iitm/node_modules/astro/dist/cli/index.js:130:23) I'm gonna look a bit into the Astro side of things to see what might be causing this. |
I think the long term solution might be to push frameworks like Astro (and any others where this is an issue) to follow Nextjs and add an |
Do any of the Astro integration hooks get called before user code is asyncroniously loaded? As long as we can get Sentry init'ed before the instrumented libraries are loaded via |
We currently already use the |
A fix was release in You can either delete your lock file and reinstall or |
It still doesn't work. I tried opening the StackBlitz and running
But the site still doesn't work. In my own project I tried forcing v1.7.1 to resolve to v1.9.1, but that didn't help. |
Thanks for testing! We also found out that the fix didn't address the |
With v8.18.0 of the SDKs just released, there's a new config option which can disable import * as Sentry from '@sentry/node';
Sentry.init({
dsn: '__DSN__',
registerEsmLoaderHooks: { exclude: ['drizzle-orm'] },
}); |
I just tested it, and even if I exclude |
hey, can you ensure to update the transitive import-in-the-middle dependency to 1.10.0? This includes some fixes, and it can happen that you don't have the latest version because opentelemetry depends on Does it still fail with these steps? |
Yes. I updated the StackBlitz with an NPM override, and it did not work. |
quick update: I'm a bit busy with other stuff unfortunately. But we asked @timfish (whenever you have some time, Tim) to take another look in the meantime. |
I am working on a solution for this! |
@RIP21 Since v8.20.0, you can now pass |
@mydea yup. I already used it yesterday. Thanks for the swift fix :) |
@RIP21 I don't understand how you solved the issue. Can you give an example please? I'm not using a preload because I don't have any special hooks, but I would love to use one if it will let me add Sentry back into my application. |
@laptou inused include instead of exclude, including all the dependencies (excluding dev dependencies) I use except Drizzle-orm and it worked. Just doing exclude on Drizzle-orm didn't work out for me. @timfish @mydea FYI, maybe it's also a problem, but I hope underlying issue will be solved so these include/exclude won't be needed (BTW which issues to subscribe to to track that?) |
We are working on #13139, and will eventually look at using this by default, which will allow you to skip wrapping anything that we are not explicitly instrumenting, without having to define packages manually! |
v8.29.0 of the Node SDK adds the new import * as Sentry from '@sentry/node';
Sentry.init({
dsn: '__PUBLIC_DSN__',
registerEsmLoaderHooks: { onlyIncludeInstrumentedModules: true },
}); When set to This feature will only work if you Please let me know if this helps solve the |
Closing this for clean-up. If this issue still applies, please re-open. Thanks! |
Is there an existing issue for this?
How do you use Sentry?
Sentry Saas (sentry.io)
Which SDK are you using?
@sentry/astro
SDK Version
8.17.0
Framework Version
Astro 4.11.5
Link to Sentry event
No response
SDK Setup/Reproduction Example
https://stackblitz.com/edit/github-yligk2?file=astro.config.mjs
Steps to Reproduce
@sentry/astro
anddrizzle-orm
.src/middleware.ts
file.drizzle-orm
:MiddlewareCantBeLoaded
becausedrizzle-orm
has no export "or".Expected Result
Pages should load as normal even if the middleware depends on
drizzle-orm
.Actual Result
The text was updated successfully, but these errors were encountered: