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

not all exceptions being caught #212

Closed
jsamuel opened this issue Jun 20, 2017 · 3 comments · Fixed by #349
Closed

not all exceptions being caught #212

jsamuel opened this issue Jun 20, 2017 · 3 comments · Fixed by #349
Assignees

Comments

@jsamuel
Copy link

jsamuel commented Jun 20, 2017

I see cases where exceptions aren't being caught from calls to increment() and other stats reporting functions. According to the code, it's only catching socket.error but there are other errors that can happen, at least in multithreaded applications.

  File "/opt/python3.6/lib/python3.6/site-packages/datadog/dogstatsd/base.py", line 153, in increment
    self._report(metric, 'c', value, tags, sample_rate)
  File "/opt/python3.6/lib/python3.6/site-packages/datadog/dogstatsd/base.py", line 262, in _report
    self._send(encoded)
  File "/opt/python3.6/lib/python3.6/site-packages/datadog/dogstatsd/base.py", line 267, in _send_to_server
    (self.socket or self.get_socket()).send(packet.encode(self.encoding))
AttributeError: 'NoneType' object has no attribute 'send'

I think you should be catching every exception and never letting an exception escape from your code. Otherwise, these types of bugs in dogstats can cause bugs in people's applications if they are using dogstats. As it stands, it looks like the only safe usage appears to be to write a wrapper around dogstats calls to make sure they can never raise exceptions.

@yannmh yannmh self-assigned this Jun 21, 2017
@jbayardo
Copy link

jbayardo commented Sep 5, 2018

Hi. Any plans on fixing this? I am getting the same exception from one of our core apps

@melinath
Copy link

Hey, just encountered this as well. Are there any plans to fix this?

@dabcoder
Copy link
Contributor

dabcoder commented Feb 8, 2019

Yes indeed, the linked PR should address this issue. Thanks for reporting.

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 a pull request may close this issue.

5 participants