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

Try/Catch everything in the servlet filter to be safe (and a bit paranoid) #57

Merged
merged 1 commit into from
Jan 29, 2015
Merged

Conversation

joeslice
Copy link
Contributor

I think all the exception-causing spots have been addressed, but as we've seen with #49 and #54, unexpected exceptions cause our restful services to return a 500.

This change will make the 500's just warn and continue, strengthening the guarantee that the tracing library won't negatively impact your application in performance or otherwise.

@joeslice
Copy link
Contributor Author

Hey @kristofa how do you feel about this pull request?

@kristofa
Copy link
Member

Ok, sorry for the delay @joeslice . Good that you reminded me this one was still open.
I'll merge it.

kristofa added a commit that referenced this pull request Jan 29, 2015
Try/Catch everything in the servlet filter to be safe (and a bit paranoid)
@kristofa kristofa merged commit 9e1b512 into openzipkin:master Jan 29, 2015
@joeslice
Copy link
Contributor Author

Great; thanks for merging this!

@joeslice joeslice deleted the safety branch January 30, 2015 02:36
@srapp
Copy link

srapp commented Feb 23, 2015

Hey @kristofa when are you next planning to release? I'd like to get our services using this change.

@kristofa
Copy link
Member

@srapp I'll try to find some time to do the release tomorrow. I think some other people are also waiting for a release, for example for #64.

@kristofa
Copy link
Member

@srapp I made the brave release that contains this pull request. See here for release notes. It should become available in Maven Central shortly.

@srapp
Copy link

srapp commented Feb 24, 2015

Awesome, thanks @kristofa !

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.

3 participants