From 213969742eba56148b46c7d93b24dc73adae890b Mon Sep 17 00:00:00 2001 From: Jason Robinson Date: Mon, 14 Nov 2016 18:36:33 +0200 Subject: [PATCH] Fix UnicodeDecodeError when failing jobs Worker handle_exception and move_to_failed_queue couldn't handle a situation where the exception raised had non-ascii characters. This caused a UnicodeDecodeError when trying to format the exception strings. If on Python 2, ensure strings get decoded before building the exception string. Closes #482 --- rq/worker.py | 13 ++++++++++--- tests/test_worker.py | 22 ++++++++++++++++++++++ 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/rq/worker.py b/rq/worker.py index 16f49cb..b182bb2 100644 --- a/rq/worker.py +++ b/rq/worker.py @@ -742,8 +742,9 @@ class Worker(object): def handle_exception(self, job, *exc_info): """Walks the exception handler stack to delegate exception handling.""" - exc_string = ''.join(traceback.format_exception_only(*exc_info[:2]) + - traceback.format_exception(*exc_info)) + exc_string = self._get_safe_exception_string( + traceback.format_exception_only(*exc_info[:2]) + traceback.format_exception(*exc_info) + ) self.log.error(exc_string, exc_info=True, extra={ 'func': job.func_name, 'arguments': job.args, @@ -765,10 +766,16 @@ class Worker(object): def move_to_failed_queue(self, job, *exc_info): """Default exception handler: move the job to the failed queue.""" - exc_string = ''.join(traceback.format_exception(*exc_info)) + exc_string = self._get_safe_exception_string(traceback.format_exception(*exc_info)) self.log.warning('Moving job to {0!r} queue'.format(self.failed_queue.name)) self.failed_queue.quarantine(job, exc_info=exc_string) + def _get_safe_exception_string(self, exc_strings): + """Ensure list of exception strings is decoded on Python 2 and joined as one string safely.""" + if sys.version_info[0] < 3: + exc_strings = [exc.decode("utf-8") for exc in exc_strings] + return ''.join(exc_strings) + def push_exc_handler(self, handler_func): """Pushes an exception handler onto the exc handler stack.""" self._exc_handlers.append(handler_func) diff --git a/tests/test_worker.py b/tests/test_worker.py index 0e782cf..8ad43a0 100644 --- a/tests/test_worker.py +++ b/tests/test_worker.py @@ -15,6 +15,7 @@ import sys import pytest import mock +from mock import Mock from tests import RQTestCase, slow from tests.fixtures import (create_file, create_file_after_timeout, @@ -894,3 +895,24 @@ class HerokuWorkerShutdownTestCase(TimeoutTestCase, RQTestCase): w._horse_pid = 19999 w.handle_warm_shutdown_request() + + +class TestExceptionHandlerMessageEncoding(RQTestCase): + def test_handle_exception_handles_non_ascii_in_exception_message(self): + """Test that handle_exception doesn't crash on non-ascii in exception message.""" + self.worker.handle_exception(Mock(), *self.exc_info) + + def test_move_to_failed_queue_handles_non_ascii_in_exception_message(self): + """Test that move_to_failed_queue doesn't crash on non-ascii in exception message.""" + self.worker.move_to_failed_queue(Mock(), *self.exc_info) + + def setUp(self): + super(TestExceptionHandlerMessageEncoding, self).setUp() + self.worker = Worker("foo") + self.worker._exc_handlers = [] + self.worker.failed_queue = Mock() + # Mimic how exception info is actually passed forwards + try: + raise Exception(u"💪") + except: + self.exc_info = sys.exc_info()