Fix Tornado error handling.

The error handling was quite broken - the exception handlers were never invoked, and they used methods which have been removed.

This fixes #601

raven/contrib/tornado/__init__.py

@@ -7,6 +7,9 @@ raven.contrib.tornado
 """
 from __future__ import absolute_import
 
+from functools import partial
+
+from tornado import ioloop
 from tornado.httpclient import AsyncHTTPClient, HTTPError
 
 from raven.base import Client
@@ -51,30 +54,25 @@ class AsyncSentryClient(Client):
             headers = {}
 
         if not self.state.should_try():
-            message = self._get_log_message(data)
-            self.error_logger.error(message)
+            data = self.decode(data)
+            self._log_failed_submission(data)
             return
 
+        future = self._send_remote(
+            url=url, data=data, headers=headers, callback=callback
+        )
+        ioloop.IOLoop.current().add_future(future, partial(self._handle_result, url, data))
+        return future
+
+    def _handle_result(self, url, data, future):
         try:
-            self._send_remote(
-                url=url, data=data, headers=headers, callback=callback
-            )
+            future.result()
         except HTTPError as e:
-            body = e.response.body
-            self.error_logger.error(
-                'Unable to reach Sentry log server: %s '
-                '(url: %%s, body: %%s)' % (e,),
-                url, body, exc_info=True,
-                extra={'data': {'body': body, 'remote_url': url}}
-            )
+            data = self.decode(data)
+            self._failed_send(e, url, data)
         except Exception as e:
-            self.error_logger.error(
-                'Unable to reach Sentry log server: %s (url: %%s)' % (e,),
-                url, exc_info=True, extra={'data': {'remote_url': url}}
-            )
-            message = self._get_log_message(data)
-            self.error_logger.error('Failed to submit message: %r', message)
-            self.state.set_fail()
+            data = self.decode(data)
+            self._failed_send(e, url, data)
         else:
             self.state.set_success()
 

raven/transport/tornado.py

@@ -7,6 +7,9 @@ raven.transport.tornado
 """
 from __future__ import absolute_import
 
+from functools import partial
+
+from raven.transport.base import AsyncTransport
 from raven.transport.http import HTTPTransport
 
 try:
@@ -17,7 +20,7 @@ except:
     has_tornado = False
 
 
-class TornadoHTTPTransport(HTTPTransport):
+class TornadoHTTPTransport(AsyncTransport, HTTPTransport):
 
     scheme = ['tornado+http', 'tornado+https']
 
@@ -27,7 +30,7 @@ class TornadoHTTPTransport(HTTPTransport):
 
         super(TornadoHTTPTransport, self).__init__(parsed_url, *args, **kwargs)
 
-    def send(self, data, headers):
+    def async_send(self, data, headers, success_cb, failure_cb):
         kwargs = dict(method='POST', headers=headers, body=data)
         kwargs["validate_cert"] = self.verify_ssl
         kwargs["connect_timeout"] = self.timeout
@@ -37,7 +40,21 @@ class TornadoHTTPTransport(HTTPTransport):
         if ioloop.IOLoop.initialized():
             client = AsyncHTTPClient()
             kwargs['callback'] = None
+
+            future = client.fetch(self._url, **kwargs)
+            ioloop.IOLoop.current().add_future(future, partial(self.handler, success_cb, failure_cb))
         else:
             client = HTTPClient()
+            try:
+                client.fetch(self._url, **kwargs)
+                success_cb()
+            except Exception as e:
+                failure_cb(e)
 
-        client.fetch(self._url, **kwargs)
+    @staticmethod
+    def handler(future, success, error):
+        try:
+            future.result()
+            success()
+        except Exception as e:
+            error(e)

tests/contrib/tornado/tests.py

@@ -3,6 +3,8 @@ from __future__ import unicode_literals
 
 from mock import patch
 from tornado import web, gen, testing
+from tornado.concurrent import Future
+from tornado.httpclient import HTTPError
 from raven.contrib.tornado import SentryMixin, AsyncSentryClient
 from raven.utils import six
 
@@ -212,3 +214,26 @@ class TornadoAsyncClientTestCase(testing.AsyncHTTPTestCase):
 
         user_data = kwargs['user']
         self.assertEqual(user_data['is_authenticated'], False)
+
+    @testing.gen_test
+    def test_sending_to_unresponsive_sentry_server_logs_error(self):
+        client = self.get_app().sentry_client
+        with patch.object(client, '_failed_send') as mock_failed:
+            client.send()
+
+            yield gen.sleep(0.01)  # we need to run after the async send
+            assert mock_failed.called
+
+    @testing.gen_test
+    def test_non_successful_responses_marks_client_as_failed(self):
+        client = self.get_app().sentry_client
+        with patch.object(client, '_failed_send') as mock_failed:
+            with patch.object(client, '_send_remote') as mock_send:
+
+                f = Future()
+                f.set_exception(HTTPError(499, 'error'))
+                mock_send.return_value = f
+                client.send()
+
+                yield gen.sleep(0.01)  # we need to run after the async send
+                assert mock_failed.called