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

Don't require a user in the DSN #361

Merged
merged 1 commit into from
Aug 7, 2015
Merged

Don't require a user in the DSN #361

merged 1 commit into from
Aug 7, 2015

Conversation

fardog
Copy link
Contributor

@fardog fardog commented Aug 7, 2015

raven-js requires a user part in the DSN passed to Raven.config, but in some scenarios this isn't desired. This PR makes user and optional part.

In our configuration, our sentry instance isn't publicly available, and we proxy requests from our frontend application through our backend app for reporting; because of this, we use our already authenticated users (and sentry auth is handled via the backend). We were on a very old version of raven-js which did not have this restriction. This restriction blocks us from being able to upgrade.

Let me know if you have any questions/requests, and thanks for your time!

@dcramer
Copy link
Member

dcramer commented Aug 7, 2015

I think I'd prefer proxies just take the input and deal with passing it forward. Can you see that as being unreasonsable?

@dcramer
Copy link
Member

dcramer commented Aug 7, 2015

Talked about this over IRC.

This is similar to how we used Sentry at Disqus. Basically pass thru everything to a backend, and apply a new DSN on the backend. I'm fine w/ supporting that use case for now, at least until we have a better answer to this behavior.

dcramer added a commit that referenced this pull request Aug 7, 2015
Don't require a user in the DSN
@dcramer dcramer merged commit 6768e4a into getsentry:master Aug 7, 2015
@fardog
Copy link
Contributor Author

fardog commented Aug 7, 2015

Thanks!

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants