From 5998afd1e06c2b8a75a84c90c1748c77be249531 Mon Sep 17 00:00:00 2001 From: Andrew Rabert Date: Thu, 29 Aug 2019 16:01:55 -0400 Subject: [PATCH] Add handling of tornado.httpclient.HTTPError Only handle OSError and httpclient.HTTPError. - We can assume any instance of httpclient.HTTPError is a failure unrelated to the status code of the response. This is due to sprockets.mixins.http setting `raise_error=False` when making the request. - ConnectionError and socket.gaierror are both instances of OSError - CurlError is an instance of tornado.httpclient.HTTPError - Other instances of tornado.httpclient.HTTPError include HTTPTimeoutError and HTTPStreamClosedError Also needed to fix the instance assertions in the test. It was always true as it was evaluating the truthyness of a list - not the isinstance values. --- docs/history.rst | 2 ++ sprockets/mixins/http/__init__.py | 10 +--------- tests.py | 19 ++++++++++++++++++- 3 files changed, 21 insertions(+), 10 deletions(-) diff --git a/docs/history.rst b/docs/history.rst index deddf1c..f32866d 100644 --- a/docs/history.rst +++ b/docs/history.rst @@ -3,6 +3,8 @@ Version History `Next Release ------------- +- Add handling of ``tornado.httpclient.HTTPTimeoutError`` and + ``tornado.httpclient.HTTPStreamClosedError`` exceptions - Fix documentation builds - Update documentation links to readthedocs.io diff --git a/sprockets/mixins/http/__init__.py b/sprockets/mixins/http/__init__.py index aa2cd28..99e28bd 100644 --- a/sprockets/mixins/http/__init__.py +++ b/sprockets/mixins/http/__init__.py @@ -9,17 +9,12 @@ import asyncio import functools import logging import os -import socket import time from urllib import parse from ietfparse import algorithms, errors, headers from sprockets.mixins.mediatype import transcoders from tornado import httpclient -try: - from tornado.curl_httpclient import CurlError -except ModuleNotFoundError: - CurlError = OSError __version__ = '2.1.0' @@ -352,10 +347,7 @@ class HTTPClientMixin: raise_error=False, validate_cert=validate_cert, allow_nonstandard_methods=allow_nonstandard_methods) - except (ConnectionError, - CurlError, - OSError, - socket.gaierror) as error: + except (OSError, httpclient.HTTPError) as error: response.append_exception(error) LOGGER.warning( 'HTTP Request Error for %s to %s attempt %i of %i: %s', diff --git a/tests.py b/tests.py index a6ab4a3..6556194 100644 --- a/tests.py +++ b/tests.py @@ -219,7 +219,24 @@ class MixinTestCase(testing.AsyncHTTPTestCase): self.assertIsNone(response.headers) self.assertIsNone(response.links) self.assertIsNone(response.raw) - self.assertTrue([isinstance(e, OSError) for e in response.exceptions]) + for e in response.exceptions: + self.assertIsInstance(e, OSError) + + @testing.gen_test + def test_tornado_httpclient_errors(self): + with mock.patch( + 'tornado.httpclient.AsyncHTTPClient.fetch') as fetch: + fetch.side_effect = httpclient.HTTPError(599) + response = yield self.mixin.http_fetch(self.get_url('/test')) + self.assertFalse(response.ok) + self.assertEqual(response.code, 599) + self.assertEqual(response.attempts, 3) + self.assertIsNone(response.body) + self.assertIsNone(response.headers) + self.assertIsNone(response.links) + self.assertIsNone(response.raw) + for e in response.exceptions: + self.assertIsInstance(e, httpclient.HTTPError) @testing.gen_test def test_without_correlation_id_behavior(self):