-
Notifications
You must be signed in to change notification settings - Fork 55
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
Allow optional disabling of connection exceptions #16
Allow optional disabling of connection exceptions #16
Conversation
Can you do a |
I don't think trigger errors or more ideal that catching exceptions. I don't know right now the best approach, but a wrapper could be one. |
@philsturgeon rebase done. I made it optional since I think how one wants to deal with these types of failures is an implementation detail. Etsy's blog post on why they chose UDP in particular says:
Since I was just hoping to avoid... try {
// ... do statsd stuff
} (catch ConnectionException $e) {
// do nothing
} .. because it gives me nightmares about how we used to write Java at university. :) |
ping? :) Or should I close this w/o merge? |
ping? |
Allow optional disabling of connection exceptions
Huge apologies for the delay on getting this merged @michaelmoussa, great contribution |
@marcqualie would you mind tagging a new release with this change please? |
I've now released version 1.3.0 |
Metrics gathering on my project is a welcome bonus, but not a critical feature, so I don't want a page load to fail if it can't connect to StatsD. I also don't want to wrap every invocation with:
I could just write a wrapper to do all that transparently, but I think "just log a warning and keep loading the page" is a common enough use case, and making the exception throwing configurable would be a helpful contribution.
The
throwConnectionExceptions
configuration setting is completely optional, and it will throw exceptions by default like it does now (so there is no BC issue).