From 3e879ec97923e9d29a4a277f942893ae7ed5f090 Mon Sep 17 00:00:00 2001 From: Hiroshi Ogawa Date: Sat, 30 Oct 2021 19:33:52 +0900 Subject: [PATCH] fix(5432): add query parameters in tracing url (#6178) * fix(5432): add query parameters in tracing url * docs: add CHANGES/5432.bugfix * test: support python 3.7 in `test_request_tracing_url_params` --- CHANGES/5432.bugfix | 1 + aiohttp/client.py | 12 ++-- tests/test_client_session.py | 112 ++++++++++++++++++++++++++++++++++- 3 files changed, 120 insertions(+), 5 deletions(-) create mode 100644 CHANGES/5432.bugfix diff --git a/CHANGES/5432.bugfix b/CHANGES/5432.bugfix new file mode 100644 index 00000000000..b5360aa75e8 --- /dev/null +++ b/CHANGES/5432.bugfix @@ -0,0 +1 @@ +Include query parameters from `params` keyword argument in tracing `URL`. diff --git a/aiohttp/client.py b/aiohttp/client.py index a91c007e104..685e8d9c229 100644 --- a/aiohttp/client.py +++ b/aiohttp/client.py @@ -403,7 +403,7 @@ async def _request( ] for trace in traces: - await trace.send_request_start(method, url, headers) + await trace.send_request_start(method, url.update_query(params), headers) timer = tm.timer() try: @@ -519,7 +519,7 @@ async def _request( for trace in traces: await trace.send_request_redirect( - method, url, headers, resp + method, url.update_query(params), headers, resp ) redirects += 1 @@ -607,7 +607,9 @@ async def _request( resp._history = tuple(history) for trace in traces: - await trace.send_request_end(method, url, headers, resp) + await trace.send_request_end( + method, url.update_query(params), headers, resp + ) return resp except BaseException as e: @@ -618,7 +620,9 @@ async def _request( handle = None for trace in traces: - await trace.send_request_exception(method, url, headers, e) + await trace.send_request_exception( + method, url.update_query(params), headers, e + ) raise def ws_connect( diff --git a/tests/test_client_session.py b/tests/test_client_session.py index effe011dbb6..63deef311a3 100644 --- a/tests/test_client_session.py +++ b/tests/test_client_session.py @@ -6,7 +6,7 @@ import sys from http.cookies import SimpleCookie from io import BytesIO -from typing import Any +from typing import Any, List from unittest import mock import pytest @@ -594,6 +594,116 @@ async def on_request_headers_sent(session, context, params): assert gathered_req_headers["Custom-Header"] == "Custom value" +async def test_request_tracing_url_params(loop: Any, aiohttp_client: Any) -> None: + async def root_handler(request): + return web.Response() + + async def redirect_handler(request): + raise web.HTTPFound("/") + + app = web.Application() + app.router.add_get("/", root_handler) + app.router.add_get("/redirect", redirect_handler) + + mocks = [mock.Mock(side_effect=make_mocked_coro(mock.Mock())) for _ in range(7)] + ( + on_request_start, + on_request_redirect, + on_request_end, + on_request_exception, + on_request_chunk_sent, + on_response_chunk_received, + on_request_headers_sent, + ) = mocks + + trace_config = aiohttp.TraceConfig( + trace_config_ctx_factory=mock.Mock(return_value=mock.Mock()) + ) + trace_config.on_request_start.append(on_request_start) + trace_config.on_request_redirect.append(on_request_redirect) + trace_config.on_request_end.append(on_request_end) + trace_config.on_request_exception.append(on_request_exception) + trace_config.on_request_chunk_sent.append(on_request_chunk_sent) + trace_config.on_response_chunk_received.append(on_response_chunk_received) + trace_config.on_request_headers_sent.append(on_request_headers_sent) + + session = await aiohttp_client(app, trace_configs=[trace_config]) + + def reset_mocks() -> None: + for m in mocks: + m.reset_mock() + + def to_trace_urls(mock_func: mock.Mock) -> List[URL]: + return [call_args[0][-1].url for call_args in mock_func.call_args_list] + + def to_url(path: str) -> URL: + return session.make_url(path) + + # Standard + for req in [ + lambda: session.get("/?x=0"), + lambda: session.get("/", params=dict(x=0)), + ]: + reset_mocks() + async with req() as resp: + await resp.text() + assert to_trace_urls(on_request_start) == [to_url("/?x=0")] + assert to_trace_urls(on_request_redirect) == [] + assert to_trace_urls(on_request_end) == [to_url("/?x=0")] + assert to_trace_urls(on_request_exception) == [] + assert to_trace_urls(on_request_chunk_sent) == [to_url("/?x=0")] + assert to_trace_urls(on_response_chunk_received) == [to_url("/?x=0")] + assert to_trace_urls(on_request_headers_sent) == [to_url("/?x=0")] + + # Redirect + for req in [ + lambda: session.get("/redirect?x=0"), + lambda: session.get("/redirect", params=dict(x=0)), + ]: + reset_mocks() + async with req() as resp: + await resp.text() + assert to_trace_urls(on_request_start) == [to_url("/redirect?x=0")] + assert to_trace_urls(on_request_redirect) == [to_url("/redirect?x=0")] + assert to_trace_urls(on_request_end) == [to_url("/")] + assert to_trace_urls(on_request_exception) == [] + assert to_trace_urls(on_request_chunk_sent) == [ + to_url("/redirect?x=0"), + to_url("/"), + ] + assert to_trace_urls(on_response_chunk_received) == [to_url("/")] + assert to_trace_urls(on_request_headers_sent) == [ + to_url("/redirect?x=0"), + to_url("/"), + ] + + # Exception + with mock.patch("aiohttp.client.TCPConnector.connect") as connect_patched: + error = Exception() + if sys.version_info >= (3, 8, 1): + connect_patched.side_effect = error + else: + loop = asyncio.get_event_loop() + f = loop.create_future() + f.set_exception(error) + connect_patched.return_value = f + + for req in [ + lambda: session.get("/?x=0"), + lambda: session.get("/", params=dict(x=0)), + ]: + reset_mocks() + with contextlib.suppress(Exception): + await req() + assert to_trace_urls(on_request_start) == [to_url("/?x=0")] + assert to_trace_urls(on_request_redirect) == [] + assert to_trace_urls(on_request_end) == [] + assert to_trace_urls(on_request_exception) == [to_url("?x=0")] + assert to_trace_urls(on_request_chunk_sent) == [] + assert to_trace_urls(on_response_chunk_received) == [] + assert to_trace_urls(on_request_headers_sent) == [] + + async def test_request_tracing_exception() -> None: on_request_end = mock.Mock(side_effect=make_mocked_coro(mock.Mock())) on_request_exception = mock.Mock(side_effect=make_mocked_coro(mock.Mock()))