Merge remote-tracking branch 'origin/master' into dev

CHANGES.rst

@@ -23,6 +23,20 @@ Changes by Version
 - Fixed bug that caused server to continue listening for incoming connections
   despite closing the channel.
 
+
+0.17.11 (2015-10-19)
+--------------------
+
+- Fix a bug that caused ``after_send_error`` event to never be fired.
+- Request tracing information is now propagated to error responses.
+
+
+0.17.10 (2015-10-16)
+--------------------
+
+- Support thriftrw 0.5.
+
+
 0.17.9 (2015-10-15)
 -------------------
 

setup.py

@@ -3,7 +3,7 @@
 
 setup(
     name='tchannel',
-    version='0.17.10.dev0',
+    version='0.17.12.dev0',
     author='Abhinav Gupta, Aiden Scandella, Bryce Lampe, Grayson Koonce, Junchao Wu',
     author_email='abg@uber.com',
     description='Network multiplexing and framing protocol for RPC',

tchannel/errors.py

@@ -20,6 +20,8 @@
 
 from __future__ import absolute_import
 
+from tchannel.zipkin.trace import Trace
+
 
 #: The request timed out.
 TIMEOUT = 0x01
@@ -77,7 +79,7 @@ def __init__(
         tracing=None,
     ):
         super(TChannelError, self).__init__(description)
-        self.tracing = tracing
+        self.tracing = tracing or Trace()
         self.id = id
         self.description = description
 

tchannel/tornado/connection.py

@@ -35,15 +35,14 @@
 from .. import glossary
 from .. import messages
 from ..errors import NetworkError
-from ..errors import FatalProtocolError
 from ..errors import TChannelError
 from ..event import EventType
 from ..io import BytesIO
 from ..messages.common import PROTOCOL_VERSION
 from ..messages.common import FlagsType
 from ..messages.common import StreamState
-from ..messages.error import ErrorMessage
 from ..messages.types import Types
+from .message_factory import build_raw_error_message
 from .message_factory import MessageFactory
 from .util import chain
 
@@ -467,25 +466,20 @@ def serve(self, handler):
                 # TODO Send error frame back
                 logging.exception("Failed to process %s", repr(message))
 
-    def send_error(self, code, description, message_id):
+    def send_error(self, error):
         """Convenience method for writing Error frames up the wire.
 
-        :param code:
-            Error code
-        :param description:
-            Error description
-        :param message_id:
-            Message in response to which this error is being sent
+        :param error:
+            TChannel Error. :py:class`tchannel.errors.TChannelError`.
         """
-        if code not in ErrorMessage.ERROR_CODES.keys():
-            raise FatalProtocolError(code)
-
-        return self._write(
-            ErrorMessage(
-                code=code,
-                description=description,
-                id=message_id,
-            ),
+
+        error_message = build_raw_error_message(error)
+        write_future = self._write(error_message)
+        write_future.add_done_callback(
+            lambda f: self.tchannel.event_emitter.fire(
+                EventType.after_send_error,
+                error,
+            )
         )
 
     def ping(self):

tchannel/tornado/dispatch.py

@@ -33,10 +33,10 @@
 from tchannel.response import response_from_mixed
 from ..context import request_context
 from ..errors import BadRequestError
+from ..errors import UnexpectedError
 from ..errors import TChannelError
 from ..event import EventType
 from ..messages import Types
-from ..messages.error import ErrorCode
 from ..serializer.raw import RawSerializer
 from .response import Response as DeprecatedResponse
 
@@ -100,6 +100,7 @@ def handle_pre_call(self, message, connection):
         :param message: CallRequestMessage or CallRequestContinueMessage
         :param connection: tornado connection
         """
+        req = None
         try:
             req = connection.request_message_factory.build(message)
             # message_factory will create Request only when it receives
@@ -110,12 +111,9 @@ def handle_pre_call(self, message, connection):
 
         except TChannelError as e:
             log.warn('Received a bad request.', exc_info=True)
-
-            connection.send_error(
-                e.code,
-                e.message,
-                message.id,
-            )
+            if req:
+                e.tracing = req.tracing
+            connection.send_error(e)
 
     @tornado.gen.coroutine
     def handle_call(self, request, connection):
@@ -149,14 +147,18 @@ def handle_call(self, request, connection):
         expected_as = handler.req_serializer.name
 
         if request.endpoint in self.handlers and requested_as != expected_as:
-            connection.send_error(
-                ErrorCode.bad_request,
-                "Your serialization was '%s' but the server expected '%s'" % (
-                    requested_as,
-                    expected_as,
+            connection.send_error(BadRequestError(
+                description=(
+                    "Server expected a '%s' but request is '%s'"
+                    % (
+                        expected_as,
+                        requested_as,
+                    )
                 ),
-                request.id,
-            )
+                id=request.id,
+                tracing=request.tracing,
+            ))
+
             raise gen.Return(None)
 
         request.serializer = handler.req_serializer
@@ -220,20 +222,22 @@ def handle_call(self, request, connection):
 
             response.flush()
         except TChannelError as e:
-            connection.send_error(
-                e.code,
-                e.message,
-                request.id,
-            )
+            e.tracing = request.tracing
+            e.id = request.id
+            connection.send_error(e)
         except Exception as e:
-            msg = "An unexpected error has occurred from the handler"
-            log.exception(msg)
+            log.exception("Unexpected Error: '%s'" % e.message)
 
-            response.set_exception(TChannelError(e.message))
+            error = UnexpectedError(
+                description='An unexpected error occurred from the handler',
+                id=request.id,
+                tracing=request.tracing,
+            )
+            response.set_exception(error)
             connection.request_message_factory.remove_buffer(response.id)
 
-            connection.send_error(ErrorCode.unexpected, msg, response.id)
-            tchannel.event_emitter.fire(EventType.on_exception, request, e)
+            connection.send_error(error)
+            tchannel.event_emitter.fire(EventType.on_exception, request, error)
 
         raise gen.Return(response)
 

tchannel/tornado/message_factory.py

@@ -52,6 +52,7 @@
 def build_raw_error_message(protocol_exception):
     """build protocol level error message based on Error object"""
     message = ErrorMessage(
+        id=protocol_exception.id,
         code=protocol_exception.code,
         tracing=Tracing(
             protocol_exception.tracing.span_id,
@@ -281,7 +282,9 @@ def build(self, message):
             if context is None:
                 # missing call msg before continue msg
                 raise FatalProtocolError(
-                    "missing call message after receiving continue message")
+                    "missing call message after receiving continue message",
+                    message.id,
+                )
 
             # find the incompleted stream
             dst = 0
@@ -388,7 +391,10 @@ def verify_message(self, message):
                 self.in_checksum.pop(message.id)
         else:
             self.in_checksum.pop(message.id, None)
-            raise InvalidChecksumError("Checksum does not match!")
+            raise InvalidChecksumError(
+                description="Checksum does not match!",
+                id=message.id,
+            )
 
     @staticmethod
     def close_argstream(request, num):
@@ -408,7 +414,9 @@ def set_inbound_exception(self, protocol_error):
         if reqres is None:
             # missing call msg before continue msg
             raise FatalProtocolError(
-                "missing call message after receiving continue message")
+                "missing call message after receiving continue message",
+                id=protocol_error.id,
+            )
 
         # find the incompleted stream
         dst = 0

tests/integration/test_retry.py

@@ -40,11 +40,7 @@
 @tornado.gen.coroutine
 def handler_error(request, response):
     yield tornado.gen.sleep(0.01)
-    response.connection.send_error(
-        ErrorCode.busy,
-        "retry",
-        response.id,
-    )
+    response.connection.send_error(BusyError("retry", request.id))
     # stop normal response streams
     response.set_exception(TChannelError("stop stream"))
 
@@ -123,7 +119,7 @@ def test_retry_on_error_fail():
                 headers={
                     're': retry.CONNECTION_ERROR_AND_TIMEOUT
                 },
-                ttl=0.02,
+                ttl=1,
                 retry_limit=2,
             )
 

tests/test_event.py

@@ -20,9 +20,13 @@
 
 from __future__ import absolute_import
 
+import mock
 import pytest
 from mock import MagicMock
 
+from tchannel import TChannel
+from tchannel import schemes
+from tchannel.errors import BadRequestError
 from tchannel.event import EventEmitter
 from tchannel.event import EventHook
 from tchannel.event import EventRegistrar
@@ -60,3 +64,24 @@ def bar():
 
     assert called[0] is True
     assert called[1] is True
+
+
+@pytest.mark.gen_test
+def test_after_send_error_event_called():
+    tchannel = TChannel('test')
+    tchannel.listen()
+    with mock.patch(
+        'tchannel.event.EventEmitter.fire', autospec=True,
+    ) as mock_fire:
+        mock_fire.return_value = None
+        with pytest.raises(BadRequestError):
+            yield tchannel.call(
+                scheme=schemes.RAW,
+                service='test',
+                arg1='endpoint',
+                hostport=tchannel.hostport,
+                timeout=0.02,
+            )
+        mock_fire.assert_any_call(
+            mock.ANY, EventType.after_send_error, mock.ANY,
+        )

tests/test_trace.py

@@ -0,0 +1,61 @@
+# Copyright (c) 2015 Uber Technologies, Inc.
+#
+# Permission is hereby granted, free of charge, to any person obtaining a copy
+# of this software and associated documentation files (the "Software"), to deal
+# in the Software without restriction, including without limitation the rights
+# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+# copies of the Software, and to permit persons to whom the Software is
+# furnished to do so, subject to the following conditions:
+#
+# The above copyright notice and this permission notice shall be included in
+# all copies or substantial portions of the Software.
+#
+# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+# THE SOFTWARE.
+
+from __future__ import absolute_import
+
+import pytest
+
+from tchannel import TChannel, schemes
+from tchannel.errors import BadRequestError
+from tchannel.event import EventHook
+
+
+@pytest.mark.gen_test
+def test_error_trace():
+    tchannel = TChannel('test')
+
+    class ErrorEventHook(EventHook):
+        def __init__(self):
+            self.request_trace = None
+            self.error_trace = None
+
+        def before_receive_request(self, request):
+            self.request_trace = request.tracing
+
+        def after_send_error(self, error):
+            self.error_trace = error.tracing
+
+    hook = ErrorEventHook()
+    tchannel.hooks.register(hook)
+
+    tchannel.listen()
+
+    with pytest.raises(BadRequestError):
+        yield tchannel.call(
+            scheme=schemes.RAW,
+            service='test',
+            arg1='endpoint',
+            hostport=tchannel.hostport,
+            timeout=0.02,
+        )
+
+    assert hook.error_trace
+    assert hook.request_trace
+    assert hook.error_trace == hook.request_trace

tests/tornado/test_dispatch.py

@@ -68,7 +68,7 @@ def handler(req, response):
 def test_default_fallback_behavior(dispatcher, req, connection):
     """Undefined endpoints return 'Bad Request' errors."""
     yield dispatcher.handle_call(req, connection)
-    assert connection.send_error.call_args[0][0] == ErrorCode.bad_request
+    assert connection.send_error.call_args[0][0].code == ErrorCode.bad_request
 
 
 @pytest.mark.gen_test