From ffc6cdd952d5c396721c8e3e92d017671de1b146 Mon Sep 17 00:00:00 2001 From: Eric Zarowny Date: Fri, 16 Jun 2017 14:49:23 -0700 Subject: [PATCH 1/8] Update Twisted integration --- rollbar/__init__.py | 47 ++++++++++++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/rollbar/__init__.py b/rollbar/__init__.py index 3767c62f..d1de15b4 100644 --- a/rollbar/__init__.py +++ b/rollbar/__init__.py @@ -93,6 +93,8 @@ def wrap(*args, **kwargs): try: import treq + from twisted.internet.defer import inlineCallbacks + from twisted.internet.defer import returnValue from twisted.python import log as twisted_log def log_handler(event): @@ -112,12 +114,8 @@ def log_handler(event): report_exc_info((err.type, err.value, err.getTracebackObject())) except: log.exception('Error while reporting to Rollbar') - - # Add Rollbar as a log handler which will report uncaught errors - twisted_log.addObserver(log_handler) - - except ImportError: + inlineCallbacks = passthrough_decorator treq = None @@ -290,8 +288,12 @@ def init(access_token, environment='production', **kw): if SETTINGS.get('allow_logging_basic_config'): logging.basicConfig() - if SETTINGS.get('handler') == 'agent': + handler = SETTINGS.get('handler') + if handler == 'agent': agent_log = _create_agent_log() + elif handler == 'twisted' and treq: + # Add Rollbar as a Twisted log handler which will report uncaught errors + twisted_log.addObserver(log_handler) # We will perform these transforms in order: # 1. Serialize the payload to be all python built-in objects @@ -1188,28 +1190,37 @@ def _send_payload_twisted(payload, access_token): try: _post_api_twisted('item/', payload, access_token=access_token) except Exception as e: + print '>>> UPPER BAD TIMES >>>' + # XXX(ezarowny): if I raise in _post_api_twisted I don't get here log.exception('Exception while posting item %r', e) +@inlineCallbacks def _post_api_twisted(path, payload, access_token=None): - def post_data_cb(data, resp): - resp._content = data - _parse_response(path, SETTINGS['access_token'], payload, resp) - - def post_cb(resp): - r = requests.Response() - r.status_code = resp.code - r.headers.update(resp.headers.getAllRawHeaders()) - return treq.content(resp).addCallback(post_data_cb, r) + # def handle_twisted_error(failure): + # print 'failure: {}'.format(failure) + # return False headers = {'Content-Type': ['application/json']} if access_token is not None: headers['X-Rollbar-Access-Token'] = [access_token] url = urljoin(SETTINGS['endpoint'], path) - d = treq.post(url, payload, headers=headers, - timeout=SETTINGS.get('timeout', DEFAULT_TIMEOUT)) - d.addCallback(post_cb) + try: + resp = yield treq.post(url, payload, headers=headers, + timeout=SETTINGS.get('timeout', DEFAULT_TIMEOUT)) + text = yield treq.content(resp) + except Exception as e: + print '>>> BAD TIMES >>>' + print e + # XXX(ezarowny): not sure what to do here? + + r = requests.Response() + r._content = text + r.status_code = resp.code + r.headers.update(resp.headers.getAllRawHeaders()) + + _parse_response(path, SETTINGS['access_token'], payload, r) def _send_failsafe(message, uuid, host): From 3ebf8363b261a09717d99c9d5f239d608ad6e7f7 Mon Sep 17 00:00:00 2001 From: Mark Williams Date: Fri, 16 Jun 2017 16:30:38 -0700 Subject: [PATCH 2/8] Stop logging recursion. --- rollbar/__init__.py | 44 +++++++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 23 deletions(-) diff --git a/rollbar/__init__.py b/rollbar/__init__.py index d1de15b4..3d786aba 100644 --- a/rollbar/__init__.py +++ b/rollbar/__init__.py @@ -106,7 +106,6 @@ def log_handler(event): return err = event['failure'] - # Don't report Rollbar internal errors to ourselves if issubclass(err.type, ApiException): log.error('Rollbar internal error: %s', err.value) @@ -1187,12 +1186,7 @@ def _post_api_tornado(path, payload, access_token=None): def _send_payload_twisted(payload, access_token): - try: - _post_api_twisted('item/', payload, access_token=access_token) - except Exception as e: - print '>>> UPPER BAD TIMES >>>' - # XXX(ezarowny): if I raise in _post_api_twisted I don't get here - log.exception('Exception while posting item %r', e) + _post_api_twisted('item/', payload, access_token=access_token) @inlineCallbacks @@ -1201,26 +1195,30 @@ def _post_api_twisted(path, payload, access_token=None): # print 'failure: {}'.format(failure) # return False - headers = {'Content-Type': ['application/json']} - if access_token is not None: - headers['X-Rollbar-Access-Token'] = [access_token] - - url = urljoin(SETTINGS['endpoint'], path) try: + headers = {'Content-Type': ['application/json']} + if access_token is not None: + headers['X-Rollbar-Access-Token'] = [access_token] + + url = urljoin(SETTINGS['endpoint'], path) resp = yield treq.post(url, payload, headers=headers, timeout=SETTINGS.get('timeout', DEFAULT_TIMEOUT)) text = yield treq.content(resp) - except Exception as e: - print '>>> BAD TIMES >>>' - print e - # XXX(ezarowny): not sure what to do here? - - r = requests.Response() - r._content = text - r.status_code = resp.code - r.headers.update(resp.headers.getAllRawHeaders()) - - _parse_response(path, SETTINGS['access_token'], payload, r) + r = requests.Response() + r._content = text + r.status_code = resp.code + r.headers.update(resp.headers.getAllRawHeaders()) + _parse_response(path, SETTINGS['access_token'], payload, r) + except: + # An exception that escapes here will be caught by Deferred + # and sent into Twisted's logging system. This will again + # invoke the observer that called this function, starting an + # infinite recursion. Catch all exceptions to prevent this, + # including exceptions in the logging system itself. + try: + log.exception("Failed to post to rollbar") + except: + pass def _send_failsafe(message, uuid, host): From 8c39fbd1b9b01a77d24a2fa29b057f3e23e802cb Mon Sep 17 00:00:00 2001 From: Eric Zarowny Date: Mon, 19 Jun 2017 15:00:47 -0700 Subject: [PATCH 3/8] Attempting to stop the loop --- rollbar/__init__.py | 70 ++++++++++++++++------ rollbar/test/twisted_tests/test_twisted.py | 8 ++- 2 files changed, 56 insertions(+), 22 deletions(-) diff --git a/rollbar/__init__.py b/rollbar/__init__.py index 3d786aba..be33625e 100644 --- a/rollbar/__init__.py +++ b/rollbar/__init__.py @@ -84,6 +84,7 @@ def wrap(*args, **kwargs): return func(*args, **kwargs) return wrap + try: from tornado.gen import coroutine as tornado_coroutine from tornado.httpclient import AsyncHTTPClient as TornadoAsyncHTTPClient @@ -94,10 +95,11 @@ def wrap(*args, **kwargs): try: import treq from twisted.internet.defer import inlineCallbacks - from twisted.internet.defer import returnValue + # from twisted.internet.task import TaskStopped + # from twisted.web._newclient import _WrapperException from twisted.python import log as twisted_log - def log_handler(event): + def twisted_log_observer(event): """ Default uncaught error handler """ @@ -109,6 +111,9 @@ def log_handler(event): # Don't report Rollbar internal errors to ourselves if issubclass(err.type, ApiException): log.error('Rollbar internal error: %s', err.value) + # XXX(ezarowny): could we possibly know it's a Twisted internal error here? + # elif issubclass(err.type, _WrapperException): + # log.error('Rollbar internal error: %s', err.value) else: report_exc_info((err.type, err.value, err.getTracebackObject())) except: @@ -292,7 +297,7 @@ def init(access_token, environment='production', **kw): agent_log = _create_agent_log() elif handler == 'twisted' and treq: # Add Rollbar as a Twisted log handler which will report uncaught errors - twisted_log.addObserver(log_handler) + twisted_log.addObserver(twisted_log_observer) # We will perform these transforms in order: # 1. Serialize the payload to be all python built-in objects @@ -403,7 +408,7 @@ def send_payload(payload, access_token): _send_payload_appengine(payload, access_token) elif handler == 'twisted': if treq is None: - log.error('Unable to find Treq') + log.error('treq and twisted are required for the twisted handler') return _send_payload_twisted(payload, access_token) else: @@ -1068,7 +1073,7 @@ def _build_server_data(): # argv does not always exist in embedded python environments argv = getattr(sys, 'argv', None) if argv: - server_data['argv'] = argv + server_data['argv'] = argv for key in ['branch', 'root']: if SETTINGS.get(key): @@ -1186,39 +1191,66 @@ def _post_api_tornado(path, payload, access_token=None): def _send_payload_twisted(payload, access_token): - _post_api_twisted('item/', payload, access_token=access_token) + try: + _post_api_twisted('item/', payload, access_token=access_token) + except Exception as e: + log.exception('Exception while posting item %r', e) @inlineCallbacks def _post_api_twisted(path, payload, access_token=None): + import datetime # def handle_twisted_error(failure): # print 'failure: {}'.format(failure) # return False - try: - headers = {'Content-Type': ['application/json']} - if access_token is not None: - headers['X-Rollbar-Access-Token'] = [access_token] + headers = {'Content-Type': ['application/json']} + if access_token is not None: + headers['X-Rollbar-Access-Token'] = [access_token] + + url = urljoin(SETTINGS['endpoint'], path) + + # resp = yield treq.post(url, payload, headers=headers, + # timeout=SETTINGS.get('timeout', DEFAULT_TIMEOUT)) + # text = yield treq.content(resp) + # + # r = requests.Response() + # r._content = text + # r.status_code = resp.code + # r.headers.update(resp.headers.getAllRawHeaders()) + # + # _parse_response(path, SETTINGS['access_token'], payload, r) - url = urljoin(SETTINGS['endpoint'], path) + try: resp = yield treq.post(url, payload, headers=headers, timeout=SETTINGS.get('timeout', DEFAULT_TIMEOUT)) text = yield treq.content(resp) - r = requests.Response() - r._content = text - r.status_code = resp.code - r.headers.update(resp.headers.getAllRawHeaders()) - _parse_response(path, SETTINGS['access_token'], payload, r) - except: + except Exception as e: # An exception that escapes here will be caught by Deferred # and sent into Twisted's logging system. This will again # invoke the observer that called this function, starting an # infinite recursion. Catch all exceptions to prevent this, # including exceptions in the logging system itself. + + # XXX(ezarowny): the removal and addition of the log observer here isn't + # having the effect of stopping the loop as I expected + twisted_log.removeObserver(twisted_log_observer) + try: - log.exception("Failed to post to rollbar") + now = datetime.datetime.utcnow().isoformat() + # log.exception('{} Failed to post to rollbar'.format(now)) + twisted_log.err(e, '{} - Failed to post to rollbar'.format(now)) except: - pass + print '>>>>>>> heyo' + + twisted_log.addObserver(twisted_log_observer) + else: + r = requests.Response() + r._content = text + r.status_code = resp.code + r.headers.update(resp.headers.getAllRawHeaders()) + + _parse_response(path, SETTINGS['access_token'], payload, r) def _send_failsafe(message, uuid, host): diff --git a/rollbar/test/twisted_tests/test_twisted.py b/rollbar/test/twisted_tests/test_twisted.py index 321a488d..f2cafb89 100644 --- a/rollbar/test/twisted_tests/test_twisted.py +++ b/rollbar/test/twisted_tests/test_twisted.py @@ -17,7 +17,9 @@ from twisted.test import proto_helpers from twisted.trial import unittest from twisted.internet import protocol - from twisted.python import log + from twisted.logger import Logger + + log = Logger() TWISTED_INSTALLED = True except ImportError: @@ -29,7 +31,7 @@ def dataReceived(self, data): try: number = int(data) except ValueError: - log.err() + log.failure('error') self.transport.write('error') else: self.transport.write(str(number**2)) @@ -55,7 +57,7 @@ def test_base_case(self, send_payload): @mock.patch('rollbar.send_payload') def test_caught_exception(self, send_payload): self.proto.dataReceived('rollbar') - self.assertEqual(self.tr.value(), "error") + self.assertEqual(self.tr.value(), 'error') errors = self.flushLoggedErrors(ValueError) self.assertEqual(len(errors), 1) From 69a3cae2d127ed1cb82b021b9396542db0d5e6a6 Mon Sep 17 00:00:00 2001 From: Andrew Weiss Date: Mon, 30 Apr 2018 14:12:48 -0700 Subject: [PATCH 4/8] clean up and make tests pass --- rollbar/__init__.py | 24 ++-------------------- rollbar/test/twisted_tests/test_twisted.py | 4 ++-- 2 files changed, 4 insertions(+), 24 deletions(-) diff --git a/rollbar/__init__.py b/rollbar/__init__.py index 5091518d..fcd55436 100644 --- a/rollbar/__init__.py +++ b/rollbar/__init__.py @@ -107,8 +107,6 @@ def wrap(*args, **kwargs): try: import treq from twisted.internet.defer import inlineCallbacks - # from twisted.internet.task import TaskStopped - # from twisted.web._newclient import _WrapperException from twisted.python import log as twisted_log def twisted_log_observer(event): @@ -123,9 +121,6 @@ def twisted_log_observer(event): # Don't report Rollbar internal errors to ourselves if issubclass(err.type, ApiException): log.error('Rollbar internal error: %s', err.value) - # XXX(ezarowny): could we possibly know it's a Twisted internal error here? - # elif issubclass(err.type, _WrapperException): - # log.error('Rollbar internal error: %s', err.value) else: report_exc_info((err.type, err.value, err.getTracebackObject())) except: @@ -1362,9 +1357,6 @@ def _send_payload_twisted(payload_str, access_token): @inlineCallbacks def _post_api_twisted(path, payload_str, access_token=None): import datetime - # def handle_twisted_error(failure): - # print 'failure: {}'.format(failure) - # return False headers = {'Content-Type': ['application/json']} if access_token is not None: @@ -1372,17 +1364,6 @@ def _post_api_twisted(path, payload_str, access_token=None): url = urljoin(SETTINGS['endpoint'], path) - # resp = yield treq.post(url, payload, headers=headers, - # timeout=SETTINGS.get('timeout', DEFAULT_TIMEOUT)) - # text = yield treq.content(resp) - # - # r = requests.Response() - # r._content = text - # r.status_code = resp.code - # r.headers.update(resp.headers.getAllRawHeaders()) - # - # _parse_response(path, SETTINGS['access_token'], payload, r) - try: resp = yield treq.post(url, payload_str, headers=headers, timeout=SETTINGS.get('timeout', DEFAULT_TIMEOUT)) @@ -1391,7 +1372,7 @@ def _post_api_twisted(path, payload_str, access_token=None): # An exception that escapes here will be caught by Deferred # and sent into Twisted's logging system. This will again # invoke the observer that called this function, starting an - # infinite recursion. Catch all exceptions to prevent this, + # infinite recursion. Catch all exceptions to prevent this, # including exceptions in the logging system itself. # XXX(ezarowny): the removal and addition of the log observer here isn't @@ -1400,10 +1381,9 @@ def _post_api_twisted(path, payload_str, access_token=None): try: now = datetime.datetime.utcnow().isoformat() - # log.exception('{} Failed to post to rollbar'.format(now)) twisted_log.err(e, '{} - Failed to post to rollbar'.format(now)) except: - print '>>>>>>> heyo' + pass twisted_log.addObserver(twisted_log_observer) else: diff --git a/rollbar/test/twisted_tests/test_twisted.py b/rollbar/test/twisted_tests/test_twisted.py index f565560e..fdd39461 100644 --- a/rollbar/test/twisted_tests/test_twisted.py +++ b/rollbar/test/twisted_tests/test_twisted.py @@ -41,7 +41,7 @@ class SquareFactory(protocol.Factory): class TwistedTest(unittest.TestCase): def setUp(self): - rollbar.init(TOKEN, 'twisted-test') + rollbar.init(TOKEN, 'twisted-test', handler='twisted') factory = SquareFactory() self.proto = factory.buildProtocol(('127.0.0.1', 0)) self.tr = proto_helpers.StringTransport() @@ -75,7 +75,7 @@ def test_caught_exception(self, send_payload): # @mock.patch('rollbar.send_payload') # def test_uncaught_exception(self, send_payload): # self.proto.dataReceived([8, 9]) - # self.assertEqual(self.tr.value(), "error") + # self.assertEqual(self.tr.value(), 'error') # errors = self.flushLoggedErrors(TypeError) # self.assertEqual(len(errors), 1) # From ac5d4cf2feea7f27162824508e68b031bc34853b Mon Sep 17 00:00:00 2001 From: Andrew Weiss Date: Mon, 30 Apr 2018 14:31:49 -0700 Subject: [PATCH 5/8] if we reinit with a different handler and that handler is twisted or agent then things will be broken, fix that --- rollbar/__init__.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/rollbar/__init__.py b/rollbar/__init__.py index fcd55436..3b4e8292 100644 --- a/rollbar/__init__.py +++ b/rollbar/__init__.py @@ -257,6 +257,8 @@ def _get_pylons_request(): _serialize_transform = None _initialized = False +_agent_log_installed = False +_twisted_log_installed = False from rollbar.lib.transforms.scrub_redact import REDACT_REF @@ -282,7 +284,16 @@ def init(access_token, environment='production', **kw): 'staging', 'yourname' **kw: provided keyword arguments will override keys in SETTINGS. """ - global SETTINGS, agent_log, _initialized, _transforms, _serialize_transform, _threads + global SETTINGS, agent_log, _initialized, _transforms, _serialize_transform,\ + _threads, _agent_log_installed, _twisted_log_installed + + handler = SETTINGS.get('handler') + if handler == 'agent' and not _agent_log_installed: + agent_log = _create_agent_log() + _agent_log_installed = True + elif handler == 'twisted' and treq and not _twisted_log_installed: + twisted_log.addObserver(twisted_log_observer) + _twisted_log_installed = True if _initialized: # NOTE: Temp solution to not being able to re-init. @@ -301,12 +312,6 @@ def init(access_token, environment='production', **kw): if SETTINGS.get('allow_logging_basic_config'): logging.basicConfig() - handler = SETTINGS.get('handler') - if handler == 'agent': - agent_log = _create_agent_log() - elif handler == 'twisted' and treq: - # Add Rollbar as a Twisted log handler which will report uncaught errors - twisted_log.addObserver(twisted_log_observer) # We will perform these transforms in order: # 1. Serialize the payload to be all python built-in objects From 3d0c72018bd361d892dfba5239c80dbd221ff771 Mon Sep 17 00:00:00 2001 From: Andrew Weiss Date: Mon, 30 Apr 2018 16:11:19 -0700 Subject: [PATCH 6/8] actually get the kw handler value for the pre settings merge check --- rollbar/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rollbar/__init__.py b/rollbar/__init__.py index 3b4e8292..8d26672a 100644 --- a/rollbar/__init__.py +++ b/rollbar/__init__.py @@ -288,6 +288,8 @@ def init(access_token, environment='production', **kw): _threads, _agent_log_installed, _twisted_log_installed handler = SETTINGS.get('handler') + if 'handler' in kw: + handler = kw.get('handler') if handler == 'agent' and not _agent_log_installed: agent_log = _create_agent_log() _agent_log_installed = True From a6795ff2c3d938280a5abbf69094189ac78995ee Mon Sep 17 00:00:00 2001 From: Andrew Weiss Date: Mon, 30 Apr 2018 16:39:01 -0700 Subject: [PATCH 7/8] fix codacy warning about threads global --- rollbar/__init__.py | 1 + 1 file changed, 1 insertion(+) diff --git a/rollbar/__init__.py b/rollbar/__init__.py index 8d26672a..ddaa0d8c 100644 --- a/rollbar/__init__.py +++ b/rollbar/__init__.py @@ -253,6 +253,7 @@ def _get_pylons_request(): _CURRENT_LAMBDA_CONTEXT = None # Set in init() +_threads = None _transforms = [] _serialize_transform = None From eea59ee7fc1d1a403e5e5a041465e3a79ac15789 Mon Sep 17 00:00:00 2001 From: Andrew Weiss Date: Tue, 1 May 2018 10:47:35 -0700 Subject: [PATCH 8/8] remove some old comments --- rollbar/__init__.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/rollbar/__init__.py b/rollbar/__init__.py index ddaa0d8c..6501be2e 100644 --- a/rollbar/__init__.py +++ b/rollbar/__init__.py @@ -1383,8 +1383,6 @@ def _post_api_twisted(path, payload_str, access_token=None): # infinite recursion. Catch all exceptions to prevent this, # including exceptions in the logging system itself. - # XXX(ezarowny): the removal and addition of the log observer here isn't - # having the effect of stopping the loop as I expected twisted_log.removeObserver(twisted_log_observer) try: