-
Notifications
You must be signed in to change notification settings - Fork 305
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
Conversation
@@ -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() |
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.
Should we be closing the socket in the second except
clause as well?
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.
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?
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.
This doesn't address the threading issue. You need to put a lock in get_socket
.
68cc551
to
86211db
Compare
Looks good to me but I would like @ofek to confirm as well regarding the lock
Co-Authored-By: dabcoder <david.bouchare@datadoghq.com>
* 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>
* 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>
Attempts to fix #212.