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

Handle more socket exceptions #349

Merged
merged 4 commits into from Feb 5, 2019

Conversation

dabcoder
Copy link
Contributor

Attempts to fix #212.

gzussa
gzussa previously requested changes Jan 25, 2019
datadog/dogstatsd/base.py Outdated Show resolved Hide resolved
datadog/dogstatsd/base.py Outdated Show resolved Hide resolved
@@ -289,9 +289,12 @@ def _send_to_server(self, packet):
except socket.timeout:
# dogstatsd is overflowing, drop the packets (mimicks the UDP behaviour)
return
except socket.error:
except (socket.error, socket.herror, socket.gaierror) as se:
log.info("Error submitting packet, dropping the packet and closing the socket")
self.close_socket()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be closing the socket in the second except clause as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure, # If set, use socket directly, would the last exception cover the case of not using the socket and thus not needing to close it? Or am I on the wrong track there?

Copy link
Contributor

@ofek ofek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't address the threading issue. You need to put a lock in get_socket.

datadog/dogstatsd/base.py Outdated Show resolved Hide resolved
@gzussa gzussa dismissed their stale review February 5, 2019 09:44

Looks good to me but I would like @ofek to confirm as well regarding the lock

datadog/dogstatsd/base.py Outdated Show resolved Hide resolved
Co-Authored-By: dabcoder <david.bouchare@datadoghq.com>
@ofek ofek changed the title Handle all exceptions Handle more socket exceptions Feb 5, 2019
@ofek ofek merged commit a058b90 into DataDog:master Feb 5, 2019
@dabcoder dabcoder deleted the davidb/handle-exceptions branch October 16, 2019 08:10
dabcoder pushed a commit to dabcoder/datadogpy that referenced this pull request Oct 25, 2019
* Handle all exceptions

* Change log levels and increase max line for tests

* Add lock to address threading issues

* Remove conversion to str

Co-Authored-By: dabcoder <david.bouchare@datadoghq.com>
dabcoder pushed a commit to dabcoder/datadogpy that referenced this pull request Nov 28, 2019
* Handle all exceptions

* Change log levels and increase max line for tests

* Add lock to address threading issues

* Remove conversion to str

Co-Authored-By: dabcoder <david.bouchare@datadoghq.com>
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.

not all exceptions being caught
6 participants