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

Update Twisted integration #168

Open
wants to merge 10 commits into
base: master
Choose a base branch
from
Open

Conversation

ezarowny
Copy link
Contributor

@ezarowny ezarowny commented Jun 16, 2017

This PR will:

Testing:

  • If you're a Rollbar employee, the easiest way to cause an error to get trapped in the reactor is to try and send an error to your dev vm using the HTTPS endpoint. Twisted doesn't like self-signed certs.

Status:

  • Attempting to reproduce some edge cases
  • Use inlineCallbacks to reduce lines of code
  • Currently deciding the best way to handle failures from within Twisted itself. The easiest way to just check for that the err.type in log_handler is a specific Twisted Exception or some subclass of one and don't send it back to report_exc_info. The nicer alternative would be to somehow tag an error as coming from Rollbar but I'm not quite there yet.

@ezarowny
Copy link
Contributor Author

@markrwilliams here's what ends up looping:

twisted.internet.task.TaskStopped:

ERROR:rollbar:2017-06-18T22:20:54.251772 Failed to post to rollbar
Traceback (most recent call last):
  File "/Users/ezarowny/Development/pyrollbar/rollbar/__init__.py", line 1205, in _post_api_twisted
    timeout=SETTINGS.get('timeout', DEFAULT_TIMEOUT))
RequestTransmissionFailed: [<twisted.python.failure.Failure OpenSSL.SSL.Error: [('SSL routines', 'tls_process_server_certificate', 'certificate verify failed')]>]
catching exception
Unexpected exception from twisted.web.client.FileBodyProducer.stopProducing
Traceback (most recent call last):
  File "/Users/ezarowny/.virtualenvs/pyrollbar/lib/python2.7/site-packages/twisted/protocols/policies.py", line 125, in connectionLost
    self.wrappedProtocol.connectionLost(reason)
  File "/Users/ezarowny/.virtualenvs/pyrollbar/lib/python2.7/site-packages/twisted/web/_newclient.py", line 964, in dispatcher
    return func(*args, **kwargs)
  File "/Users/ezarowny/.virtualenvs/pyrollbar/lib/python2.7/site-packages/twisted/web/_newclient.py", line 1634, in _connectionLost_TRANSMITTING
    self._currentRequest.stopWriting()
  File "/Users/ezarowny/.virtualenvs/pyrollbar/lib/python2.7/site-packages/twisted/web/_newclient.py", line 865, in stopWriting
    _callAppFunction(self.bodyProducer.stopProducing)
--- <exception caught here> ---
  File "/Users/ezarowny/.virtualenvs/pyrollbar/lib/python2.7/site-packages/twisted/web/_newclient.py", line 194, in _callAppFunction
    function()
  File "/Users/ezarowny/.virtualenvs/pyrollbar/lib/python2.7/site-packages/twisted/web/client.py", line 1079, in stopProducing
    self._task.stop()
  File "/Users/ezarowny/.virtualenvs/pyrollbar/lib/python2.7/site-packages/twisted/internet/task.py", line 497, in stop
    self._checkFinish()
  File "/Users/ezarowny/.virtualenvs/pyrollbar/lib/python2.7/site-packages/twisted/internet/task.py", line 507, in _checkFinish
    raise self._completionState

@rokob
Copy link
Contributor

rokob commented Feb 16, 2018

#172 needs to get addressed and this appears to go towards that, but I'm going to bump this to the next release because getting into the weeds of twisted is not what I want to have hold up a bunch of other changes

@rokob rokob modified the milestones: v0.13.18, v0.14.0 Feb 16, 2018
@rokob
Copy link
Contributor

rokob commented Apr 30, 2018

Fixes #163

@ezarowny
Copy link
Contributor Author

This is a tough one dude! Very curious to see how you end up preventing that infinite loop.

@rokob
Copy link
Contributor

rokob commented May 1, 2018

With the current code, as far as I can tell, there are no infinite loops in the case of an SSL error. If you get rid of the add/remove observer stuff then you do get some looping so it appears that is part of the solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants