From: Ben Darnell Date: Sat, 7 Apr 2018 23:31:39 +0000 (-0400) Subject: httpclient: Deprecate fetch callback X-Git-Tag: v5.1.0b1~30^2~6 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=71fa43a1cb29627441742da20fe042cc74f032f6;p=thirdparty%2Ftornado.git httpclient: Deprecate fetch callback --- diff --git a/tornado/auth.py b/tornado/auth.py index beee6453b..0069efcb4 100644 --- a/tornado/auth.py +++ b/tornado/auth.py @@ -202,9 +202,9 @@ class OpenIdMixin(object): url = self._OPENID_ENDPOINT if http_client is None: http_client = self.get_auth_http_client() - http_client.fetch(url, functools.partial( - self._on_authentication_verified, callback), - method="POST", body=urllib_parse.urlencode(args)) + fut = http_client.fetch(url, method="POST", body=urllib_parse.urlencode(args)) + fut.add_done_callback(functools.partial( + self._on_authentication_verified, callback)) def _openid_args(self, callback_uri, ax_attrs=[], oauth_scope=None): url = urlparse.urljoin(self.request.full_url(), callback_uri) @@ -254,11 +254,16 @@ class OpenIdMixin(object): }) return args - def _on_authentication_verified(self, future, response): - if response.error or b"is_valid:true" not in response.body: + def _on_authentication_verified(self, future, response_fut): + try: + response = response_fut.result() + except Exception as e: future.set_exception(AuthError( - "Invalid OpenID response: %s" % (response.error or - response.body))) + "Error response %s" % e)) + return + if b"is_valid:true" not in response.body: + future.set_exception(AuthError( + "Invalid OpenID response: %s" % response.body)) return # Make sure we got back at least an email from attribute exchange @@ -374,17 +379,17 @@ class OAuthMixin(object): if http_client is None: http_client = self.get_auth_http_client() if getattr(self, "_OAUTH_VERSION", "1.0a") == "1.0a": - http_client.fetch( + fut = http_client.fetch( self._oauth_request_token_url(callback_uri=callback_uri, - extra_params=extra_params), - functools.partial( - self._on_request_token, - self._OAUTH_AUTHORIZE_URL, - callback_uri, - callback)) + extra_params=extra_params)) + fut.add_done_callback(functools.partial( + self._on_request_token, + self._OAUTH_AUTHORIZE_URL, + callback_uri, + callback)) else: - http_client.fetch( - self._oauth_request_token_url(), + fut = http_client.fetch(self._oauth_request_token_url()) + fut.add_done_callback( functools.partial( self._on_request_token, self._OAUTH_AUTHORIZE_URL, callback_uri, @@ -427,8 +432,8 @@ class OAuthMixin(object): token["verifier"] = oauth_verifier if http_client is None: http_client = self.get_auth_http_client() - http_client.fetch(self._oauth_access_token_url(token), - functools.partial(self._on_access_token, callback)) + fut = http_client.fetch(self._oauth_access_token_url(token)) + fut.add_done_callback(functools.partial(self._on_access_token, callback)) def _oauth_request_token_url(self, callback_uri=None, extra_params=None): consumer_token = self._oauth_consumer_token() @@ -456,9 +461,11 @@ class OAuthMixin(object): return url + "?" + urllib_parse.urlencode(args) def _on_request_token(self, authorize_url, callback_uri, callback, - response): - if response.error: - raise Exception("Could not get request token: %s" % response.error) + response_fut): + try: + response = response_fut.result() + except Exception as e: + raise Exception("Could not get request token: %s" % e) request_token = _oauth_parse_response(response.body) data = (base64.b64encode(escape.utf8(request_token["key"])) + b"|" + base64.b64encode(escape.utf8(request_token["secret"]))) @@ -498,8 +505,10 @@ class OAuthMixin(object): args["oauth_signature"] = signature return url + "?" + urllib_parse.urlencode(args) - def _on_access_token(self, future, response): - if response.error: + def _on_access_token(self, future, response_fut): + try: + response = response_fut.result() + except Exception: future.set_exception(AuthError("Could not fetch access token")) return @@ -707,15 +716,16 @@ class OAuth2Mixin(object): callback = functools.partial(self._on_oauth2_request, callback) http = self.get_auth_http_client() if post_args is not None: - http.fetch(url, method="POST", body=urllib_parse.urlencode(post_args), - callback=callback) + fut = http.fetch(url, method="POST", body=urllib_parse.urlencode(post_args)) else: - http.fetch(url, callback=callback) - - def _on_oauth2_request(self, future, response): - if response.error: - future.set_exception(AuthError("Error response %s fetching %s" % - (response.error, response.request.url))) + fut = http.fetch(url) + fut.add_done_callback(callback) + + def _on_oauth2_request(self, future, response_fut): + try: + response = response_fut.result() + except Exception as e: + future.set_exception(AuthError("Error response %s" % e)) return future_set_result_unless_cancelled(future, escape.json_decode(response.body)) @@ -857,18 +867,19 @@ class TwitterMixin(OAuthMixin): if args: url += "?" + urllib_parse.urlencode(args) http = self.get_auth_http_client() - http_callback = functools.partial(self._on_twitter_request, callback) + http_callback = functools.partial(self._on_twitter_request, callback, url) if post_args is not None: - http.fetch(url, method="POST", body=urllib_parse.urlencode(post_args), - callback=http_callback) + fut = http.fetch(url, method="POST", body=urllib_parse.urlencode(post_args)) else: - http.fetch(url, callback=http_callback) + fut = http.fetch(url) + fut.add_done_callback(http_callback) - def _on_twitter_request(self, future, response): - if response.error: + def _on_twitter_request(self, future, url, response_fut): + try: + response = response_fut.result() + except Exception as e: future.set_exception(AuthError( - "Error response %s fetching %s" % (response.error, - response.request.url))) + "Error response %s fetching %s" % (e, url))) return future_set_result_unless_cancelled(future, escape.json_decode(response.body)) @@ -967,16 +978,18 @@ class GoogleOAuth2Mixin(OAuth2Mixin): "grant_type": "authorization_code", }) - http.fetch(self._OAUTH_ACCESS_TOKEN_URL, - functools.partial(self._on_access_token, callback), - method="POST", - headers={'Content-Type': 'application/x-www-form-urlencoded'}, - body=body) + fut = http.fetch(self._OAUTH_ACCESS_TOKEN_URL, + method="POST", + headers={'Content-Type': 'application/x-www-form-urlencoded'}, + body=body) + fut.add_done_callback(functools.partial(self._on_access_token, callback)) - def _on_access_token(self, future, response): + def _on_access_token(self, future, response_fut): """Callback function for the exchange to the access token.""" - if response.error: - future.set_exception(AuthError('Google auth error: %s' % str(response))) + try: + response = response_fut.result() + except Exception as e: + future.set_exception(AuthError('Google auth error: %s' % str(e))) return args = escape.json_decode(response.body) @@ -1053,15 +1066,17 @@ class FacebookGraphMixin(OAuth2Mixin): if extra_fields: fields.update(extra_fields) - http.fetch(self._oauth_request_token_url(**args), - functools.partial(self._on_access_token, redirect_uri, client_id, - client_secret, callback, fields)) + fut = http.fetch(self._oauth_request_token_url(**args)) + fut.add_done_callback(functools.partial(self._on_access_token, redirect_uri, client_id, + client_secret, callback, fields)) @gen.coroutine def _on_access_token(self, redirect_uri, client_id, client_secret, - future, fields, response): - if response.error: - future.set_exception(AuthError('Facebook auth error: %s' % str(response))) + future, fields, response_fut): + try: + response = response_fut.result() + except Exception as e: + future.set_exception(AuthError('Facebook auth error: %s' % str(e))) return args = escape.json_decode(response.body) diff --git a/tornado/httpclient.py b/tornado/httpclient.py index 9c438d15c..d41a02730 100644 --- a/tornado/httpclient.py +++ b/tornado/httpclient.py @@ -42,6 +42,7 @@ from __future__ import absolute_import, division, print_function import functools import time +import warnings import weakref from tornado.concurrent import Future, future_set_result_unless_cancelled @@ -238,6 +239,12 @@ class AsyncHTTPClient(Configurable): In the callback interface, `HTTPError` is not automatically raised. Instead, you must check the response's ``error`` attribute or call its `~HTTPResponse.rethrow` method. + + .. deprecated:: 5.1 + + The ``callback`` argument is deprecated and will be removed + in 6.0. Use the returned `.Future` instead. + """ if self._closed: raise RuntimeError("fetch() called on closed AsyncHTTPClient") @@ -253,6 +260,8 @@ class AsyncHTTPClient(Configurable): 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): diff --git a/tornado/simple_httpclient.py b/tornado/simple_httpclient.py index 1fc3e7075..c9d159e43 100644 --- a/tornado/simple_httpclient.py +++ b/tornado/simple_httpclient.py @@ -498,7 +498,8 @@ class _HTTPConnection(httputil.HTTPMessageDelegate): final_callback = self.final_callback self.final_callback = None self._release() - self.client.fetch(new_request, final_callback) + fut = self.client.fetch(new_request, raise_error=False) + fut.add_done_callback(lambda f: final_callback(f.result())) self._on_end_request() return if self.request.streaming_callback: diff --git a/tornado/test/httpclient_test.py b/tornado/test/httpclient_test.py index a76d9eaee..e135526f5 100644 --- a/tornado/test/httpclient_test.py +++ b/tornado/test/httpclient_test.py @@ -20,7 +20,7 @@ from tornado.log import gen_log from tornado import netutil from tornado.stack_context import ExceptionStackContext, NullContext from tornado.testing import AsyncHTTPTestCase, bind_unused_port, gen_test, ExpectLog -from tornado.test.util import unittest, skipOnTravis +from tornado.test.util import unittest, skipOnTravis, ignore_deprecation from tornado.web import Application, RequestHandler, url from tornado.httputil import format_timestamp, HTTPHeaders @@ -212,8 +212,7 @@ Transfer-Encoding: chunked stream.read_until(b"\r\n\r\n", functools.partial(write_response, stream)) netutil.add_accept_handler(sock, accept_callback) - self.http_client.fetch("http://127.0.0.1:%d/" % port, self.stop) - resp = self.wait() + resp = self.fetch("http://127.0.0.1:%d/" % port) resp.rethrow() self.assertEqual(resp.body, b"12") self.io_loop.remove_handler(sock.fileno()) @@ -272,8 +271,7 @@ Transfer-Encoding: chunked def test_credentials_in_url(self): url = self.get_url("/auth").replace("http://", "http://me:secret@") - self.http_client.fetch(url, self.stop) - response = self.wait() + response = self.fetch(url) self.assertEqual(b"Basic " + base64.b64encode(b"me:secret"), response.body) @@ -353,14 +351,14 @@ Transfer-Encoding: chunked self.assertEqual(len(exc_info), 1) self.assertIs(exc_info[0][0], ZeroDivisionError) + @gen_test def test_configure_defaults(self): defaults = dict(user_agent='TestDefaultUserAgent', allow_ipv6=False) # Construct a new instance of the configured client class client = self.http_client.__class__(force_instance=True, defaults=defaults) try: - client.fetch(self.get_url('/user_agent'), callback=self.stop) - response = self.wait() + response = yield client.fetch(self.get_url('/user_agent')) self.assertEqual(response.body, b'TestDefaultUserAgent') finally: client.close() @@ -400,8 +398,7 @@ X-XSS-Protection: 1; stream.read_until(b"\r\n\r\n", functools.partial(write_response, stream)) netutil.add_accept_handler(sock, accept_callback) - self.http_client.fetch("http://127.0.0.1:%d/" % port, self.stop) - resp = self.wait() + resp = self.fetch("http://127.0.0.1:%d/" % port) resp.rethrow() self.assertEqual(resp.headers['X-XSS-Protection'], "1; mode=block") self.io_loop.remove_handler(sock.fileno()) @@ -431,8 +428,9 @@ X-XSS-Protection: 1; self.stop() self.io_loop.handle_callback_exception = handle_callback_exception with NullContext(): - self.http_client.fetch(self.get_url('/hello'), - lambda response: 1 / 0) + with ignore_deprecation(): + self.http_client.fetch(self.get_url('/hello'), + lambda response: 1 / 0) self.wait() self.assertEqual(exc_info[0][0], ZeroDivisionError) diff --git a/tornado/test/httpserver_test.py b/tornado/test/httpserver_test.py index bfac9ab52..341dfbe8f 100644 --- a/tornado/test/httpserver_test.py +++ b/tornado/test/httpserver_test.py @@ -109,21 +109,17 @@ class SSLTestMixin(object): # misbehaving. with ExpectLog(gen_log, '(SSL Error|uncaught exception)'): with ExpectLog(gen_log, 'Uncaught exception', required=False): - self.http_client.fetch( + response = self.fetch( self.get_url("/").replace('https:', 'http:'), - self.stop, request_timeout=3600, connect_timeout=3600) - response = self.wait() self.assertEqual(response.code, 599) def test_error_logging(self): # No stack traces are logged for SSL errors. with ExpectLog(gen_log, 'SSL Error') as expect_log: - self.http_client.fetch( - self.get_url("/").replace("https:", "http:"), - self.stop) - response = self.wait() + response = self.fetch( + self.get_url("/").replace("https:", "http:")) self.assertEqual(response.code, 599) self.assertFalse(expect_log.logged_stack) diff --git a/tornado/test/simple_httpclient_test.py b/tornado/test/simple_httpclient_test.py index 973353e03..8856c2369 100644 --- a/tornado/test/simple_httpclient_test.py +++ b/tornado/test/simple_httpclient_test.py @@ -22,7 +22,7 @@ from tornado.netutil import Resolver, bind_sockets 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 +from tornado.testing import AsyncHTTPTestCase, AsyncHTTPSTestCase, AsyncTestCase, ExpectLog, gen_test from tornado.test.util import skipOnTravis, skipIfNoIPv6, refusing_port, skipBefore35, exec_test from tornado.web import RequestHandler, Application, asynchronous, url, stream_request_body @@ -167,8 +167,8 @@ class SimpleHTTPClientTestMixin(object): # Send 4 requests. Two can be sent immediately, while the others # will be queued for i in range(4): - client.fetch(self.get_url("/trigger"), - lambda response, i=i: (seen.append(i), self.stop())) + client.fetch(self.get_url("/trigger")).add_done_callback( + lambda fut, i=i: (seen.append(i), self.stop())) self.wait(condition=lambda: len(self.triggers) == 2) self.assertEqual(len(client.queue), 2) @@ -187,12 +187,12 @@ class SimpleHTTPClientTestMixin(object): self.assertEqual(set(seen), set([0, 1, 2, 3])) self.assertEqual(len(self.triggers), 0) + @gen_test def test_redirect_connection_limit(self): # following redirects should not consume additional connections with closing(self.create_client(max_clients=1)) as client: - client.fetch(self.get_url('/countdown/3'), self.stop, - max_redirects=3) - response = self.wait() + response = yield client.fetch(self.get_url('/countdown/3'), + max_redirects=3) response.rethrow() def test_gzip(self): @@ -236,6 +236,7 @@ class SimpleHTTPClientTestMixin(object): self.assertEqual("POST", response.request.method) @skipOnTravis + @gen_test def test_connect_timeout(self): timeout = 0.1 timeout_min, timeout_max = 0.099, 1.0 @@ -245,9 +246,9 @@ class SimpleHTTPClientTestMixin(object): return Future() # never completes with closing(self.create_client(resolver=TimeoutResolver())) as client: - client.fetch(self.get_url('/hello'), self.stop, - connect_timeout=timeout) - response = self.wait() + 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) @@ -277,12 +278,10 @@ class SimpleHTTPClientTestMixin(object): url = '%s://[::1]:%d/hello' % (self.get_protocol(), port) # ipv6 is currently enabled by default but can be disabled - self.http_client.fetch(url, self.stop, allow_ipv6=False) - response = self.wait() + response = self.fetch(url, allow_ipv6=False) self.assertEqual(response.code, 599) - self.http_client.fetch(url, self.stop) - response = self.wait() + response = self.fetch(url) self.assertEqual(response.body, b"Hello world!") def xtest_multiple_content_length_accepted(self): @@ -324,16 +323,14 @@ class SimpleHTTPClientTestMixin(object): self.assertTrue(host_re.match(response.body)) url = self.get_url("/host_echo").replace("http://", "http://me:secret@") - self.http_client.fetch(url, self.stop) - response = self.wait() + response = self.fetch(url) self.assertTrue(host_re.match(response.body), response.body) def test_connection_refused(self): cleanup_func, port = refusing_port() self.addCleanup(cleanup_func) with ExpectLog(gen_log, ".*", required=False): - self.http_client.fetch("http://127.0.0.1:%d/" % port, self.stop) - response = self.wait() + response = self.fetch("http://127.0.0.1:%d/" % port) self.assertEqual(599, response.code) if sys.platform != 'cygwin': @@ -350,19 +347,22 @@ class SimpleHTTPClientTestMixin(object): def test_queue_timeout(self): with closing(self.create_client(max_clients=1)) as client: - client.fetch(self.get_url('/trigger'), self.stop, - request_timeout=10) # Wait for the trigger request to block, not complete. + fut1 = client.fetch(self.get_url('/trigger'), + request_timeout=10, raise_error=False) self.wait() - client.fetch(self.get_url('/hello'), self.stop, - connect_timeout=0.1) - response = 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") @@ -640,15 +640,13 @@ class HostnameMappingTestCase(AsyncHTTPTestCase): return Application([url("/hello", HelloWorldHandler), ]) def test_hostname_mapping(self): - self.http_client.fetch( - 'http://www.example.com:%d/hello' % self.get_http_port(), self.stop) - response = self.wait() + response = self.fetch( + 'http://www.example.com:%d/hello' % self.get_http_port()) response.rethrow() self.assertEqual(response.body, b'Hello world!') def test_port_mapping(self): - self.http_client.fetch('http://foo.example.com:8000/hello', self.stop) - response = self.wait() + response = self.fetch('http://foo.example.com:8000/hello') response.rethrow() self.assertEqual(response.body, b'Hello world!') diff --git a/tornado/test/stack_context_test.py b/tornado/test/stack_context_test.py index 4afe2a86f..243ea47f3 100644 --- a/tornado/test/stack_context_test.py +++ b/tornado/test/stack_context_test.py @@ -6,7 +6,7 @@ from tornado.log import app_log from tornado.stack_context import (StackContext, wrap, NullContext, StackContextInconsistentError, ExceptionStackContext, run_with_stack_context, _state) from tornado.testing import AsyncHTTPTestCase, AsyncTestCase, ExpectLog, gen_test -from tornado.test.util import unittest +from tornado.test.util import unittest, ignore_deprecation from tornado.web import asynchronous, Application, RequestHandler import contextlib import functools @@ -48,8 +48,9 @@ class HTTPStackContextTest(AsyncHTTPTestCase): def test_stack_context(self): with ExpectLog(app_log, "Uncaught exception GET /"): - self.http_client.fetch(self.get_url('/'), self.handle_response) - self.wait() + with ignore_deprecation(): + self.http_client.fetch(self.get_url('/'), self.handle_response) + self.wait() self.assertEqual(self.response.code, 500) self.assertTrue(b'got expected exception' in self.response.body) diff --git a/tornado/test/web_test.py b/tornado/test/web_test.py index 6673a4f26..d757d78e7 100644 --- a/tornado/test/web_test.py +++ b/tornado/test/web_test.py @@ -350,16 +350,14 @@ class AuthRedirectTest(WebTestCase): dict(login_url='http://example.com/login'))] def test_relative_auth_redirect(self): - self.http_client.fetch(self.get_url('/relative'), self.stop, - follow_redirects=False) - response = self.wait() + response = self.fetch(self.get_url('/relative'), + follow_redirects=False) self.assertEqual(response.code, 302) self.assertEqual(response.headers['Location'], '/login?next=%2Frelative') def test_absolute_auth_redirect(self): - self.http_client.fetch(self.get_url('/absolute'), self.stop, - follow_redirects=False) - response = self.wait() + response = self.fetch(self.get_url('/absolute'), + follow_redirects=False) self.assertEqual(response.code, 302) self.assertTrue(re.match( 'http://example.com/login\?next=http%3A%2F%2F127.0.0.1%3A[0-9]+%2Fabsolute', diff --git a/tornado/testing.py b/tornado/testing.py index 19bc5a946..c3450272a 100644 --- a/tornado/testing.py +++ b/tornado/testing.py @@ -397,14 +397,19 @@ class AsyncHTTPTestCase(AsyncTestCase): If the path begins with http:// or https://, it will be treated as a full URL and will be fetched as-is. + Unlike awaiting `.AsyncHTTPClient.fetch` in a coroutine, no + exception is raised for non-200 response codes (as if the + ``raise_error=True`` option were used). + .. versionchanged:: 5.0 Added support for absolute URLs. + """ if path.lower().startswith(('http://', 'https://')): - self.http_client.fetch(path, self.stop, **kwargs) + url = path else: - self.http_client.fetch(self.get_url(path), self.stop, **kwargs) - return self.wait() + url = self.get_url(path) + return self.io_loop.run_sync(lambda: self.http_client.fetch(url, raise_error=False, **kwargs)) def get_httpserver_options(self): """May be overridden by subclasses to return additional