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 should raise an exception if the DSN has not been configured #2494

Closed
jamescrosswell opened this issue Jul 18, 2023 · 1 comment · Fixed by #2655
Closed

Sentry should raise an exception if the DSN has not been configured #2494

jamescrosswell opened this issue Jul 18, 2023 · 1 comment · Fixed by #2655
Assignees
Labels
Feature New feature or request
Projects
Milestone

Comments

@jamescrosswell
Copy link
Collaborator

jamescrosswell commented Jul 18, 2023

Problem Statement

At the moment, if the DSN is empty, you get a DisabledHub. But that's because we've decided to use "" as the DSN for a disabled hub.

This means the SentrySDK is unable to tell if someone forgot to supply the DSN or if they intentionally left it blank in order to disable Sentry.

Solution Brainstorm

If the DisableSdkDsnValue was a non-empty string (e.g. "disabled") then people could still disable Sentry via code/config. and we could then provide some useful guidance when initialising the SDK in cases where they forgot to supply the DSN (e.g. "Error: You forgot to supply a DSN. Sentry needs a DSN to do anything useful.").

This is a breaking change, so would need to wait until the next major release.

Additional comments from Kanadaj

@jamescrosswell I just want to highlight the fact that dsn is an optional argument of WriteTo.Sentry():

So supplying the dsn is required, but the dsn argument is not? It's kind of super confusing as a user of the library, especially since the Serilog provider doesn't seem to read the value from IConfiguration unlike UseSentry().

I guess a solution would be to have 2 alternative calls with WriteTo.Sentry(), one that takes a non-null string as the first argument, and one that takes a boolean for initializeSdk as the first non-optional argument.

Originally posted by @kanadaj in #2883 (comment)

@bruno-garcia
Copy link
Member

This "" behavior is 10 years old and set in all SDks. Not sure this is worth doing. Usualy not passing the DSN means null is passed (DSN should be nullable). And in this case, throwing would be OK.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature or request
Projects
Archived in project
.NET SDK
Awaiting triage
Development

Successfully merging a pull request may close this issue.

3 participants