Skip to content
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 hard-codes DSN during build stage for output:server with node #12412

Closed
3 tasks done
arty-name opened this issue Jun 7, 2024 · 11 comments
Closed
3 tasks done
Labels
Package: astro Issues related to the Sentry Astro SDK Type: Bug

Comments

@arty-name
Copy link
Contributor

Is there an existing issue for this?

How do you use Sentry?

Self-hosted/on-premise

Which SDK are you using?

@sentry/astro

SDK Version

8.8.0

Framework Version

4.10.0

Link to Sentry event

No response

SDK Setup

  output: 'server',
  adapter: node({ mode: 'standalone' }),
  integrations: [sentry({ enabled: true })],

Steps to Reproduce

  1. Run astro build without providing DSN
  2. Run PUBLIC_SENTRY_DSN=FAIL astro preview

Or visit the minimal reproduction at https://stackblitz.com/edit/github-5bh1gz?file=package.json

Expected Result

Sentry should log Invalid Sentry Dsn: FAIL

Actual Result

Sentry ignores the provided DSN and does not activate.

Sentry logs this error if you provide PUBLIC_SENTRY_DSN=FAIL during the build stage. Providing a correct DSN at runtime does not fix that.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Jun 7, 2024
@github-actions github-actions bot added the Package: astro Issues related to the Sentry Astro SDK label Jun 7, 2024
@Lms24
Copy link
Member

Lms24 commented Jun 10, 2024

Hey @arty-name thanks for writing in!

If I understand correctly, the issue seems to be that when we generate the server-side Sentry.init code, we use import.meta.env.PUBLIC_SENTRY_DSN, which, during build already, is replaced by the environment variable's value. So you're not able to override the value during runtime with PUBLIC_SENTRY_DSN. Is this correct?

If so, I think we could get away with replacing import.meta.env with process.env but not sure yet

@arty-name
Copy link
Contributor Author

You understood the issue correctly, Lukas!

I do believe that the change to process.env should help. By the way, in case you haven’t stumbled on this yet, Astro node integration handles the environment variables in a peculiar manner.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Jun 11, 2024
@Lms24
Copy link
Member

Lms24 commented Jun 11, 2024

That's peculiar indeed and I wasn't aware of this at all 😬 I guess we need to try if switching to process.env would make any difference at all then. But ultimately, I believe this is only something users can control.

@getsantry
Copy link

getsantry bot commented Nov 23, 2024

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you remove the label Waiting for: Community, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@getsantry getsantry bot added the Stale label Nov 23, 2024
@arty-name
Copy link
Contributor Author

I don’t know why "waiting for community" label was added here.
I have just verified that even with the latest versions of dependencies the error still happens:

    "@astrojs/node": "^8.3.4",
    "@sentry/astro": "^8.40.0",
    "astro": "^4.16.15"

@getsantry getsantry bot moved this from Waiting for: Community to Waiting for: Product Owner in GitHub Issues with 👀 3 Nov 26, 2024
@getsantry getsantry bot removed the Stale label Nov 27, 2024
@chargome
Copy link
Member

Hey @arty-name, passing the DSN directly to our astro integration will be deprecated soon (#14489), instead we encourage users to create runtime specific config files for client and server respectively.

@arty-name
Copy link
Contributor Author

Thank you for taking time to reply @chargome but I do not pass the DSN to the integration

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Nov 28, 2024
@chargome
Copy link
Member

@arty-name yep I saw that, but the recommended setup for the server instrumentation is creating a sentry.sever.config.js file where you add:

import * as Sentry from "@sentry/astro";

Sentry.init({
  dsn: // get your dsn here

  // Define how likely traces are sampled. Adjust this value in production,
  // or use tracesSampler for greater control.
  tracesSampleRate: 1.0,
});

So here you can get the dsn whichever way you want.

@arty-name
Copy link
Contributor Author

Has the previously suggested approach of providing the environment variable PUBLIC_SENTRY_DSN been deprecated as well?

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Nov 28, 2024
@chargome
Copy link
Member

Yes inherently, since providing a sentry.sever.config.js will prevent the integration from automatically creating this file. We took this step because providing the config values to the integration introduces intransparent behaviour which we want to prevent. I hope this setup does not cause you too much extra hassle!

@arty-name
Copy link
Contributor Author

Thank you for your patience and detailed explanations @chargome! I guess I will close this ticket then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Package: astro Issues related to the Sentry Astro SDK Type: Bug
Projects
Archived in project
Development

No branches or pull requests

4 participants