httpclient: Deprecation warning for narrowed scope of raise_error=False

diff --git a/tornado/httpclient.py b/tornado/httpclient.py
index d41a027301d7b0549515a48f1411f2d2361ccb90..4ed96e4d40551f4bca5c5053a5b683f8cc69a38c 100644 (file)
--- 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 8856c2369bf4ea8332d0cc8b335b92e6d87d6a8e..a8893a99ebab37966009561ff33538f40d62720e 100644 (file)
--- 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 c3450272aebcf9a2c03755ba0e8398b6d5ad0632..ab1779f20be2e0739a4b752b463d71f9cec9cd07 100644 (file)
--- 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