From d736f93891f78f2fa22bda677c40f69b5064851d Mon Sep 17 00:00:00 2001 From: Ben Darnell Date: Fri, 6 Jul 2018 19:25:15 -0400 Subject: [PATCH] httpclient: Remove callback argument and deprecated error behavior --- tornado/httpclient.py | 48 +++++++--------------------- tornado/test/curl_httpclient_test.py | 15 --------- tornado/test/httpclient_test.py | 22 ------------- tornado/test/httpserver_test.py | 5 +-- tornado/test/testing_test.py | 43 +++++++++++-------------- 5 files changed, 33 insertions(+), 100 deletions(-) diff --git a/tornado/httpclient.py b/tornado/httpclient.py index 5ed2ee677..abfd8f24c 100644 --- a/tornado/httpclient.py +++ b/tornado/httpclient.py @@ -220,7 +220,7 @@ class AsyncHTTPClient(Configurable): raise RuntimeError("inconsistent AsyncHTTPClient cache") del self._instance_cache[self.io_loop] - def fetch(self, request, callback=None, raise_error=True, **kwargs): + def fetch(self, request, raise_error=True, **kwargs): """Executes a request, asynchronously returning an `HTTPResponse`. The request may be either a string URL or an `HTTPRequest` object. @@ -240,17 +240,14 @@ class AsyncHTTPClient(Configurable): Instead, you must check the response's ``error`` attribute or call its `~HTTPResponse.rethrow` method. - .. deprecated:: 5.1 + .. versionchanged:: 6.0 - 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. + The ``callback`` argument was removed. Use the returned + `.Future` instead. + The ``raise_error=False`` argument only affects the + `HTTPError` raised when a non-200 response code is used, + instead of suppressing all errors. """ if self._closed: raise RuntimeError("fetch() called on closed AsyncHTTPClient") @@ -265,34 +262,13 @@ class AsyncHTTPClient(Configurable): request.headers = httputil.HTTPHeaders(request.headers) request = _RequestProxy(request, self.defaults) future = Future() - if callback is not None: - warnings.warn("callback arguments are deprecated, use the returned Future instead", - DeprecationWarning) - callback = stack_context.wrap(callback) - - def handle_future(future): - exc = future.exception() - if isinstance(exc, HTTPError) and exc.response is not None: - response = exc.response - elif exc is not None: - response = HTTPResponse( - request, 599, error=exc, - request_time=time.time() - request.start_time) - else: - response = future.result() - self.io_loop.add_callback(callback, response) - future.add_done_callback(handle_future) 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) + if response.error: + if raise_error or not response._error_is_response_code: + future.set_exception(response.error) + return + future_set_result_unless_cancelled(future, response) self.fetch_impl(request, handle_response) return future diff --git a/tornado/test/curl_httpclient_test.py b/tornado/test/curl_httpclient_test.py index 4230d4cd6..347bde641 100644 --- a/tornado/test/curl_httpclient_test.py +++ b/tornado/test/curl_httpclient_test.py @@ -132,21 +132,6 @@ class CurlHTTPClientTestCase(AsyncHTTPTestCase): response = self.fetch('/custom_fail_reason') self.assertEqual(str(response.error), "HTTP 400: Custom reason") - def test_failed_setup(self): - self.http_client = self.create_client(max_clients=1) - for i in range(5): - with ignore_deprecation(): - response = self.fetch(u'/ユニコード') - self.assertIsNot(response.error, None) - - with self.assertRaises((UnicodeEncodeError, HTTPClientError)): - # This raises UnicodeDecodeError on py3 and - # HTTPClientError(404) on py2. The main motivation of - # this test is to ensure that the UnicodeEncodeError - # during the setup phase doesn't lead the request to - # be dropped on the floor. - response = self.fetch(u'/ユニコード', raise_error=True) - def test_digest_auth_non_ascii(self): response = self.fetch('/digest_non_ascii', auth_mode='digest', auth_username='foo', auth_password='barユ£') diff --git a/tornado/test/httpclient_test.py b/tornado/test/httpclient_test.py index 35426a7df..d994ab8d0 100644 --- a/tornado/test/httpclient_test.py +++ b/tornado/test/httpclient_test.py @@ -430,28 +430,6 @@ X-XSS-Protection: 1; self.assertEqual(response.code, 304) self.assertEqual(response.headers['Content-Length'], '42') - def test_final_callback_stack_context(self): - # The final callback should be run outside of the httpclient's - # stack_context. We want to ensure that there is not stack_context - # between the user's callback and the IOLoop, so monkey-patch - # IOLoop.handle_callback_exception and disable the test harness's - # context with a NullContext. - # Note that this does not apply to secondary callbacks (header - # and streaming_callback), as errors there must be seen as errors - # by the http client so it can clean up the connection. - exc_info = [] - - def handle_callback_exception(callback): - exc_info.append(sys.exc_info()) - self.stop() - self.io_loop.handle_callback_exception = handle_callback_exception - with NullContext(): - with ignore_deprecation(): - self.http_client.fetch(self.get_url('/hello'), - lambda response: 1 / 0) - self.wait() - self.assertEqual(exc_info[0][0], ZeroDivisionError) - @gen_test def test_future_interface(self): response = yield self.http_client.fetch(self.get_url('/hello')) diff --git a/tornado/test/httpserver_test.py b/tornado/test/httpserver_test.py index acaf2a002..68ba51777 100644 --- a/tornado/test/httpserver_test.py +++ b/tornado/test/httpserver_test.py @@ -970,8 +970,9 @@ class MaxHeaderSizeTest(AsyncHTTPTestCase): except HTTPError as e: # 431 is "Request Header Fields Too Large", defined in RFC # 6585. However, many implementations just close the - # connection in this case, resulting in a 599. - self.assertIn(e.response.code, (431, 599)) + # connection in this case, resulting in a missing response. + if e.response is not None: + self.assertIn(e.response.code, (431, 599)) @skipOnTravis diff --git a/tornado/test/testing_test.py b/tornado/test/testing_test.py index 0f0ceba79..d9695c777 100644 --- a/tornado/test/testing_test.py +++ b/tornado/test/testing_test.py @@ -1,6 +1,7 @@ from __future__ import absolute_import, division, print_function from tornado import gen, ioloop +from tornado.httpserver import HTTPServer from tornado.log import app_log from tornado.simple_httpclient import SimpleAsyncHTTPClient, HTTPTimeoutError from tornado.test.util import unittest, skipBefore35, exec_test, ignore_deprecation @@ -84,12 +85,15 @@ class AsyncTestCaseTest(AsyncTestCase): class AsyncHTTPTestCaseTest(AsyncHTTPTestCase): - @classmethod - def setUpClass(cls): - super(AsyncHTTPTestCaseTest, cls).setUpClass() - # An unused port is bound so we can make requests upon it without - # impacting a real local web server. - cls.external_sock, cls.external_port = bind_unused_port() + def setUp(self): + super(AsyncHTTPTestCaseTest, self).setUp() + # Bind a second port. + sock, port = bind_unused_port() + app = Application() + server = HTTPServer(app, **self.get_httpserver_options()) + server.add_socket(sock) + self.second_port = port + self.second_server = server def get_app(self): return Application() @@ -99,28 +103,17 @@ class AsyncHTTPTestCaseTest(AsyncHTTPTestCase): response = self.fetch(path) self.assertEqual(response.request.url, self.get_url(path)) - @gen_test def test_fetch_full_http_url(self): - path = 'http://localhost:%d/path' % self.external_port + # Ensure that self.fetch() recognizes absolute urls and does + # not transform them into references to our main test server. + path = 'http://localhost:%d/path' % self.second_port - with contextlib.closing(SimpleAsyncHTTPClient(force_instance=True)) as client: - with self.assertRaises(HTTPTimeoutError) as cm: - yield client.fetch(path, request_timeout=0.1, raise_error=True) - self.assertEqual(cm.exception.response.request.url, path) + response = self.fetch(path) + self.assertEqual(response.request.url, path) - @gen_test - def test_fetch_full_https_url(self): - path = 'https://localhost:%d/path' % self.external_port - - with contextlib.closing(SimpleAsyncHTTPClient(force_instance=True)) as client: - with self.assertRaises(HTTPTimeoutError) as cm: - yield client.fetch(path, request_timeout=0.1, raise_error=True) - self.assertEqual(cm.exception.response.request.url, path) - - @classmethod - def tearDownClass(cls): - cls.external_sock.close() - super(AsyncHTTPTestCaseTest, cls).tearDownClass() + def tearDown(self): + self.second_server.stop() + super(AsyncHTTPTestCaseTest, self).tearDown() class AsyncTestCaseWrapperTest(unittest.TestCase): -- 2.47.2