From 00eb8632aedb6bf56a416ca28dfcb34b5d360e51 Mon Sep 17 00:00:00 2001 From: Ben Darnell Date: Sat, 7 Apr 2018 23:39:34 -0400 Subject: [PATCH] httpclient: Deprecation warning for narrowed scope of raise_error=False --- tornado/httpclient.py | 13 +++++++ tornado/test/simple_httpclient_test.py | 52 +++++++++++++------------- tornado/testing.py | 19 +++++++++- 3 files changed, 57 insertions(+), 27 deletions(-) diff --git a/tornado/httpclient.py b/tornado/httpclient.py index d41a02730..4ed96e4d4 100644 --- a/tornado/httpclient.py +++ b/tornado/httpclient.py @@ -245,6 +245,12 @@ class AsyncHTTPClient(Configurable): The ``callback`` argument is deprecated and will be removed in 6.0. Use the returned `.Future` instead. + The ``raise_error=False`` argument currently suppresses + *all* errors, encapsulating them in `HTTPResponse` objects + with a 599 response code. This will change in Tornado 6.0: + ``raise_error=False`` will only affect the `HTTPError` + raised when a non-200 response code is used. + """ if self._closed: raise RuntimeError("fetch() called on closed AsyncHTTPClient") @@ -279,8 +285,13 @@ class AsyncHTTPClient(Configurable): def handle_response(response): if raise_error and response.error: + if isinstance(response.error, HTTPError): + response.error.response = response future.set_exception(response.error) else: + if response.error and not response._error_is_response_code: + warnings.warn("raise_error=False will allow '%s' to be raised in the future" % + response.error, DeprecationWarning) future_set_result_unless_cancelled(future, response) self.fetch_impl(request, handle_response) return future @@ -594,8 +605,10 @@ class HTTPResponse(object): self.effective_url = request.url else: self.effective_url = effective_url + self._error_is_response_code = False if error is None: if self.code < 200 or self.code >= 300: + self._error_is_response_code = True self.error = HTTPError(self.code, message=self.reason, response=self) else: diff --git a/tornado/test/simple_httpclient_test.py b/tornado/test/simple_httpclient_test.py index 8856c2369..a8893a99e 100644 --- a/tornado/test/simple_httpclient_test.py +++ b/tornado/test/simple_httpclient_test.py @@ -23,7 +23,7 @@ from tornado.simple_httpclient import SimpleAsyncHTTPClient from tornado.test.httpclient_test import ChunkHandler, CountdownHandler, HelloWorldHandler, RedirectHandler # noqa: E501 from tornado.test import httpclient_test from tornado.testing import AsyncHTTPTestCase, AsyncHTTPSTestCase, AsyncTestCase, ExpectLog, gen_test -from tornado.test.util import skipOnTravis, skipIfNoIPv6, refusing_port, skipBefore35, exec_test +from tornado.test.util import skipOnTravis, skipIfNoIPv6, refusing_port, skipBefore35, exec_test, ignore_deprecation from tornado.web import RequestHandler, Application, asynchronous, url, stream_request_body @@ -246,13 +246,14 @@ class SimpleHTTPClientTestMixin(object): return Future() # never completes with closing(self.create_client(resolver=TimeoutResolver())) as client: - response = yield client.fetch(self.get_url('/hello'), - connect_timeout=timeout, - raise_error=False) - self.assertEqual(response.code, 599) - self.assertTrue(timeout_min < response.request_time < timeout_max, - response.request_time) - self.assertEqual(str(response.error), "HTTP 599: Timeout while connecting") + with ignore_deprecation(): + response = yield client.fetch(self.get_url('/hello'), + connect_timeout=timeout, + raise_error=False) + self.assertEqual(response.code, 599) + self.assertTrue(timeout_min < response.request_time < timeout_max, + response.request_time) + self.assertEqual(str(response.error), "HTTP 599: Timeout while connecting") @skipOnTravis def test_request_timeout(self): @@ -346,23 +347,24 @@ class SimpleHTTPClientTestMixin(object): response.error) def test_queue_timeout(self): - with closing(self.create_client(max_clients=1)) as client: - # Wait for the trigger request to block, not complete. - fut1 = client.fetch(self.get_url('/trigger'), - request_timeout=10, raise_error=False) - self.wait() - fut2 = client.fetch(self.get_url('/hello'), - connect_timeout=0.1, raise_error=False) - fut2.add_done_callback(self.stop) - response = self.wait().result() - - self.assertEqual(response.code, 599) - self.assertTrue(response.request_time < 1, response.request_time) - self.assertEqual(str(response.error), "HTTP 599: Timeout in request queue") - self.triggers.popleft()() - fut1.add_done_callback(self.stop) - self.wait() - fut1.result() + with ignore_deprecation(): + with closing(self.create_client(max_clients=1)) as client: + # Wait for the trigger request to block, not complete. + fut1 = client.fetch(self.get_url('/trigger'), + request_timeout=10, raise_error=False) + self.wait() + fut2 = client.fetch(self.get_url('/hello'), + connect_timeout=0.1, raise_error=False) + fut2.add_done_callback(self.stop) + response = self.wait().result() + + self.assertEqual(response.code, 599) + self.assertTrue(response.request_time < 1, response.request_time) + self.assertEqual(str(response.error), "HTTP 599: Timeout in request queue") + self.triggers.popleft()() + fut1.add_done_callback(self.stop) + self.wait() + fut1.result() def test_no_content_length(self): response = self.fetch("/no_content_length") diff --git a/tornado/testing.py b/tornado/testing.py index c3450272a..ab1779f20 100644 --- a/tornado/testing.py +++ b/tornado/testing.py @@ -13,7 +13,7 @@ from __future__ import absolute_import, division, print_function try: from tornado import gen - from tornado.httpclient import AsyncHTTPClient + from tornado.httpclient import AsyncHTTPClient, HTTPError, HTTPResponse from tornado.httpserver import HTTPServer from tornado.simple_httpclient import SimpleAsyncHTTPClient from tornado.ioloop import IOLoop, TimeoutError @@ -404,12 +404,27 @@ class AsyncHTTPTestCase(AsyncTestCase): .. versionchanged:: 5.0 Added support for absolute URLs. + .. deprecated:: 5.1 + + This method currently turns any exception into an + `.HTTPResponse` with status code 599. In Tornado 6.0, + errors other than `tornado.httpclient.HTTPError` + will be passed through, and this method will only suppress + errors that would be raised due to non-200 response codes. + """ if path.lower().startswith(('http://', 'https://')): url = path else: url = self.get_url(path) - return self.io_loop.run_sync(lambda: self.http_client.fetch(url, raise_error=False, **kwargs)) + try: + return self.io_loop.run_sync(lambda: self.http_client.fetch(url, **kwargs)) + except HTTPError as e: + if e.response is not None: + return e.response + return HTTPResponse(None, 599, error=e, effective_url='unknown') + except Exception as e: + return HTTPResponse(None, 599, error=e, effective_url='unknown') def get_httpserver_options(self): """May be overridden by subclasses to return additional -- 2.47.2