-
Notifications
You must be signed in to change notification settings - Fork 188
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
If inside a HTTP request, include URL in sentry error #775
Conversation
|
||
RequestStash() { | ||
this.url = new ThreadLocal<>(); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: ditch the constructor and initialize inline
lgtm, afaik Guice doesn't have anything to make this easier. The other option would be to have a provider that catches the
This is simpler in some ways, but the drawback is you need to be very careful to always inject a |
@jhaber gotcha, thanks. I originally hit issues with your OOSE-catching approach in a standalone provider, but I think I just made a mistake on my end because it's working now. I'm not too worried about the only using |
cool lgtm |
If inside a HTTP request, include URL in sentry error
Will update PR with full implementation soon -- wanted to get @jhaber's eyes on the request stash stuff.