From a66873bcc885c0b4186080f66ab83fc1481956b8 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Fri, 30 Jun 2023 14:07:51 +0200 Subject: [PATCH 01/39] Capture GraphQL client errors --- sentry_sdk/integrations/aiohttp.py | 126 +++++++++++++++-- sentry_sdk/integrations/httpx.py | 94 ++++++++++++- sentry_sdk/integrations/stdlib.py | 117 ++++++++++++++-- sentry_sdk/scrubber.py | 9 ++ sentry_sdk/utils.py | 29 ++++ tests/integrations/aiohttp/test_aiohttp.py | 113 ++++++++++++++- tests/integrations/httpx/test_httpx.py | 139 +++++++++++++++++-- tests/integrations/requests/test_requests.py | 90 ++++++++++++ 8 files changed, 685 insertions(+), 32 deletions(-) diff --git a/sentry_sdk/integrations/aiohttp.py b/sentry_sdk/integrations/aiohttp.py index d2d431aefd..0867a59936 100644 --- a/sentry_sdk/integrations/aiohttp.py +++ b/sentry_sdk/integrations/aiohttp.py @@ -1,5 +1,7 @@ +import json import sys import weakref +from urllib.parse import parse_qsl from sentry_sdk.api import continue_trace from sentry_sdk._compat import reraise @@ -29,6 +31,9 @@ CONTEXTVARS_ERROR_MESSAGE, SENSITIVE_DATA_SUBSTITUTE, AnnotatedValue, + SentryGraphQLClientError, + _get_graphql_operation_name, + _get_graphql_operation_type, ) try: @@ -36,7 +41,7 @@ from aiohttp import __version__ as AIOHTTP_VERSION from aiohttp import ClientSession, TraceConfig - from aiohttp.web import Application, HTTPException, UrlDispatcher + from aiohttp.web import Application, HTTPException, UrlDispatcher, Response except ImportError: raise DidNotEnable("AIOHTTP not installed") @@ -111,7 +116,7 @@ async def sentry_app_handle(self, request, *args, **kwargs): # create a task to wrap each request. with hub.configure_scope() as scope: scope.clear_breadcrumbs() - scope.add_event_processor(_make_request_processor(weak_request)) + scope.add_event_processor(_make_server_processor(weak_request)) transaction = continue_trace( request.headers, @@ -139,6 +144,7 @@ async def sentry_app_handle(self, request, *args, **kwargs): reraise(*_capture_exception(hub)) transaction.set_http_status(response.status) + return response Application._handle = sentry_app_handle @@ -234,27 +240,74 @@ async def on_request_start(session, trace_config_ctx, params): trace_config_ctx.span = span + if params.url.path == "/graphql": + trace_config_ctx.request_headers = params.headers + + async def on_request_chunk_sent(session, trace_config_ctx, params): + if params.url.path == "/graphql" and params.method == "POST": + with capture_internal_exceptions(): + trace_config_ctx.request_body = json.loads(params.chunk) + async def on_request_end(session, trace_config_ctx, params): # type: (ClientSession, SimpleNamespace, TraceRequestEndParams) -> None - if trace_config_ctx.span is None: - return + response = params.response + + if trace_config_ctx.span is not None: + span = trace_config_ctx.span + span.set_http_status(int(response.status)) + span.set_data("reason", response.reason) + + hub = Hub.current + if ( + response.url.path == "/graphql" + and response.method in ("GET", "POST") + and response.status == 200 + ): + with hub.configure_scope() as scope: + with capture_internal_exceptions(): + response_content = await response.json() + scope.add_event_processor( + _make_client_processor( + trace_config_ctx=trace_config_ctx, + response=response, + response_content=response_content, + ) + ) + + if ( + response_content + and isinstance(response_content, dict) + and response_content.get("errors") + ): + try: + raise SentryGraphQLClientError + except SentryGraphQLClientError as ex: + event, hint = event_from_exception( + ex, + client_options=hub.client.options + if hub.client + else None, + mechanism={ + "type": AioHttpIntegration.identifier, + "handled": False, + }, + ) + hub.capture_event(event, hint=hint) - span = trace_config_ctx.span - span.set_http_status(int(params.response.status)) - span.set_data("reason", params.response.reason) span.finish() trace_config = TraceConfig() trace_config.on_request_start.append(on_request_start) + trace_config.on_request_chunk_sent.append(on_request_chunk_sent) trace_config.on_request_end.append(on_request_end) return trace_config -def _make_request_processor(weak_request): +def _make_server_processor(weak_request): # type: (Callable[[], Request]) -> EventProcessor - def aiohttp_processor( + def aiohttp_server_processor( event, # type: Dict[str, Any] hint, # type: Dict[str, Tuple[type, BaseException, Any]] ): @@ -286,7 +339,60 @@ def aiohttp_processor( return event - return aiohttp_processor + return aiohttp_server_processor + + +def _make_client_processor(trace_config_ctx, response, response_content): + # type: (SimpleNamespace, Response, Optional[dict]) -> EventProcessor + def aiohttp_client_processor( + event, # type: Dict[str, Any] + hint, # type: Dict[str, Tuple[type, BaseException, Any]] + ): + # type: (...) -> Dict[str, Any] + with capture_internal_exceptions(): + request_info = event.setdefault("request", {}) + + parsed_url = parse_url(str(response.url), sanitize=False) + request_info["url"] = parsed_url.url + request_info["query_string"] = parsed_url.query + request_info["method"] = response.method + + if getattr(trace_config_ctx, "request_headers", None): + request_info["headers"] = _filter_headers( + dict(trace_config_ctx.request_headers) + ) + if getattr(trace_config_ctx, "request_body", None): + request_info["data"] = trace_config_ctx.request_body + + if response.url.path == "/graphql": + request_info["api_target"] = "graphql" + + query = request_info.get("data") + if response.method == "GET": + query = dict(parse_qsl(parsed_url.query)) + + if query: + operation_name = _get_graphql_operation_name(query) + operation_type = _get_graphql_operation_type(query) + event["fingerprint"] = [ + operation_name, + operation_type, + response.status, + ] + event["exception"]["values"][0][ + "value" + ] = "GraphQL request failed, name: {}, type: {}".format( + operation_name, operation_type + ) + + if response_content: + contexts = event.setdefault("contexts", {}) + response_context = contexts.setdefault("response", {}) + response_context["data"] = response_content + + return event + + return aiohttp_client_processor def _capture_exception(hub): diff --git a/sentry_sdk/integrations/httpx.py b/sentry_sdk/integrations/httpx.py index 04db5047b4..6448659eaf 100644 --- a/sentry_sdk/integrations/httpx.py +++ b/sentry_sdk/integrations/httpx.py @@ -1,3 +1,6 @@ +import json +from urllib.parse import parse_qsl + from sentry_sdk import Hub from sentry_sdk.consts import OP, SPANDATA from sentry_sdk.integrations import Integration, DidNotEnable @@ -5,15 +8,20 @@ from sentry_sdk.tracing_utils import should_propagate_trace from sentry_sdk.utils import ( SENSITIVE_DATA_SUBSTITUTE, + SentryGraphQLClientError, capture_internal_exceptions, + event_from_exception, logger, parse_url, + _get_graphql_operation_name, + _get_graphql_operation_type, ) - from sentry_sdk._types import TYPE_CHECKING +from sentry_sdk.integrations._wsgi_common import _filter_headers if TYPE_CHECKING: - from typing import Any + from typing import Any, Dict, Tuple + from sentry_sdk._types import EventProcessor try: @@ -86,6 +94,8 @@ def send(self, request, **kwargs): span.set_http_status(rv.status_code) span.set_data("reason", rv.reason_phrase) + _capture_graphql_errors(hub, request, rv) + return rv Client.send = send @@ -139,6 +149,86 @@ async def send(self, request, **kwargs): span.set_http_status(rv.status_code) span.set_data("reason", rv.reason_phrase) + _capture_graphql_errors(hub, request, rv) + return rv AsyncClient.send = send + + +def _make_request_processor(request, response): + # type: (Request, Response) -> EventProcessor + def httpx_processor( + event, # type: Dict[str, Any] + hint, # type: Dict[str, Tuple[type, BaseException, Any]] + ): + # type: (...) -> Dict[str, Any] + with capture_internal_exceptions(): + request_info = event.setdefault("request", {}) + + parsed_url = parse_url(str(request.url), sanitize=False) + request_info["url"] = parsed_url.url + request_info["method"] = request.method + request_info["headers"] = _filter_headers(dict(request.headers)) + request_info["query_string"] = parsed_url.query + + if request.content: + try: + request_info["data"] = json.loads(request.content) + except json.JSONDecodeError: + pass + + if response: + response_content = response.json() + contexts = event.setdefault("contexts", {}) + response_context = contexts.setdefault("response", {}) + response_context["data"] = response_content + + if request.url.path == "/graphql": + request_info["api_target"] = "graphql" + + query = request_info.get("data") + if request.method == "GET": + query = dict(parse_qsl(request.url.query.decode())) + + if query: + operation_name = _get_graphql_operation_name(query) + operation_type = _get_graphql_operation_type(query) + event["fingerprint"] = [operation_name, operation_type, 200] + event["exception"]["values"][0][ + "value" + ] = "GraphQL request failed, name: {}, type: {}".format( + operation_name, operation_type + ) + + return event + + return httpx_processor + + +def _capture_graphql_errors(hub, request, response): + if ( + request.url.path == "/graphql" + and request.method in ("GET", "POST") + and response.status_code == 200 + ): + with hub.configure_scope() as scope: + scope.add_event_processor(_make_request_processor(request, response)) + + with capture_internal_exceptions(): + response_content = response.json() + if isinstance(response_content, dict) and response_content.get( + "errors" + ): + try: + raise SentryGraphQLClientError + except SentryGraphQLClientError as ex: + event, hint = event_from_exception( + ex, + client_options=hub.client.options if hub.client else None, + mechanism={ + "type": HttpxIntegration.identifier, + "handled": False, + }, + ) + hub.capture_event(event, hint=hint) diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index be02779d88..8dca57b34d 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/sentry_sdk/integrations/stdlib.py @@ -1,31 +1,37 @@ +import json import os import subprocess import sys import platform -from sentry_sdk.consts import OP, SPANDATA +from urllib.parse import parse_qs, urlparse +from sentry_sdk.consts import OP, SPANDATA from sentry_sdk.hub import Hub from sentry_sdk.integrations import Integration from sentry_sdk.scope import add_global_event_processor from sentry_sdk.tracing_utils import EnvironHeaders, should_propagate_trace from sentry_sdk.utils import ( SENSITIVE_DATA_SUBSTITUTE, + SentryGraphQLClientError, capture_internal_exceptions, + event_from_exception, logger, safe_repr, parse_url, + _get_graphql_operation_name, + _get_graphql_operation_type, ) - from sentry_sdk._types import TYPE_CHECKING if TYPE_CHECKING: from typing import Any from typing import Callable from typing import Dict - from typing import Optional from typing import List + from typing import Optional + from typing import Tuple - from sentry_sdk._types import Event, Hint + from sentry_sdk._types import Event, EventProcessor, Hint try: @@ -64,6 +70,7 @@ def add_python_runtime_context(event, hint): def _install_httplib(): # type: () -> None real_putrequest = HTTPConnection.putrequest + real_send = HTTPConnection.send real_getresponse = HTTPConnection.getresponse def putrequest(self, method, url, *args, **kwargs): @@ -84,10 +91,12 @@ def putrequest(self, method, url, *args, **kwargs): port != default_port and ":%s" % port or "", url, ) + self._sentrysdk_url = real_url parsed_url = None with capture_internal_exceptions(): parsed_url = parse_url(real_url, sanitize=False) + self._sentrysdk_is_graphql_request = parsed_url.url.endswith("/graphql") span = hub.start_span( op=OP.HTTP_CLIENT, @@ -113,28 +122,116 @@ def putrequest(self, method, url, *args, **kwargs): self.putheader(key, value) self._sentrysdk_span = span + self._sentrysdk_method = method + + return rv + + def send(self, data, *args, **kwargs): + # type: (HTTPConnection, Any, *Any, *Any) -> Any + if getattr(self, "_sentrysdk_is_graphql_request", False): + self._sentry_request_body = data + rv = real_send(self, data, *args, **kwargs) return rv def getresponse(self, *args, **kwargs): # type: (HTTPConnection, *Any, **Any) -> Any span = getattr(self, "_sentrysdk_span", None) - if span is None: - return real_getresponse(self, *args, **kwargs) - rv = real_getresponse(self, *args, **kwargs) - span.set_http_status(int(rv.status)) - span.set_data("reason", rv.reason) - span.finish() + if span is not None: + span.set_http_status(int(rv.status)) + span.set_data("reason", rv.reason) + span.finish() + + url = getattr(self, "_sentrysdk_url", None) + if url is None: + return rv + + response_body = None + if getattr(self, "_sentrysdk_is_graphql_request", False): + with capture_internal_exceptions(): + try: + response_body = json.loads(rv.read()) + except (json.JSONDecodeError, UnicodeDecodeError): + return rv + + if isinstance(response_body, dict) and response_body.get("errors"): + method = getattr(self, "_sentrysdk_method", None) + request_body = getattr(self, "_sentry_request_body", None) + hub = Hub.current + with hub.configure_scope() as scope: + scope.add_event_processor( + _make_request_processor( + url, method, rv.status, request_body, response_body + ) + ) + try: + raise SentryGraphQLClientError + except SentryGraphQLClientError as ex: + event, hint = event_from_exception( + ex, + client_options=hub.client.options if hub.client else None, + mechanism={ + "type": StdlibIntegration.identifier, + "handled": False, + }, + ) + + hub.capture_event(event, hint=hint) return rv HTTPConnection.putrequest = putrequest + HTTPConnection.send = send HTTPConnection.getresponse = getresponse +def _make_request_processor(url, method, status, request_body, response_body): + # type: (str, str, int, Any, Any) -> EventProcessor + def stdlib_processor( + event, # type: Dict[str, Any] + hint, # type: Dict[str, Tuple[type, BaseException, Any]] + ): + with capture_internal_exceptions(): + parsed_url = urlparse(url) + + request_info = event.setdefault("request", {}) + request_info["url"] = url + request_info["method"] = method + request_info["query_string"] = parsed_url.query + try: + request_info["data"] = json.loads(request_body) + except json.JSONDecodeError: + pass + + if response_body: + contexts = event.setdefault("contexts", {}) + response_context = contexts.setdefault("response", {}) + response_context["data"] = response_body + + if parsed_url.path == "/graphql": + request_info["api_target"] = "graphql" + query = request_info.get("data") + if method == "GET": + query = parse_qs(parsed_url.query) + + if query: + operation_name = _get_graphql_operation_name(query) + operation_type = _get_graphql_operation_type(query) + event["fingerprint"] = [operation_name, operation_type, status] + event["exception"]["values"][0][ + "value" + ] = "GraphQL request failed, name: {}, type: {}".format( + operation_name, operation_type + ) + + return event + + return stdlib_processor + + def _init_argument(args, kwargs, name, position, setdefault_callback=None): # type: (List[Any], Dict[Any, Any], str, int, Optional[Callable[[Any], Any]]) -> Any """ diff --git a/sentry_sdk/scrubber.py b/sentry_sdk/scrubber.py index 838ef08b4b..f96000fc95 100644 --- a/sentry_sdk/scrubber.py +++ b/sentry_sdk/scrubber.py @@ -84,6 +84,14 @@ def scrub_request(self, event): if "data" in event["request"]: self.scrub_dict(event["request"]["data"]) + def scrub_response(self, event): + # type: (Event) -> None + with capture_internal_exceptions(): + if "contexts" in event: + if "response" in event["contexts"]: + if "data" in event["contexts"]["response"]: + self.scrub_dict(event["contexts"]["response"]["data"]) + def scrub_extra(self, event): # type: (Event) -> None with capture_internal_exceptions(): @@ -123,6 +131,7 @@ def scrub_spans(self, event): def scrub_event(self, event): # type: (Event) -> None self.scrub_request(event) + self.scrub_response(event) self.scrub_extra(event) self.scrub_user(event) self.scrub_breadcrumbs(event) diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index 5c43fa3cc6..3983678f86 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -1273,6 +1273,35 @@ class ServerlessTimeoutWarning(Exception): # noqa: N818 pass +class SentryGraphQLClientError(Exception): + """Synthetic exception for GraphQL client errors.""" + + pass + + +def _get_graphql_operation_name(query): + # type: (dict) -> str + if query.get("operationName"): + return query["operationName"] + + query = query["query"].strip() + + match = re.match(r"^[a-z]* +([a-zA-Z0-9]+) \(?.*\)? *\{", query) + if match: + return match.group(1) + return "anonymous" + + +def _get_graphql_operation_type(query): + # type: (dict) -> str + query = query["query"].strip() + if query.startswith("mutation"): + return "mutation" + if query.startswith("subscription"): + return "subscription" + return "query" + + class TimeoutThread(threading.Thread): """Creates a Thread which runs (sleeps) for a time duration equal to waiting_time and raises a custom ServerlessTimeout exception. diff --git a/tests/integrations/aiohttp/test_aiohttp.py b/tests/integrations/aiohttp/test_aiohttp.py index 8068365334..af0abe4579 100644 --- a/tests/integrations/aiohttp/test_aiohttp.py +++ b/tests/integrations/aiohttp/test_aiohttp.py @@ -1,11 +1,12 @@ import asyncio import json from contextlib import suppress +from textwrap import dedent import pytest from aiohttp import web from aiohttp.client import ServerDisconnectedError -from aiohttp.web_request import Request +from aiohttp.web import Request, json_response from sentry_sdk import capture_message, start_transaction from sentry_sdk.integrations.aiohttp import AioHttpIntegration @@ -534,3 +535,113 @@ async def handler(request): resp.request_info.headers["baggage"] == "custom=value,sentry-trace_id=0123456789012345678901234567890,sentry-environment=production,sentry-release=d08ebdb9309e1b004c6f52202de58a09c2268e42,sentry-transaction=/interactions/other-dogs/new-dog,sentry-sample_rate=1.0,sentry-sampled=true" ) + + +@pytest.mark.asyncio +async def test_graphql_get_client_error_captured( + sentry_init, capture_events, aiohttp_raw_server, aiohttp_client +): + sentry_init(integrations=[AioHttpIntegration()]) + + graphql_response = { + "data": None, + "errors": [ + { + "message": "some error", + "locations": [{"line": 2, "column": 3}], + "path": ["pet"], + } + ], + } + + async def handler(request): + return json_response(graphql_response) + + raw_server = await aiohttp_raw_server(handler) + events = capture_events() + + client = await aiohttp_client(raw_server) + response = await client.get( + "/graphql", params={"query": "query GetPet {pet{name}}"} + ) + + assert response.status == 200 + assert await response.json() == graphql_response + + (event,) = events + + assert event["request"]["url"] == "http://127.0.0.1:{}/graphql".format( + raw_server.port + ) + assert event["request"]["method"] == "GET" + assert event["request"]["query_string"] == "query=query+GetPet+%7Bpet%7Bname%7D%7D" + assert "data" not in event["request"] + assert event["contexts"]["response"]["data"] == graphql_response + + assert event["request"]["api_target"] == "graphql" + assert event["fingerprint"] == ["GetPet", "query", 200] + assert ( + event["exception"]["values"][0]["value"] + == "GraphQL request failed, name: GetPet, type: query" + ) + + +@pytest.mark.asyncio +async def test_graphql_post_client_error_captured( + sentry_init, capture_events, aiohttp_client, aiohttp_raw_server +): + sentry_init(integrations=[AioHttpIntegration()]) + + graphql_request = { + "query": dedent( + """ + mutation AddPet ($name: String!) { + addPet(name: $name) { + id + name + } + } + """ + ), + "variables": { + "name": "Lucy", + }, + } + graphql_response = { + "data": None, + "errors": [ + { + "message": "already have too many pets", + "locations": [{"line": 1, "column": 1}], + } + ], + } + + async def handler(request): + return json_response(graphql_response) + + raw_server = await aiohttp_raw_server(handler) + events = capture_events() + + client = await aiohttp_client(raw_server) + response = await client.post("/graphql", json=graphql_request) + + assert response.status == 200 + assert await response.json() == graphql_response + + (event,) = events + + assert event["request"]["url"] == "http://127.0.0.1:{}/graphql".format( + raw_server.port + ) + assert event["request"]["method"] == "POST" + assert event["request"]["query_string"] == "" + assert event["request"]["data"] == graphql_request + assert event["contexts"]["response"]["data"] == graphql_response + + assert event["request"]["api_target"] == "graphql" + assert event["fingerprint"] == ["AddPet", "mutation", 200] + assert ( + event["exception"]["values"][0]["value"] + == "GraphQL request failed, name: AddPet, type: mutation" + ) diff --git a/tests/integrations/httpx/test_httpx.py b/tests/integrations/httpx/test_httpx.py index e141faa282..d4fc0d4566 100644 --- a/tests/integrations/httpx/test_httpx.py +++ b/tests/integrations/httpx/test_httpx.py @@ -2,7 +2,7 @@ import pytest import httpx -import responses +from textwrap import dedent from sentry_sdk import capture_message, start_transaction from sentry_sdk.consts import MATCH_ALL, SPANDATA @@ -18,7 +18,7 @@ "httpx_client", (httpx.Client(), httpx.AsyncClient()), ) -def test_crumb_capture_and_hint(sentry_init, capture_events, httpx_client): +def test_crumb_capture_and_hint(sentry_init, capture_events, httpx_client, httpx_mock): def before_breadcrumb(crumb, hint): crumb["data"]["extra"] = "foo" return crumb @@ -26,7 +26,7 @@ def before_breadcrumb(crumb, hint): sentry_init(integrations=[HttpxIntegration()], before_breadcrumb=before_breadcrumb) url = "http://example.com/" - responses.add(responses.GET, url, status=200) + httpx_mock.add_response() with start_transaction(): events = capture_events() @@ -61,11 +61,11 @@ def before_breadcrumb(crumb, hint): "httpx_client", (httpx.Client(), httpx.AsyncClient()), ) -def test_outgoing_trace_headers(sentry_init, httpx_client): +def test_outgoing_trace_headers(sentry_init, httpx_client, httpx_mock): sentry_init(traces_sample_rate=1.0, integrations=[HttpxIntegration()]) url = "http://example.com/" - responses.add(responses.GET, url, status=200) + httpx_mock.add_response() with start_transaction( name="/interactions/other-dogs/new-dog", @@ -93,7 +93,9 @@ def test_outgoing_trace_headers(sentry_init, httpx_client): "httpx_client", (httpx.Client(), httpx.AsyncClient()), ) -def test_outgoing_trace_headers_append_to_baggage(sentry_init, httpx_client): +def test_outgoing_trace_headers_append_to_baggage( + sentry_init, httpx_client, httpx_mock +): sentry_init( traces_sample_rate=1.0, integrations=[HttpxIntegration()], @@ -101,7 +103,7 @@ def test_outgoing_trace_headers_append_to_baggage(sentry_init, httpx_client): ) url = "http://example.com/" - responses.add(responses.GET, url, status=200) + httpx_mock.add_response() with start_transaction( name="/interactions/other-dogs/new-dog", @@ -273,12 +275,12 @@ def test_option_trace_propagation_targets( @pytest.mark.tests_internal_exceptions -def test_omit_url_data_if_parsing_fails(sentry_init, capture_events): +def test_omit_url_data_if_parsing_fails(sentry_init, capture_events, httpx_mock): sentry_init(integrations=[HttpxIntegration()]) httpx_client = httpx.Client() url = "http://example.com" - responses.add(responses.GET, url, status=200) + httpx_mock.add_response() events = capture_events() with mock.patch( @@ -297,3 +299,122 @@ def test_omit_url_data_if_parsing_fails(sentry_init, capture_events): "reason": "OK", # no url related data } + + +@pytest.mark.parametrize( + "httpx_client", + (httpx.Client(), httpx.AsyncClient()), +) +def test_graphql_get_client_error_captured( + sentry_init, capture_events, httpx_client, httpx_mock +): + sentry_init(integrations=[HttpxIntegration()]) + + url = "http://example.com/graphql" + graphql_response = { + "data": None, + "errors": [ + { + "message": "some error", + "locations": [{"line": 2, "column": 3}], + "path": ["user"], + } + ], + } + httpx_mock.add_response(method="GET", json=graphql_response) + + events = capture_events() + + if asyncio.iscoroutinefunction(httpx_client.get): + response = asyncio.get_event_loop().run_until_complete( + httpx_client.get(url, params={"query": "query QueryName {user{name}}"}) + ) + else: + response = httpx_client.get( + url, params={"query": "query QueryName {user{name}}"} + ) + + assert response.status_code == 200 + assert response.json() == graphql_response + + (event,) = events + + assert event["request"]["url"] == url + assert event["request"]["method"] == "GET" + assert ( + event["request"]["query_string"] + == "query=query%20QueryName%20%7Buser%7Bname%7D%7D" + ) + assert "data" not in event["request"] + assert event["contexts"]["response"]["data"] == graphql_response + + assert event["request"]["api_target"] == "graphql" + assert event["fingerprint"] == ["QueryName", "query", 200] + assert ( + event["exception"]["values"][0]["value"] + == "GraphQL request failed, name: QueryName, type: query" + ) + + +@pytest.mark.parametrize( + "httpx_client", + (httpx.Client(), httpx.AsyncClient()), +) +def test_graphql_post_client_error_captured( + sentry_init, capture_events, httpx_client, httpx_mock +): + sentry_init(integrations=[HttpxIntegration()]) + + url = "http://example.com/graphql" + graphql_request = { + "query": dedent( + """ + mutation AddPet ($name: String!) { + addPet(name: $name) { + id + name + } + } + """ + ), + "variables": { + "name": "Lucy", + }, + } + graphql_response = { + "data": None, + "errors": [ + { + "message": "already have too many pets", + "locations": [{"line": 1, "column": 1}], + } + ], + } + httpx_mock.add_response(method="POST", json=graphql_response) + + events = capture_events() + + if asyncio.iscoroutinefunction(httpx_client.post): + response = asyncio.get_event_loop().run_until_complete( + httpx_client.post(url, json=graphql_request) + ) + else: + response = httpx_client.post(url, json=graphql_request) + + assert response.status_code == 200 + assert response.json() == graphql_response + + (event,) = events + + assert event["request"]["url"] == url + assert event["request"]["method"] == "POST" + assert event["request"]["query_string"] == "" + assert event["request"]["data"] == graphql_request + assert event["contexts"]["response"]["data"] == graphql_response + + assert event["request"]["api_target"] == "graphql" + assert event["fingerprint"] == ["AddPet", "mutation", 200] + assert ( + event["exception"]["values"][0]["value"] + == "GraphQL request failed, name: AddPet, type: mutation" + ) diff --git a/tests/integrations/requests/test_requests.py b/tests/integrations/requests/test_requests.py index aecf64762d..f9c72eae8d 100644 --- a/tests/integrations/requests/test_requests.py +++ b/tests/integrations/requests/test_requests.py @@ -1,5 +1,6 @@ import pytest import responses +from textwrap import dedent requests = pytest.importorskip("requests") @@ -62,3 +63,92 @@ def test_omit_url_data_if_parsing_fails(sentry_init, capture_events): "reason": response.reason, # no url related data } + + +def test_graphql_get_client_error_captured(sentry_init, capture_events): + sentry_init(integrations=[StdlibIntegration()]) + + url = "http://example.com/graphql" + response = { + "data": None, + "errors": [ + { + "message": "some error", + "locations": [{"line": 2, "column": 3}], + "path": ["user"], + } + ], + } + responses.add(responses.GET, url, status=200, json=response) + + events = capture_events() + + requests.get(url, params={"query": "query QueryName {user{name}}"}) + + (event,) = events + + assert event["request"]["url"] == url + assert event["request"]["method"] == "GET" + assert ( + event["request"]["query_string"] + == "query=query%20QueryName%20%7Buser%7Bname%7D%7D" + ) + assert "data" not in event["request"] + assert event["contexts"]["response"]["data"] == response + + assert event["request"]["api_target"] == "graphql" + assert event["fingerprint"] == ["QueryName", "query", 200] + assert ( + event["exception"]["values"][0]["value"] + == "GraphQL request failed, name: QueryName, type: query" + ) + + +def test_graphql_post_client_error_captured(sentry_init, capture_events): + sentry_init(integrations=[StdlibIntegration()]) + + url = "http://example.com/graphql" + request = { + "query": dedent( + """ + mutation AddPet ($name: String!) { + addPet(name: $name) { + id + name + } + } + """ + ), + "variables": { + "name": "Lucy", + }, + } + response = { + "data": None, + "errors": [ + { + "message": "already have too many pets", + "locations": [{"line": 1, "column": 1}], + } + ], + } + responses.add(responses.POST, url, status=200, json=response) + + events = capture_events() + + requests.post(url, json=request) + + (event,) = events + + assert event["request"]["url"] == url + assert event["request"]["method"] == "POST" + assert event["request"]["query_string"] == "" + assert event["request"]["data"] == request + assert event["contexts"]["response"]["data"] == response + + assert event["request"]["api_target"] == "graphql" + assert event["fingerprint"] == ["AddPet", "mutation", 200] + assert ( + event["exception"]["values"][0]["value"] + == "GraphQL request failed, name: AddPet, type: mutation" + ) From d1e2a7c8e987e9a8510430afcfdafb62de15e1da Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Wed, 12 Jul 2023 12:15:51 +0200 Subject: [PATCH 02/39] fix older on_request_chunk_sent --- sentry_sdk/integrations/aiohttp.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sentry_sdk/integrations/aiohttp.py b/sentry_sdk/integrations/aiohttp.py index 0867a59936..17a3168350 100644 --- a/sentry_sdk/integrations/aiohttp.py +++ b/sentry_sdk/integrations/aiohttp.py @@ -244,6 +244,9 @@ async def on_request_start(session, trace_config_ctx, params): trace_config_ctx.request_headers = params.headers async def on_request_chunk_sent(session, trace_config_ctx, params): + if not hasattr(params, "url") or not hasattr(params, "method"): + return + if params.url.path == "/graphql" and params.method == "POST": with capture_internal_exceptions(): trace_config_ctx.request_body = json.loads(params.chunk) From 9931ea811832f2908e5939b8830274fac14755fd Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Wed, 12 Jul 2023 12:23:46 +0200 Subject: [PATCH 03/39] fixes --- sentry_sdk/integrations/stdlib.py | 2 +- tests/integrations/aiohttp/test_aiohttp.py | 9 +++++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index 8dca57b34d..411c3dccef 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/sentry_sdk/integrations/stdlib.py @@ -127,7 +127,7 @@ def putrequest(self, method, url, *args, **kwargs): return rv def send(self, data, *args, **kwargs): - # type: (HTTPConnection, Any, *Any, *Any) -> Any + # type: (HTTPConnection, Any, *Any, **Any) -> Any if getattr(self, "_sentrysdk_is_graphql_request", False): self._sentry_request_body = data diff --git a/tests/integrations/aiohttp/test_aiohttp.py b/tests/integrations/aiohttp/test_aiohttp.py index af0abe4579..bff2b555a0 100644 --- a/tests/integrations/aiohttp/test_aiohttp.py +++ b/tests/integrations/aiohttp/test_aiohttp.py @@ -1,5 +1,6 @@ import asyncio import json +import sys from contextlib import suppress from textwrap import dedent @@ -17,6 +18,12 @@ import mock # python < 3.3 +def requires_python_version(major, minor, reason=None): + if reason is None: + reason = "Requires Python {}.{}".format(major, minor) + return pytest.mark.skipif(sys.version_info < (major, minor), reason=reason) + + @pytest.mark.asyncio async def test_basic(sentry_init, aiohttp_client, capture_events): sentry_init(integrations=[AioHttpIntegration()]) @@ -537,6 +544,7 @@ async def handler(request): ) +@requires_python_version(3, 8, "GraphQL aiohttp integration requires py>=3.8") @pytest.mark.asyncio async def test_graphql_get_client_error_captured( sentry_init, capture_events, aiohttp_raw_server, aiohttp_client @@ -586,6 +594,7 @@ async def handler(request): ) +@requires_python_version(3, 8, "GraphQL aiohttp integration requires py>=3.8") @pytest.mark.asyncio async def test_graphql_post_client_error_captured( sentry_init, capture_events, aiohttp_client, aiohttp_raw_server From da4f1385cd3d30310ef7dc4ff9aba836f5a0f97a Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Wed, 12 Jul 2023 15:52:18 +0200 Subject: [PATCH 04/39] types --- sentry_sdk/integrations/aiohttp.py | 9 +- sentry_sdk/integrations/httpx.py | 1 + sentry_sdk/integrations/stdlib.py | 14 +-- sentry_sdk/utils.py | 4 +- tests/integrations/requests/test_requests.py | 90 -------------------- 5 files changed, 18 insertions(+), 100 deletions(-) diff --git a/sentry_sdk/integrations/aiohttp.py b/sentry_sdk/integrations/aiohttp.py index 17a3168350..4540a969e8 100644 --- a/sentry_sdk/integrations/aiohttp.py +++ b/sentry_sdk/integrations/aiohttp.py @@ -50,7 +50,11 @@ if TYPE_CHECKING: from aiohttp.web_request import Request from aiohttp.abc import AbstractMatchInfo - from aiohttp import TraceRequestStartParams, TraceRequestEndParams + from aiohttp import ( + TraceRequestStartParams, + TraceRequestEndParams, + TraceRequestChunkSentParams, + ) from types import SimpleNamespace from typing import Any from typing import Dict @@ -244,6 +248,7 @@ async def on_request_start(session, trace_config_ctx, params): trace_config_ctx.request_headers = params.headers async def on_request_chunk_sent(session, trace_config_ctx, params): + # type: (ClientSession, SimpleNamespace, TraceRequestChunkSentParams) -> None if not hasattr(params, "url") or not hasattr(params, "method"): return @@ -346,7 +351,7 @@ def aiohttp_server_processor( def _make_client_processor(trace_config_ctx, response, response_content): - # type: (SimpleNamespace, Response, Optional[dict]) -> EventProcessor + # type: (SimpleNamespace, Response, Optional[Dict[str, Any]]) -> EventProcessor def aiohttp_client_processor( event, # type: Dict[str, Any] hint, # type: Dict[str, Tuple[type, BaseException, Any]] diff --git a/sentry_sdk/integrations/httpx.py b/sentry_sdk/integrations/httpx.py index 6448659eaf..b83294497e 100644 --- a/sentry_sdk/integrations/httpx.py +++ b/sentry_sdk/integrations/httpx.py @@ -207,6 +207,7 @@ def httpx_processor( def _capture_graphql_errors(hub, request, response): + # type: (Hub, Request, Response) -> None if ( request.url.path == "/graphql" and request.method in ("GET", "POST") diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index 411c3dccef..b2c02bc655 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/sentry_sdk/integrations/stdlib.py @@ -145,7 +145,7 @@ def getresponse(self, *args, **kwargs): span.set_data("reason", rv.reason) span.finish() - url = getattr(self, "_sentrysdk_url", None) + url = getattr(self, "_sentrysdk_url", None) # type: Optional[str] if url is None: return rv @@ -189,18 +189,20 @@ def getresponse(self, *args, **kwargs): def _make_request_processor(url, method, status, request_body, response_body): - # type: (str, str, int, Any, Any) -> EventProcessor + # type: (Optional[str], str, int, Any, Any) -> EventProcessor def stdlib_processor( event, # type: Dict[str, Any] hint, # type: Dict[str, Tuple[type, BaseException, Any]] ): with capture_internal_exceptions(): - parsed_url = urlparse(url) - request_info = event.setdefault("request", {}) - request_info["url"] = url + + if url is not None: + parsed_url = urlparse(url) + request_info["query_string"] = parsed_url.query + request_info["url"] = url + request_info["method"] = method - request_info["query_string"] = parsed_url.query try: request_info["data"] = json.loads(request_body) except json.JSONDecodeError: diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index 3983678f86..67d363149f 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -1280,7 +1280,7 @@ class SentryGraphQLClientError(Exception): def _get_graphql_operation_name(query): - # type: (dict) -> str + # type: (Dict[str, Any]) -> str if query.get("operationName"): return query["operationName"] @@ -1293,7 +1293,7 @@ def _get_graphql_operation_name(query): def _get_graphql_operation_type(query): - # type: (dict) -> str + # type: (Dict[str, Any]) -> str query = query["query"].strip() if query.startswith("mutation"): return "mutation" diff --git a/tests/integrations/requests/test_requests.py b/tests/integrations/requests/test_requests.py index f9c72eae8d..aecf64762d 100644 --- a/tests/integrations/requests/test_requests.py +++ b/tests/integrations/requests/test_requests.py @@ -1,6 +1,5 @@ import pytest import responses -from textwrap import dedent requests = pytest.importorskip("requests") @@ -63,92 +62,3 @@ def test_omit_url_data_if_parsing_fails(sentry_init, capture_events): "reason": response.reason, # no url related data } - - -def test_graphql_get_client_error_captured(sentry_init, capture_events): - sentry_init(integrations=[StdlibIntegration()]) - - url = "http://example.com/graphql" - response = { - "data": None, - "errors": [ - { - "message": "some error", - "locations": [{"line": 2, "column": 3}], - "path": ["user"], - } - ], - } - responses.add(responses.GET, url, status=200, json=response) - - events = capture_events() - - requests.get(url, params={"query": "query QueryName {user{name}}"}) - - (event,) = events - - assert event["request"]["url"] == url - assert event["request"]["method"] == "GET" - assert ( - event["request"]["query_string"] - == "query=query%20QueryName%20%7Buser%7Bname%7D%7D" - ) - assert "data" not in event["request"] - assert event["contexts"]["response"]["data"] == response - - assert event["request"]["api_target"] == "graphql" - assert event["fingerprint"] == ["QueryName", "query", 200] - assert ( - event["exception"]["values"][0]["value"] - == "GraphQL request failed, name: QueryName, type: query" - ) - - -def test_graphql_post_client_error_captured(sentry_init, capture_events): - sentry_init(integrations=[StdlibIntegration()]) - - url = "http://example.com/graphql" - request = { - "query": dedent( - """ - mutation AddPet ($name: String!) { - addPet(name: $name) { - id - name - } - } - """ - ), - "variables": { - "name": "Lucy", - }, - } - response = { - "data": None, - "errors": [ - { - "message": "already have too many pets", - "locations": [{"line": 1, "column": 1}], - } - ], - } - responses.add(responses.POST, url, status=200, json=response) - - events = capture_events() - - requests.post(url, json=request) - - (event,) = events - - assert event["request"]["url"] == url - assert event["request"]["method"] == "POST" - assert event["request"]["query_string"] == "" - assert event["request"]["data"] == request - assert event["contexts"]["response"]["data"] == response - - assert event["request"]["api_target"] == "graphql" - assert event["fingerprint"] == ["AddPet", "mutation", 200] - assert ( - event["exception"]["values"][0]["value"] - == "GraphQL request failed, name: AddPet, type: mutation" - ) From 25af57f11d056c60fc2dd081634f058f8326e91b Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Wed, 12 Jul 2023 16:18:56 +0200 Subject: [PATCH 05/39] 2.7 fixes --- sentry_sdk/integrations/aiohttp.py | 6 +++++- sentry_sdk/integrations/httpx.py | 6 +++++- sentry_sdk/integrations/stdlib.py | 23 +++++++++++++---------- 3 files changed, 23 insertions(+), 12 deletions(-) diff --git a/sentry_sdk/integrations/aiohttp.py b/sentry_sdk/integrations/aiohttp.py index 4540a969e8..92f0468e0d 100644 --- a/sentry_sdk/integrations/aiohttp.py +++ b/sentry_sdk/integrations/aiohttp.py @@ -1,7 +1,11 @@ import json import sys import weakref -from urllib.parse import parse_qsl + +try: + from urllib.parse import parse_qsl +except ImportError: + from urlparse import parse_qsl from sentry_sdk.api import continue_trace from sentry_sdk._compat import reraise diff --git a/sentry_sdk/integrations/httpx.py b/sentry_sdk/integrations/httpx.py index b83294497e..b9bfff6f07 100644 --- a/sentry_sdk/integrations/httpx.py +++ b/sentry_sdk/integrations/httpx.py @@ -1,5 +1,9 @@ import json -from urllib.parse import parse_qsl + +try: + from urllib.parse import parse_qsl +except ImportError: + from urlparse import parse_qsl from sentry_sdk import Hub from sentry_sdk.consts import OP, SPANDATA diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index b2c02bc655..02aa980835 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/sentry_sdk/integrations/stdlib.py @@ -3,7 +3,11 @@ import subprocess import sys import platform -from urllib.parse import parse_qs, urlparse + +try: + from urllib.parse import parse_qsl +except ImportError: + from urlparse import parse_qsl from sentry_sdk.consts import OP, SPANDATA from sentry_sdk.hub import Hub @@ -158,7 +162,7 @@ def getresponse(self, *args, **kwargs): return rv if isinstance(response_body, dict) and response_body.get("errors"): - method = getattr(self, "_sentrysdk_method", None) + method = getattr(self, "_sentrysdk_method", None) # type: Optional[str] request_body = getattr(self, "_sentry_request_body", None) hub = Hub.current with hub.configure_scope() as scope: @@ -189,19 +193,18 @@ def getresponse(self, *args, **kwargs): def _make_request_processor(url, method, status, request_body, response_body): - # type: (Optional[str], str, int, Any, Any) -> EventProcessor + # type: (str, Optional[str], int, Any, Any) -> EventProcessor def stdlib_processor( event, # type: Dict[str, Any] hint, # type: Dict[str, Tuple[type, BaseException, Any]] ): + # type: (...) -> Optional[Event] with capture_internal_exceptions(): request_info = event.setdefault("request", {}) - if url is not None: - parsed_url = urlparse(url) - request_info["query_string"] = parsed_url.query - request_info["url"] = url - + parsed_url = parse_url(url, sanitize=False) + request_info["query_string"] = parsed_url.query + request_info["url"] = url request_info["method"] = method try: request_info["data"] = json.loads(request_body) @@ -213,11 +216,11 @@ def stdlib_processor( response_context = contexts.setdefault("response", {}) response_context["data"] = response_body - if parsed_url.path == "/graphql": + if parsed_url.url.endswith("/graphql"): request_info["api_target"] = "graphql" query = request_info.get("data") if method == "GET": - query = parse_qs(parsed_url.query) + query = dict(parse_qsl(parsed_url.query)) if query: operation_name = _get_graphql_operation_name(query) From 2577ef2670f7866c36c64e89b17f7b9eece1d84b Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Wed, 12 Jul 2023 16:21:31 +0200 Subject: [PATCH 06/39] ignore 2.7 imports when checking --- sentry_sdk/integrations/aiohttp.py | 2 +- sentry_sdk/integrations/httpx.py | 2 +- sentry_sdk/integrations/stdlib.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/sentry_sdk/integrations/aiohttp.py b/sentry_sdk/integrations/aiohttp.py index 92f0468e0d..f63d697187 100644 --- a/sentry_sdk/integrations/aiohttp.py +++ b/sentry_sdk/integrations/aiohttp.py @@ -5,7 +5,7 @@ try: from urllib.parse import parse_qsl except ImportError: - from urlparse import parse_qsl + from urlparse import parse_qsl # type: ignore from sentry_sdk.api import continue_trace from sentry_sdk._compat import reraise diff --git a/sentry_sdk/integrations/httpx.py b/sentry_sdk/integrations/httpx.py index b9bfff6f07..e1bb1f6b95 100644 --- a/sentry_sdk/integrations/httpx.py +++ b/sentry_sdk/integrations/httpx.py @@ -3,7 +3,7 @@ try: from urllib.parse import parse_qsl except ImportError: - from urlparse import parse_qsl + from urlparse import parse_qsl # type: ignore from sentry_sdk import Hub from sentry_sdk.consts import OP, SPANDATA diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index 02aa980835..5606f2e7b4 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/sentry_sdk/integrations/stdlib.py @@ -7,7 +7,7 @@ try: from urllib.parse import parse_qsl except ImportError: - from urlparse import parse_qsl + from urlparse import parse_qsl # type: ignore from sentry_sdk.consts import OP, SPANDATA from sentry_sdk.hub import Hub From eed47b663eebbd9e5fd79b98ce52ea225b945955 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Wed, 12 Jul 2023 16:36:45 +0200 Subject: [PATCH 07/39] change httpx query saving --- sentry_sdk/integrations/httpx.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/httpx.py b/sentry_sdk/integrations/httpx.py index e1bb1f6b95..8e31c8e676 100644 --- a/sentry_sdk/integrations/httpx.py +++ b/sentry_sdk/integrations/httpx.py @@ -193,7 +193,7 @@ def httpx_processor( query = request_info.get("data") if request.method == "GET": - query = dict(parse_qsl(request.url.query.decode())) + query = dict(parse_qsl(parsed_url.query)) if query: operation_name = _get_graphql_operation_name(query) From 0ba4217c5739e40671dcbd906644410626d59b25 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Wed, 12 Jul 2023 16:47:37 +0200 Subject: [PATCH 08/39] more robust qs parsing --- tests/integrations/httpx/test_httpx.py | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/tests/integrations/httpx/test_httpx.py b/tests/integrations/httpx/test_httpx.py index d4fc0d4566..0637490f6f 100644 --- a/tests/integrations/httpx/test_httpx.py +++ b/tests/integrations/httpx/test_httpx.py @@ -13,6 +13,11 @@ except ImportError: import mock # python < 3.3 +try: + from urllib.parse import parse_qsl +except ImportError: + from urlparse import parse_qsl # type: ignore + @pytest.mark.parametrize( "httpx_client", @@ -321,18 +326,18 @@ def test_graphql_get_client_error_captured( } ], } + params = {"query": "query QueryName {user{name}}"} + httpx_mock.add_response(method="GET", json=graphql_response) events = capture_events() if asyncio.iscoroutinefunction(httpx_client.get): response = asyncio.get_event_loop().run_until_complete( - httpx_client.get(url, params={"query": "query QueryName {user{name}}"}) + httpx_client.get(url, params=params) ) else: - response = httpx_client.get( - url, params={"query": "query QueryName {user{name}}"} - ) + response = httpx_client.get(url, params=params) assert response.status_code == 200 assert response.json() == graphql_response @@ -341,10 +346,7 @@ def test_graphql_get_client_error_captured( assert event["request"]["url"] == url assert event["request"]["method"] == "GET" - assert ( - event["request"]["query_string"] - == "query=query%20QueryName%20%7Buser%7Bname%7D%7D" - ) + assert dict(parse_qsl(event["request"]["query_string"])) == params assert "data" not in event["request"] assert event["contexts"]["response"]["data"] == graphql_response From b0ae249ae77ae39ab9eaee1d9e73499c19268f4d Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Wed, 12 Jul 2023 16:54:00 +0200 Subject: [PATCH 09/39] more aiohttp tests --- tests/integrations/aiohttp/test_aiohttp.py | 96 +++++++++++++++++++++- 1 file changed, 92 insertions(+), 4 deletions(-) diff --git a/tests/integrations/aiohttp/test_aiohttp.py b/tests/integrations/aiohttp/test_aiohttp.py index bff2b555a0..eb966e35b3 100644 --- a/tests/integrations/aiohttp/test_aiohttp.py +++ b/tests/integrations/aiohttp/test_aiohttp.py @@ -18,12 +18,18 @@ import mock # python < 3.3 -def requires_python_version(major, minor, reason=None): +def min_python_version(major, minor, reason=None): if reason is None: - reason = "Requires Python {}.{}".format(major, minor) + reason = "Requires Python {}.{} or higher".format(major, minor) return pytest.mark.skipif(sys.version_info < (major, minor), reason=reason) +def max_python_version(major, minor, reason=None): + if reason is None: + reason = "Requires Python {}.{} or lower".format(major, minor) + return pytest.mark.skipif(sys.version_info > (major, minor), reason=reason) + + @pytest.mark.asyncio async def test_basic(sentry_init, aiohttp_client, capture_events): sentry_init(integrations=[AioHttpIntegration()]) @@ -544,7 +550,7 @@ async def handler(request): ) -@requires_python_version(3, 8, "GraphQL aiohttp integration requires py>=3.8") +@min_python_version(3, 8, "GraphQL aiohttp integration requires py>=3.8") @pytest.mark.asyncio async def test_graphql_get_client_error_captured( sentry_init, capture_events, aiohttp_raw_server, aiohttp_client @@ -594,7 +600,7 @@ async def handler(request): ) -@requires_python_version(3, 8, "GraphQL aiohttp integration requires py>=3.8") +@min_python_version(3, 8, "GraphQL aiohttp integration requires py>=3.8") @pytest.mark.asyncio async def test_graphql_post_client_error_captured( sentry_init, capture_events, aiohttp_client, aiohttp_raw_server @@ -654,3 +660,85 @@ async def handler(request): event["exception"]["values"][0]["value"] == "GraphQL request failed, name: AddPet, type: mutation" ) + + +@max_python_version(3, 7, "No GraphQL aiohttp integration for py<=3.7") +@pytest.mark.asyncio +async def test_graphql_get_client_error_not_captured( + sentry_init, capture_events, aiohttp_raw_server, aiohttp_client +): + """Test that firing GraphQL requests works, there will just be no event.""" + sentry_init(integrations=[AioHttpIntegration()]) + + graphql_response = { + "data": None, + "errors": [ + { + "message": "some error", + "locations": [{"line": 2, "column": 3}], + "path": ["pet"], + } + ], + } + + async def handler(request): + return json_response(graphql_response) + + raw_server = await aiohttp_raw_server(handler) + events = capture_events() + + client = await aiohttp_client(raw_server) + response = await client.get( + "/graphql", params={"query": "query GetPet {pet{name}}"} + ) + + assert response.status == 200 + assert await response.json() == graphql_response + assert not events + + +@max_python_version(3, 7, "No GraphQL aiohttp integration for py<=3.7") +@pytest.mark.asyncio +async def test_graphql_post_client_error_not_captured( + sentry_init, capture_events, aiohttp_client, aiohttp_raw_server +): + """Test that firing GraphQL requests works, there will just be no event.""" + sentry_init(integrations=[AioHttpIntegration()]) + + graphql_request = { + "query": dedent( + """ + mutation AddPet ($name: String!) { + addPet(name: $name) { + id + name + } + } + """ + ), + "variables": { + "name": "Lucy", + }, + } + graphql_response = { + "data": None, + "errors": [ + { + "message": "already have too many pets", + "locations": [{"line": 1, "column": 1}], + } + ], + } + + async def handler(request): + return json_response(graphql_response) + + raw_server = await aiohttp_raw_server(handler) + events = capture_events() + + client = await aiohttp_client(raw_server) + response = await client.post("/graphql", json=graphql_request) + + assert response.status == 200 + assert await response.json() == graphql_response + assert not events From 3a81e1cac079fa22bf5db2b6f3ef3f48e4ba48e5 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Wed, 12 Jul 2023 16:57:08 +0200 Subject: [PATCH 10/39] older httpx compat --- sentry_sdk/integrations/httpx.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/integrations/httpx.py b/sentry_sdk/integrations/httpx.py index 8e31c8e676..2573438ece 100644 --- a/sentry_sdk/integrations/httpx.py +++ b/sentry_sdk/integrations/httpx.py @@ -176,9 +176,10 @@ def httpx_processor( request_info["headers"] = _filter_headers(dict(request.headers)) request_info["query_string"] = parsed_url.query - if request.content: + request_content = request.read() + if request_content: try: - request_info["data"] = json.loads(request.content) + request_info["data"] = json.loads(request_content) except json.JSONDecodeError: pass From 4480fd6ebc618a87e023d64aa3e7819dd3f2f1fb Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Wed, 12 Jul 2023 18:37:07 +0200 Subject: [PATCH 11/39] fix regex --- sentry_sdk/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index 67d363149f..4c090e11d5 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -1286,7 +1286,7 @@ def _get_graphql_operation_name(query): query = query["query"].strip() - match = re.match(r"^[a-z]* +([a-zA-Z0-9]+) \(?.*\)? *\{", query) + match = re.match(r"^[a-z]* *([a-zA-Z0-9]+) \(?.*\)? *\{", query) if match: return match.group(1) return "anonymous" From 2e66b1d7e66cce7a579e290ae558082a09c2dc32 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Wed, 12 Jul 2023 20:04:01 +0200 Subject: [PATCH 12/39] wip --- sentry_sdk/integrations/stdlib.py | 7 +- tests/conftest.py | 6 ++ tests/integrations/stdlib/test_httplib.py | 119 +++++++++++++++++++++- 3 files changed, 129 insertions(+), 3 deletions(-) diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index 5606f2e7b4..28f74bb07c 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/sentry_sdk/integrations/stdlib.py @@ -1,3 +1,4 @@ +import io import json import os import subprocess @@ -157,7 +158,9 @@ def getresponse(self, *args, **kwargs): if getattr(self, "_sentrysdk_is_graphql_request", False): with capture_internal_exceptions(): try: - response_body = json.loads(rv.read()) + response_data = rv.read() + response_body = json.loads(response_data) + rv.read = io.BytesIO(response_data).read except (json.JSONDecodeError, UnicodeDecodeError): return rv @@ -204,7 +207,7 @@ def stdlib_processor( parsed_url = parse_url(url, sanitize=False) request_info["query_string"] = parsed_url.query - request_info["url"] = url + request_info["url"] = parsed_url.url request_info["method"] = method try: request_info["data"] = json.loads(request_body) diff --git a/tests/conftest.py b/tests/conftest.py index d9d88067dc..cb61bbbdbf 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -584,6 +584,12 @@ def do_GET(self): # noqa: N802 self.end_headers() return + def do_POST(self): # noqa: N802 + # Process an HTTP POST request and return a response with an HTTP 200 status. + self.send_response(200) + self.end_headers() + return + def get_free_port(): s = socket.socket(socket.AF_INET, type=socket.SOCK_STREAM) diff --git a/tests/integrations/stdlib/test_httplib.py b/tests/integrations/stdlib/test_httplib.py index e40f5222d7..ef4534de11 100644 --- a/tests/integrations/stdlib/test_httplib.py +++ b/tests/integrations/stdlib/test_httplib.py @@ -1,4 +1,7 @@ +import json import random +from textwrap import dedent +from urllib.parse import urlencode import pytest @@ -16,6 +19,13 @@ # py3 from http.client import HTTPConnection, HTTPSConnection +try: + # py3 + from urllib.parse import parse_qsl +except ImportError: + # py2 + from urlparse import parse_qsl # type: ignore + try: from unittest import mock # python 3.3 and above except ImportError: @@ -27,7 +37,7 @@ from sentry_sdk.tracing import Transaction from sentry_sdk.integrations.stdlib import StdlibIntegration -from tests.conftest import create_mock_http_server +from tests.conftest import MockServerRequestHandler, create_mock_http_server PORT = create_mock_http_server() @@ -341,3 +351,110 @@ def test_option_trace_propagation_targets( else: assert "sentry-trace" not in request_headers assert "baggage" not in request_headers + + +def test_graphql_get_client_error_captured(sentry_init, capture_events): + sentry_init(integrations=[StdlibIntegration()]) + + graphql_response = { + "data": None, + "errors": [ + { + "message": "some error", + "locations": [{"line": 2, "column": 3}], + "path": ["user"], + } + ], + } + + events = capture_events() + + params = {"query": "query QueryName {user{name}}"} + + def do_GET(self): + self.send_response(200) + self.end_headers() + self.wfile.write(json.dumps(graphql_response).encode()) + + with mock.patch.object(MockServerRequestHandler, "do_GET", do_GET): + conn = HTTPConnection("localhost:{}".format(PORT)) + conn.request("GET", "/graphql?" + urlencode(params)) + response = conn.getresponse() + + assert response.read() == json.dumps(graphql_response).encode() + + (event,) = events + + assert event["request"]["url"] == "http://localhost:{}/graphql".format(PORT) + assert event["request"]["method"] == "GET" + assert dict(parse_qsl(event["request"]["query_string"])) == params + assert "data" not in event["request"] + assert ( + event["contexts"]["response"]["data"] == json.dumps(graphql_response).encode() + ) + + assert event["request"]["api_target"] == "graphql" + assert event["fingerprint"] == ["QueryName", "query", 200] + assert ( + event["exception"]["values"][0]["value"] + == "GraphQL request failed, name: QueryName, type: query" + ) + + +def test_graphql_post_client_error_captured(sentry_init, capture_events): + sentry_init(integrations=[StdlibIntegration()]) + + graphql_request = { + "query": dedent( + """ + mutation AddPet ($name: String!) { + addPet(name: $name) { + id + name + } + } + """ + ), + "variables": { + "name": "Lucy", + }, + } + graphql_response = { + "data": None, + "errors": [ + { + "message": "already have too many pets", + "locations": [{"line": 1, "column": 1}], + } + ], + } + + events = capture_events() + + def do_POST(self): + self.send_response(200) + self.end_headers() + self.wfile.write(json.dumps(graphql_response).encode()) + + with mock.patch.object(MockServerRequestHandler, "do_POST", do_POST): + conn = HTTPConnection("localhost:{}".format(PORT)) + conn.request("POST", "/graphql", body=json.dumps(graphql_request).encode()) + response = conn.getresponse() + + # the response can still be read normally + assert response.read() == json.dumps(graphql_response).encode() + + (event,) = events + + assert event["request"]["url"] == "http://localhost:{}/graphql".format(PORT) + assert event["request"]["method"] == "POST" + assert event["request"]["query_string"] == "" + assert event["request"]["data"] == graphql_request + assert event["contexts"]["response"]["data"] == graphql_response + + assert event["request"]["api_target"] == "graphql" + assert event["fingerprint"] == ["AddPet", "mutation", 200] + assert ( + event["exception"]["values"][0]["value"] + == "GraphQL request failed, name: AddPet, type: mutation" + ) From c910a35ecccd5a41c4e279c748645b77c95f3644 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Wed, 12 Jul 2023 20:08:10 +0200 Subject: [PATCH 13/39] fix test --- tests/integrations/stdlib/test_httplib.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/integrations/stdlib/test_httplib.py b/tests/integrations/stdlib/test_httplib.py index ef4534de11..c7fbe17087 100644 --- a/tests/integrations/stdlib/test_httplib.py +++ b/tests/integrations/stdlib/test_httplib.py @@ -389,9 +389,7 @@ def do_GET(self): assert event["request"]["method"] == "GET" assert dict(parse_qsl(event["request"]["query_string"])) == params assert "data" not in event["request"] - assert ( - event["contexts"]["response"]["data"] == json.dumps(graphql_response).encode() - ) + assert event["contexts"]["response"]["data"] == graphql_response assert event["request"]["api_target"] == "graphql" assert event["fingerprint"] == ["QueryName", "query", 200] From 8de1aa7d3b422f7a20364231620ad14e0f1cd3d0 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Wed, 12 Jul 2023 20:11:25 +0200 Subject: [PATCH 14/39] don't pester me --- tests/integrations/stdlib/test_httplib.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/integrations/stdlib/test_httplib.py b/tests/integrations/stdlib/test_httplib.py index c7fbe17087..4e9937e414 100644 --- a/tests/integrations/stdlib/test_httplib.py +++ b/tests/integrations/stdlib/test_httplib.py @@ -371,7 +371,7 @@ def test_graphql_get_client_error_captured(sentry_init, capture_events): params = {"query": "query QueryName {user{name}}"} - def do_GET(self): + def do_GET(self): # noqa: N802 self.send_response(200) self.end_headers() self.wfile.write(json.dumps(graphql_response).encode()) @@ -429,7 +429,7 @@ def test_graphql_post_client_error_captured(sentry_init, capture_events): events = capture_events() - def do_POST(self): + def do_POST(self): # noqa: N802 self.send_response(200) self.end_headers() self.wfile.write(json.dumps(graphql_response).encode()) From e18f9b57ca482435bfeb6e2870bee92774e3db0f Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Thu, 13 Jul 2023 09:12:03 +0200 Subject: [PATCH 15/39] fix 2.7 import --- tests/integrations/stdlib/test_httplib.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/integrations/stdlib/test_httplib.py b/tests/integrations/stdlib/test_httplib.py index 4e9937e414..7114517596 100644 --- a/tests/integrations/stdlib/test_httplib.py +++ b/tests/integrations/stdlib/test_httplib.py @@ -1,7 +1,6 @@ import json import random from textwrap import dedent -from urllib.parse import urlencode import pytest @@ -21,10 +20,10 @@ try: # py3 - from urllib.parse import parse_qsl + from urllib.parse import parse_qsl, urlencode except ImportError: # py2 - from urlparse import parse_qsl # type: ignore + from urlparse import parse_qsl, urlencode # type: ignore try: from unittest import mock # python 3.3 and above From 27723055d4ba05696ff70b46c3104ccc4c9ff1b6 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Thu, 13 Jul 2023 09:20:29 +0200 Subject: [PATCH 16/39] more 2.7 import fixes --- tests/integrations/stdlib/test_httplib.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/integrations/stdlib/test_httplib.py b/tests/integrations/stdlib/test_httplib.py index 7114517596..c1596b64dd 100644 --- a/tests/integrations/stdlib/test_httplib.py +++ b/tests/integrations/stdlib/test_httplib.py @@ -23,7 +23,8 @@ from urllib.parse import parse_qsl, urlencode except ImportError: # py2 - from urlparse import parse_qsl, urlencode # type: ignore + from urlparse import parse_qsl # type: ignore + from urllib import urlencode # type: ignore try: from unittest import mock # python 3.3 and above From 5e7c99ce737ef1e0c374f49b76d0069741e84481 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Thu, 13 Jul 2023 09:30:36 +0200 Subject: [PATCH 17/39] cleanup --- sentry_sdk/integrations/stdlib.py | 6 ++++-- tests/integrations/stdlib/test_httplib.py | 6 +++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index 28f74bb07c..47bea09193 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/sentry_sdk/integrations/stdlib.py @@ -157,10 +157,12 @@ def getresponse(self, *args, **kwargs): response_body = None if getattr(self, "_sentrysdk_is_graphql_request", False): with capture_internal_exceptions(): + response_data = rv.read() + # once we've read() the body it can't be read() again by the + # app; save it so that it can be accessed again + rv.read = io.BytesIO(response_data).read try: - response_data = rv.read() response_body = json.loads(response_data) - rv.read = io.BytesIO(response_data).read except (json.JSONDecodeError, UnicodeDecodeError): return rv diff --git a/tests/integrations/stdlib/test_httplib.py b/tests/integrations/stdlib/test_httplib.py index c1596b64dd..4639f5bbfa 100644 --- a/tests/integrations/stdlib/test_httplib.py +++ b/tests/integrations/stdlib/test_httplib.py @@ -356,6 +356,7 @@ def test_option_trace_propagation_targets( def test_graphql_get_client_error_captured(sentry_init, capture_events): sentry_init(integrations=[StdlibIntegration()]) + params = {"query": "query QueryName {user{name}}"} graphql_response = { "data": None, "errors": [ @@ -369,8 +370,6 @@ def test_graphql_get_client_error_captured(sentry_init, capture_events): events = capture_events() - params = {"query": "query QueryName {user{name}}"} - def do_GET(self): # noqa: N802 self.send_response(200) self.end_headers() @@ -381,6 +380,7 @@ def do_GET(self): # noqa: N802 conn.request("GET", "/graphql?" + urlencode(params)) response = conn.getresponse() + # make sure the response can still be read() normally assert response.read() == json.dumps(graphql_response).encode() (event,) = events @@ -439,7 +439,7 @@ def do_POST(self): # noqa: N802 conn.request("POST", "/graphql", body=json.dumps(graphql_request).encode()) response = conn.getresponse() - # the response can still be read normally + # make sure the response can still be read() normally assert response.read() == json.dumps(graphql_response).encode() (event,) = events From 52b3a5970bd7c01adf98eda028249314faa31f9a Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Thu, 13 Jul 2023 09:45:27 +0200 Subject: [PATCH 18/39] More 2.7 compat --- sentry_sdk/integrations/httpx.py | 11 ++++++++++- sentry_sdk/integrations/stdlib.py | 11 ++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/integrations/httpx.py b/sentry_sdk/integrations/httpx.py index 2573438ece..75740febd0 100644 --- a/sentry_sdk/integrations/httpx.py +++ b/sentry_sdk/integrations/httpx.py @@ -1,10 +1,19 @@ import json try: + # py3 from urllib.parse import parse_qsl except ImportError: + # py2 from urlparse import parse_qsl # type: ignore +try: + # py3 + from json import JSONDecodeError +except ImportError: + # py2 doesn't throw a specialized json error, just Value/TypeErrors + JSONDecodeError = ValueError + from sentry_sdk import Hub from sentry_sdk.consts import OP, SPANDATA from sentry_sdk.integrations import Integration, DidNotEnable @@ -180,7 +189,7 @@ def httpx_processor( if request_content: try: request_info["data"] = json.loads(request_content) - except json.JSONDecodeError: + except (JSONDecodeError, TypeError): pass if response: diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index 47bea09193..81280a416a 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/sentry_sdk/integrations/stdlib.py @@ -6,10 +6,19 @@ import platform try: + # py3 from urllib.parse import parse_qsl except ImportError: + # py2 from urlparse import parse_qsl # type: ignore +try: + # py3 + from json import JSONDecodeError +except ImportError: + # py2 doesn't throw a specialized json error, just Value/TypeErrors + JSONDecodeError = ValueError + from sentry_sdk.consts import OP, SPANDATA from sentry_sdk.hub import Hub from sentry_sdk.integrations import Integration @@ -163,7 +172,7 @@ def getresponse(self, *args, **kwargs): rv.read = io.BytesIO(response_data).read try: response_body = json.loads(response_data) - except (json.JSONDecodeError, UnicodeDecodeError): + except (JSONDecodeError, UnicodeDecodeError, TypeError): return rv if isinstance(response_body, dict) and response_body.get("errors"): From 2f826544a873c7c271c0f2c51f6f6b679a70b248 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Thu, 13 Jul 2023 09:55:06 +0200 Subject: [PATCH 19/39] 2.7 --- sentry_sdk/integrations/stdlib.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index 81280a416a..872ead26e5 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/sentry_sdk/integrations/stdlib.py @@ -222,7 +222,7 @@ def stdlib_processor( request_info["method"] = method try: request_info["data"] = json.loads(request_body) - except json.JSONDecodeError: + except JSONDecodeError: pass if response_body: From 3595db4f3281e0889fbe032d463a953b25648862 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Thu, 13 Jul 2023 10:01:13 +0200 Subject: [PATCH 20/39] mypy --- sentry_sdk/integrations/httpx.py | 2 +- sentry_sdk/integrations/stdlib.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/integrations/httpx.py b/sentry_sdk/integrations/httpx.py index 75740febd0..40e040d6b2 100644 --- a/sentry_sdk/integrations/httpx.py +++ b/sentry_sdk/integrations/httpx.py @@ -12,7 +12,7 @@ from json import JSONDecodeError except ImportError: # py2 doesn't throw a specialized json error, just Value/TypeErrors - JSONDecodeError = ValueError + JSONDecodeError = ValueError # type: ignore from sentry_sdk import Hub from sentry_sdk.consts import OP, SPANDATA diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index 872ead26e5..efd220fe26 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/sentry_sdk/integrations/stdlib.py @@ -17,7 +17,7 @@ from json import JSONDecodeError except ImportError: # py2 doesn't throw a specialized json error, just Value/TypeErrors - JSONDecodeError = ValueError + JSONDecodeError = ValueError # type: ignore from sentry_sdk.consts import OP, SPANDATA from sentry_sdk.hub import Hub From 7d11f90a0b17fb486669c9c403a4018220983ff5 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Thu, 13 Jul 2023 10:20:53 +0200 Subject: [PATCH 21/39] note --- sentry_sdk/integrations/aiohttp.py | 1 + 1 file changed, 1 insertion(+) diff --git a/sentry_sdk/integrations/aiohttp.py b/sentry_sdk/integrations/aiohttp.py index f63d697187..577430c155 100644 --- a/sentry_sdk/integrations/aiohttp.py +++ b/sentry_sdk/integrations/aiohttp.py @@ -254,6 +254,7 @@ async def on_request_start(session, trace_config_ctx, params): async def on_request_chunk_sent(session, trace_config_ctx, params): # type: (ClientSession, SimpleNamespace, TraceRequestChunkSentParams) -> None if not hasattr(params, "url") or not hasattr(params, "method"): + # these are missing from params in earlier aiohttp versions return if params.url.path == "/graphql" and params.method == "POST": From 364dface24a70c62b022142e1637518fc32ac2e4 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Thu, 13 Jul 2023 10:59:22 +0200 Subject: [PATCH 22/39] compat fixes --- sentry_sdk/integrations/stdlib.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index efd220fe26..fbcca98bb4 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/sentry_sdk/integrations/stdlib.py @@ -171,7 +171,7 @@ def getresponse(self, *args, **kwargs): # app; save it so that it can be accessed again rv.read = io.BytesIO(response_data).read try: - response_body = json.loads(response_data) + response_body = json.loads(response_data.decode()) except (JSONDecodeError, UnicodeDecodeError, TypeError): return rv @@ -221,7 +221,7 @@ def stdlib_processor( request_info["url"] = parsed_url.url request_info["method"] = method try: - request_info["data"] = json.loads(request_body) + request_info["data"] = json.loads(request_body.decode()) except JSONDecodeError: pass From 2aafc36b0989380fcb988de794ab22e91d4298b3 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Thu, 13 Jul 2023 11:02:35 +0200 Subject: [PATCH 23/39] add comment --- sentry_sdk/integrations/stdlib.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index fbcca98bb4..d384cecef4 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/sentry_sdk/integrations/stdlib.py @@ -171,6 +171,8 @@ def getresponse(self, *args, **kwargs): # app; save it so that it can be accessed again rv.read = io.BytesIO(response_data).read try: + # py3.6+ json.loads() can deal with bytes out of the box, but + # for older version we have to explicitly decode first response_body = json.loads(response_data.decode()) except (JSONDecodeError, UnicodeDecodeError, TypeError): return rv From 5f01782aafc566516b89ef903c1073fbdf6dfd37 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Thu, 13 Jul 2023 13:10:02 +0200 Subject: [PATCH 24/39] fix --- sentry_sdk/integrations/aiohttp.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/aiohttp.py b/sentry_sdk/integrations/aiohttp.py index 577430c155..2f0fa1b36c 100644 --- a/sentry_sdk/integrations/aiohttp.py +++ b/sentry_sdk/integrations/aiohttp.py @@ -307,7 +307,8 @@ async def on_request_end(session, trace_config_ctx, params): ) hub.capture_event(event, hint=hint) - span.finish() + if trace_config_ctx.span is not None: + span.finish() trace_config = TraceConfig() From 6ec07702bed66bab068c4b5f2393844dbe0383a2 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Fri, 14 Jul 2023 13:47:14 +0200 Subject: [PATCH 25/39] add tests for graphql type/opname extraction --- sentry_sdk/utils.py | 6 ++- tests/test_utils.py | 90 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 94 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index 4c090e11d5..4dcb02fbe6 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -1286,7 +1286,9 @@ def _get_graphql_operation_name(query): query = query["query"].strip() - match = re.match(r"^[a-z]* *([a-zA-Z0-9]+) \(?.*\)? *\{", query) + match = re.match( + r"^[a-z]* *([a-zA-Z0-9]+) \(?.*\)? *\{", query, flags=re.IGNORECASE + ) if match: return match.group(1) return "anonymous" @@ -1294,7 +1296,7 @@ def _get_graphql_operation_name(query): def _get_graphql_operation_type(query): # type: (Dict[str, Any]) -> str - query = query["query"].strip() + query = query["query"].strip().lower() if query.startswith("mutation"): return "mutation" if query.startswith("subscription"): diff --git a/tests/test_utils.py b/tests/test_utils.py index 47460d39b0..21235e23f2 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -11,6 +11,8 @@ parse_version, sanitize_url, serialize_frame, + _get_graphql_operation_name, + _get_graphql_operation_type, ) try: @@ -423,3 +425,91 @@ def test_match_regex_list(item, regex_list, expected_result): ) def test_parse_version(version, expected_result): assert parse_version(version) == expected_result + + +@pytest.mark.parametrize( + "query,expected_result", + [ + [{"query": '{cats(id: "7") {name}}'}, "anonymous"], + [{"query": 'query {cats(id: "7") {name}}'}, "anonymous"], + [{"query": 'query CatQuery {cats(id: "7") {name}}'}, "CatQuery"], + [ + { + "query": 'mutation {addCategory(id: 6, name: "Lily", cats: [8, 2]) {name cats {name}}}' + }, + "anonymous", + ], + [ + { + "query": 'mutation categoryAdd {addCategory(id: 6, name: "Lily", cats: [8, 2]) {name cats {name}}}' + }, + "categoryAdd", + ], + [ + { + "query": "subscription {newLink {id url description postedBy {id name email}}}" + }, + "anonymous", + ], + [ + { + "query": "subscription PostSubcription {newLink {id url description postedBy {id name email}}}" + }, + "PostSubcription", + ], + [ + { + "query": 'query CatQuery {cats(id: "7") {name}}', + "operationName": "SomeOtherOperation", + "variables": {}, + }, + "SomeOtherOperation", + ], + ], +) +def test_graphql_operation_name_extraction(query, expected_result): + assert _get_graphql_operation_name(query) == expected_result + + +@pytest.mark.parametrize( + "query,expected_result", + [ + [{"query": '{cats(id: "7") {name}}'}, "query"], + [{"query": 'query {cats(id: "7") {name}}'}, "query"], + [{"query": 'query CatQuery {cats(id: "7") {name}}'}, "query"], + [ + { + "query": 'mutation {addCategory(id: 6, name: "Lily", cats: [8, 2]) {name cats {name}}}' + }, + "mutation", + ], + [ + { + "query": 'mutation categoryAdd {addCategory(id: 6, name: "Lily", cats: [8, 2]) {name cats {name}}}' + }, + "mutation", + ], + [ + { + "query": "subscription {newLink {id url description postedBy {id name email}}}" + }, + "subscription", + ], + [ + { + "query": "subscription PostSubcription {newLink {id url description postedBy {id name email}}}" + }, + "subscription", + ], + [ + { + "query": 'query CatQuery {cats(id: "7") {name}}', + "operationName": "SomeOtherOperation", + "variables": {}, + }, + "query", + ], + ], +) +def test_graphql_operation_type_extraction(query, expected_result): + assert _get_graphql_operation_type(query) == expected_result From 511b8e663bf88746195ecca53fff0a0e97378785 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Fri, 14 Jul 2023 13:57:02 +0200 Subject: [PATCH 26/39] var for better readability --- sentry_sdk/integrations/stdlib.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index d384cecef4..f9b22cf6f0 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/sentry_sdk/integrations/stdlib.py @@ -177,7 +177,10 @@ def getresponse(self, *args, **kwargs): except (JSONDecodeError, UnicodeDecodeError, TypeError): return rv - if isinstance(response_body, dict) and response_body.get("errors"): + is_graphql_response_with_errors = isinstance( + response_body, dict + ) and response_body.get("errors") + if is_graphql_response_with_errors: method = getattr(self, "_sentrysdk_method", None) # type: Optional[str] request_body = getattr(self, "_sentry_request_body", None) hub = Hub.current From cf64228c69979cfe2d964d373d02b116723ca23e Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Fri, 14 Jul 2023 14:13:47 +0200 Subject: [PATCH 27/39] add integration options --- sentry_sdk/integrations/aiohttp.py | 30 ++++++--- sentry_sdk/integrations/httpx.py | 16 +++-- sentry_sdk/integrations/stdlib.py | 98 +++++++++++++++++------------- 3 files changed, 91 insertions(+), 53 deletions(-) diff --git a/sentry_sdk/integrations/aiohttp.py b/sentry_sdk/integrations/aiohttp.py index 2f0fa1b36c..ea9b3bd62a 100644 --- a/sentry_sdk/integrations/aiohttp.py +++ b/sentry_sdk/integrations/aiohttp.py @@ -77,8 +77,8 @@ class AioHttpIntegration(Integration): identifier = "aiohttp" - def __init__(self, transaction_style="handler_name"): - # type: (str) -> None + def __init__(self, transaction_style="handler_name", capture_graphql_errors=True): + # type: (str, bool) -> None if transaction_style not in TRANSACTION_STYLE_VALUES: raise ValueError( "Invalid value for transaction_style: %s (must be in %s)" @@ -86,6 +86,8 @@ def __init__(self, transaction_style="handler_name"): ) self.transaction_style = transaction_style + self.capture_graphql_errors = capture_graphql_errors + @staticmethod def setup_once(): # type: () -> None @@ -212,7 +214,8 @@ def create_trace_config(): async def on_request_start(session, trace_config_ctx, params): # type: (ClientSession, SimpleNamespace, TraceRequestStartParams) -> None hub = Hub.current - if hub.get_integration(AioHttpIntegration) is None: + integration = hub.get_integration(AioHttpIntegration) + if integration is None: return method = params.method.upper() @@ -248,21 +251,34 @@ async def on_request_start(session, trace_config_ctx, params): trace_config_ctx.span = span - if params.url.path == "/graphql": + if integration.capture_graphql_errors and params.url.path == "/graphql": trace_config_ctx.request_headers = params.headers async def on_request_chunk_sent(session, trace_config_ctx, params): # type: (ClientSession, SimpleNamespace, TraceRequestChunkSentParams) -> None + integration = Hub.current.get_integration(AioHttpIntegration) + if integration is None: + return + if not hasattr(params, "url") or not hasattr(params, "method"): # these are missing from params in earlier aiohttp versions return - if params.url.path == "/graphql" and params.method == "POST": + if ( + integration.capture_graphql_errors + and params.url.path == "/graphql" + and params.method == "POST" + ): with capture_internal_exceptions(): trace_config_ctx.request_body = json.loads(params.chunk) async def on_request_end(session, trace_config_ctx, params): # type: (ClientSession, SimpleNamespace, TraceRequestEndParams) -> None + hub = Hub.current + integration = hub.get_integration(AioHttpIntegration) + if integration is None: + return + response = params.response if trace_config_ctx.span is not None: @@ -270,9 +286,9 @@ async def on_request_end(session, trace_config_ctx, params): span.set_http_status(int(response.status)) span.set_data("reason", response.reason) - hub = Hub.current if ( - response.url.path == "/graphql" + integration.capture_graphql_errors + and response.url.path == "/graphql" and response.method in ("GET", "POST") and response.status == 200 ): diff --git a/sentry_sdk/integrations/httpx.py b/sentry_sdk/integrations/httpx.py index 40e040d6b2..6101acf104 100644 --- a/sentry_sdk/integrations/httpx.py +++ b/sentry_sdk/integrations/httpx.py @@ -48,6 +48,10 @@ class HttpxIntegration(Integration): identifier = "httpx" + def __init__(self, capture_graphql_errors=True): + # type: (bool) -> None + self.capture_graphql_errors = capture_graphql_errors + @staticmethod def setup_once(): # type: () -> None @@ -66,7 +70,8 @@ def _install_httpx_client(): def send(self, request, **kwargs): # type: (Client, Request, **Any) -> Response hub = Hub.current - if hub.get_integration(HttpxIntegration) is None: + integration = hub.get_integration(HttpxIntegration) + if integration is None: return real_send(self, request, **kwargs) parsed_url = None @@ -107,7 +112,8 @@ def send(self, request, **kwargs): span.set_http_status(rv.status_code) span.set_data("reason", rv.reason_phrase) - _capture_graphql_errors(hub, request, rv) + if integration.capture_graphql_errors: + _capture_graphql_errors(hub, request, rv) return rv @@ -121,7 +127,8 @@ def _install_httpx_async_client(): async def send(self, request, **kwargs): # type: (AsyncClient, Request, **Any) -> Response hub = Hub.current - if hub.get_integration(HttpxIntegration) is None: + integration = hub.get_integration(HttpxIntegration) + if integration is None: return await real_send(self, request, **kwargs) parsed_url = None @@ -162,7 +169,8 @@ async def send(self, request, **kwargs): span.set_http_status(rv.status_code) span.set_data("reason", rv.reason_phrase) - _capture_graphql_errors(hub, request, rv) + if integration.capture_graphql_errors: + _capture_graphql_errors(hub, request, rv) return rv diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index f9b22cf6f0..9deaae19eb 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/sentry_sdk/integrations/stdlib.py @@ -64,6 +64,10 @@ class StdlibIntegration(Integration): identifier = "stdlib" + def __init__(self, capture_graphql_errors=True): + # type: (bool) -> None + self.capture_graphql_errors = capture_graphql_errors + @staticmethod def setup_once(): # type: () -> None @@ -142,7 +146,13 @@ def putrequest(self, method, url, *args, **kwargs): def send(self, data, *args, **kwargs): # type: (HTTPConnection, Any, *Any, **Any) -> Any - if getattr(self, "_sentrysdk_is_graphql_request", False): + integration = Hub.current.get_integration(StdlibIntegration) + if integration is None: + return real_send(self, data, *args, **kwargs) + + if integration.capture_graphql_errors and getattr( + self, "_sentrysdk_is_graphql_request", False + ): self._sentry_request_body = data rv = real_send(self, data, *args, **kwargs) @@ -150,10 +160,14 @@ def send(self, data, *args, **kwargs): def getresponse(self, *args, **kwargs): # type: (HTTPConnection, *Any, **Any) -> Any - span = getattr(self, "_sentrysdk_span", None) - rv = real_getresponse(self, *args, **kwargs) + hub = Hub.current + integration = hub.get_integration(StdlibIntegration) + if integration is None: + return rv + + span = getattr(self, "_sentrysdk_span", None) if span is not None: span.set_http_status(int(rv.status)) span.set_data("reason", rv.reason) @@ -163,46 +177,46 @@ def getresponse(self, *args, **kwargs): if url is None: return rv - response_body = None - if getattr(self, "_sentrysdk_is_graphql_request", False): - with capture_internal_exceptions(): - response_data = rv.read() - # once we've read() the body it can't be read() again by the - # app; save it so that it can be accessed again - rv.read = io.BytesIO(response_data).read - try: - # py3.6+ json.loads() can deal with bytes out of the box, but - # for older version we have to explicitly decode first - response_body = json.loads(response_data.decode()) - except (JSONDecodeError, UnicodeDecodeError, TypeError): - return rv - - is_graphql_response_with_errors = isinstance( - response_body, dict - ) and response_body.get("errors") - if is_graphql_response_with_errors: - method = getattr(self, "_sentrysdk_method", None) # type: Optional[str] - request_body = getattr(self, "_sentry_request_body", None) - hub = Hub.current - with hub.configure_scope() as scope: - scope.add_event_processor( - _make_request_processor( - url, method, rv.status, request_body, response_body + if integration.capture_graphql_errors: + response_body = None + if getattr(self, "_sentrysdk_is_graphql_request", False): + with capture_internal_exceptions(): + response_data = rv.read() + # once we've read() the body it can't be read() again by the + # app; save it so that it can be accessed again + rv.read = io.BytesIO(response_data).read + try: + # py3.6+ json.loads() can deal with bytes out of the box, but + # for older version we have to explicitly decode first + response_body = json.loads(response_data.decode()) + except (JSONDecodeError, UnicodeDecodeError, TypeError): + return rv + + is_graphql_response_with_errors = isinstance( + response_body, dict + ) and response_body.get("errors") + if is_graphql_response_with_errors: + method = getattr(self, "_sentrysdk_method", None) # type: Optional[str] + request_body = getattr(self, "_sentry_request_body", None) + with hub.configure_scope() as scope: + scope.add_event_processor( + _make_request_processor( + url, method, rv.status, request_body, response_body + ) ) - ) - try: - raise SentryGraphQLClientError - except SentryGraphQLClientError as ex: - event, hint = event_from_exception( - ex, - client_options=hub.client.options if hub.client else None, - mechanism={ - "type": StdlibIntegration.identifier, - "handled": False, - }, - ) - - hub.capture_event(event, hint=hint) + try: + raise SentryGraphQLClientError + except SentryGraphQLClientError as ex: + event, hint = event_from_exception( + ex, + client_options=hub.client.options if hub.client else None, + mechanism={ + "type": StdlibIntegration.identifier, + "handled": False, + }, + ) + + hub.capture_event(event, hint=hint) return rv From 898f689f4c2f58f4b85bd9949a418090516ad978 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Fri, 14 Jul 2023 15:07:26 +0200 Subject: [PATCH 28/39] fix test --- sentry_sdk/utils.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index 4dcb02fbe6..7274622aa8 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -1287,10 +1287,12 @@ def _get_graphql_operation_name(query): query = query["query"].strip() match = re.match( - r"^[a-z]* *([a-zA-Z0-9]+) \(?.*\)? *\{", query, flags=re.IGNORECASE + r"((query|mutation|subscription) )(?P[a-zA-Z0-9]+) *\{", + query, + flags=re.IGNORECASE, ) if match: - return match.group(1) + return match.group("name") return "anonymous" From 60c7e2486ab89981ca298b0334419c37c7be52ea Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Fri, 14 Jul 2023 15:10:33 +0200 Subject: [PATCH 29/39] format --- sentry_sdk/scrubber.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/sentry_sdk/scrubber.py b/sentry_sdk/scrubber.py index f96000fc95..8c828fe444 100644 --- a/sentry_sdk/scrubber.py +++ b/sentry_sdk/scrubber.py @@ -87,10 +87,12 @@ def scrub_request(self, event): def scrub_response(self, event): # type: (Event) -> None with capture_internal_exceptions(): - if "contexts" in event: - if "response" in event["contexts"]: - if "data" in event["contexts"]["response"]: - self.scrub_dict(event["contexts"]["response"]["data"]) + if ( + "contexts" in event + and "response" in event["contexts"] + and "data" in event["contexts"]["response"] + ): + self.scrub_dict(event["contexts"]["response"]["data"]) def scrub_extra(self, event): # type: (Event) -> None From 7ffa46513473caf74c7bc14432fccf02e3da0146 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Fri, 14 Jul 2023 15:45:05 +0200 Subject: [PATCH 30/39] add pii guards --- sentry_sdk/integrations/aiohttp.py | 13 ++++++---- sentry_sdk/integrations/httpx.py | 28 ++++++++++++---------- sentry_sdk/integrations/stdlib.py | 25 +++++++++++-------- tests/integrations/aiohttp/test_aiohttp.py | 8 +++---- tests/integrations/httpx/test_httpx.py | 4 ++-- tests/integrations/stdlib/test_httplib.py | 4 ++-- 6 files changed, 46 insertions(+), 36 deletions(-) diff --git a/sentry_sdk/integrations/aiohttp.py b/sentry_sdk/integrations/aiohttp.py index ea9b3bd62a..cd3316b8bf 100644 --- a/sentry_sdk/integrations/aiohttp.py +++ b/sentry_sdk/integrations/aiohttp.py @@ -10,7 +10,7 @@ from sentry_sdk.api import continue_trace from sentry_sdk._compat import reraise from sentry_sdk.consts import OP, SPANDATA -from sentry_sdk.hub import Hub +from sentry_sdk.hub import Hub, _should_send_default_pii from sentry_sdk.integrations import Integration, DidNotEnable from sentry_sdk.integrations.logging import ignore_logger from sentry_sdk.sessions import auto_session_tracking @@ -384,15 +384,18 @@ def aiohttp_client_processor( parsed_url = parse_url(str(response.url), sanitize=False) request_info["url"] = parsed_url.url - request_info["query_string"] = parsed_url.query request_info["method"] = response.method if getattr(trace_config_ctx, "request_headers", None): request_info["headers"] = _filter_headers( dict(trace_config_ctx.request_headers) ) - if getattr(trace_config_ctx, "request_body", None): - request_info["data"] = trace_config_ctx.request_body + + if _should_send_default_pii(): + if getattr(trace_config_ctx, "request_body", None): + request_info["data"] = trace_config_ctx.request_body + + request_info["query_string"] = parsed_url.query if response.url.path == "/graphql": request_info["api_target"] = "graphql" @@ -415,7 +418,7 @@ def aiohttp_client_processor( operation_name, operation_type ) - if response_content: + if _should_send_default_pii() and response_content: contexts = event.setdefault("contexts", {}) response_context = contexts.setdefault("response", {}) response_context["data"] = response_content diff --git a/sentry_sdk/integrations/httpx.py b/sentry_sdk/integrations/httpx.py index 6101acf104..9f0de42ea2 100644 --- a/sentry_sdk/integrations/httpx.py +++ b/sentry_sdk/integrations/httpx.py @@ -14,8 +14,8 @@ # py2 doesn't throw a specialized json error, just Value/TypeErrors JSONDecodeError = ValueError # type: ignore -from sentry_sdk import Hub from sentry_sdk.consts import OP, SPANDATA +from sentry_sdk.hub import Hub, _should_send_default_pii from sentry_sdk.integrations import Integration, DidNotEnable from sentry_sdk.tracing import BAGGAGE_HEADER_NAME from sentry_sdk.tracing_utils import should_propagate_trace @@ -191,20 +191,22 @@ def httpx_processor( request_info["url"] = parsed_url.url request_info["method"] = request.method request_info["headers"] = _filter_headers(dict(request.headers)) - request_info["query_string"] = parsed_url.query - request_content = request.read() - if request_content: - try: - request_info["data"] = json.loads(request_content) - except (JSONDecodeError, TypeError): - pass + if _should_send_default_pii(): + request_info["query_string"] = parsed_url.query - if response: - response_content = response.json() - contexts = event.setdefault("contexts", {}) - response_context = contexts.setdefault("response", {}) - response_context["data"] = response_content + request_content = request.read() + if request_content: + try: + request_info["data"] = json.loads(request_content) + except (JSONDecodeError, TypeError): + pass + + if response: + response_content = response.json() + contexts = event.setdefault("contexts", {}) + response_context = contexts.setdefault("response", {}) + response_context["data"] = response_content if request.url.path == "/graphql": request_info["api_target"] = "graphql" diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index 9deaae19eb..9d0fd424b4 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/sentry_sdk/integrations/stdlib.py @@ -20,7 +20,7 @@ JSONDecodeError = ValueError # type: ignore from sentry_sdk.consts import OP, SPANDATA -from sentry_sdk.hub import Hub +from sentry_sdk.hub import Hub, _should_send_default_pii from sentry_sdk.integrations import Integration from sentry_sdk.scope import add_global_event_processor from sentry_sdk.tracing_utils import EnvironHeaders, should_propagate_trace @@ -236,18 +236,23 @@ def stdlib_processor( request_info = event.setdefault("request", {}) parsed_url = parse_url(url, sanitize=False) - request_info["query_string"] = parsed_url.query + + if _should_send_default_pii(): + request_info["query_string"] = parsed_url.query + request_info["url"] = parsed_url.url request_info["method"] = method - try: - request_info["data"] = json.loads(request_body.decode()) - except JSONDecodeError: - pass - if response_body: - contexts = event.setdefault("contexts", {}) - response_context = contexts.setdefault("response", {}) - response_context["data"] = response_body + if _should_send_default_pii(): + try: + request_info["data"] = json.loads(request_body.decode()) + except JSONDecodeError: + pass + + if response_body: + contexts = event.setdefault("contexts", {}) + response_context = contexts.setdefault("response", {}) + response_context["data"] = response_body if parsed_url.url.endswith("/graphql"): request_info["api_target"] = "graphql" diff --git a/tests/integrations/aiohttp/test_aiohttp.py b/tests/integrations/aiohttp/test_aiohttp.py index eb966e35b3..dd294875ea 100644 --- a/tests/integrations/aiohttp/test_aiohttp.py +++ b/tests/integrations/aiohttp/test_aiohttp.py @@ -555,7 +555,7 @@ async def handler(request): async def test_graphql_get_client_error_captured( sentry_init, capture_events, aiohttp_raw_server, aiohttp_client ): - sentry_init(integrations=[AioHttpIntegration()]) + sentry_init(send_default_pii=True, integrations=[AioHttpIntegration()]) graphql_response = { "data": None, @@ -605,7 +605,7 @@ async def handler(request): async def test_graphql_post_client_error_captured( sentry_init, capture_events, aiohttp_client, aiohttp_raw_server ): - sentry_init(integrations=[AioHttpIntegration()]) + sentry_init(send_default_pii=True, integrations=[AioHttpIntegration()]) graphql_request = { "query": dedent( @@ -668,7 +668,7 @@ async def test_graphql_get_client_error_not_captured( sentry_init, capture_events, aiohttp_raw_server, aiohttp_client ): """Test that firing GraphQL requests works, there will just be no event.""" - sentry_init(integrations=[AioHttpIntegration()]) + sentry_init(send_default_pii=True, integrations=[AioHttpIntegration()]) graphql_response = { "data": None, @@ -703,7 +703,7 @@ async def test_graphql_post_client_error_not_captured( sentry_init, capture_events, aiohttp_client, aiohttp_raw_server ): """Test that firing GraphQL requests works, there will just be no event.""" - sentry_init(integrations=[AioHttpIntegration()]) + sentry_init(send_default_pii=True, integrations=[AioHttpIntegration()]) graphql_request = { "query": dedent( diff --git a/tests/integrations/httpx/test_httpx.py b/tests/integrations/httpx/test_httpx.py index 0637490f6f..38718951c6 100644 --- a/tests/integrations/httpx/test_httpx.py +++ b/tests/integrations/httpx/test_httpx.py @@ -313,7 +313,7 @@ def test_omit_url_data_if_parsing_fails(sentry_init, capture_events, httpx_mock) def test_graphql_get_client_error_captured( sentry_init, capture_events, httpx_client, httpx_mock ): - sentry_init(integrations=[HttpxIntegration()]) + sentry_init(send_default_pii=True, integrations=[HttpxIntegration()]) url = "http://example.com/graphql" graphql_response = { @@ -365,7 +365,7 @@ def test_graphql_get_client_error_captured( def test_graphql_post_client_error_captured( sentry_init, capture_events, httpx_client, httpx_mock ): - sentry_init(integrations=[HttpxIntegration()]) + sentry_init(send_default_pii=True, integrations=[HttpxIntegration()]) url = "http://example.com/graphql" graphql_request = { diff --git a/tests/integrations/stdlib/test_httplib.py b/tests/integrations/stdlib/test_httplib.py index 4639f5bbfa..2d7c87af69 100644 --- a/tests/integrations/stdlib/test_httplib.py +++ b/tests/integrations/stdlib/test_httplib.py @@ -354,7 +354,7 @@ def test_option_trace_propagation_targets( def test_graphql_get_client_error_captured(sentry_init, capture_events): - sentry_init(integrations=[StdlibIntegration()]) + sentry_init(send_default_pii=True, integrations=[StdlibIntegration()]) params = {"query": "query QueryName {user{name}}"} graphql_response = { @@ -400,7 +400,7 @@ def do_GET(self): # noqa: N802 def test_graphql_post_client_error_captured(sentry_init, capture_events): - sentry_init(integrations=[StdlibIntegration()]) + sentry_init(send_default_pii=True, integrations=[StdlibIntegration()]) graphql_request = { "query": dedent( From c18ee003ee2cb23a32af8a86a4d3b7ac3514225a Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Fri, 14 Jul 2023 16:01:13 +0200 Subject: [PATCH 31/39] update graphql regex --- sentry_sdk/utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index 7274622aa8..789b9901fe 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -1287,7 +1287,7 @@ def _get_graphql_operation_name(query): query = query["query"].strip() match = re.match( - r"((query|mutation|subscription) )(?P[a-zA-Z0-9]+) *\{", + r"((query|mutation|subscription) )(?P[a-zA-Z0-9]+).*\{", query, flags=re.IGNORECASE, ) From 3726bc53f93a1ab8ecf88fb00ee7647c7ba19089 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Fri, 14 Jul 2023 16:01:45 +0200 Subject: [PATCH 32/39] add test cases --- tests/test_utils.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/test_utils.py b/tests/test_utils.py index 21235e23f2..3a5a4bd384 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -465,6 +465,12 @@ def test_parse_version(version, expected_result): }, "SomeOtherOperation", ], + [ + { + "query": "mutation AddPet ($name: String!) {addPet(name: $name) {id name}}}" + }, + "AddPet", + ], ], ) def test_graphql_operation_name_extraction(query, expected_result): @@ -509,6 +515,12 @@ def test_graphql_operation_name_extraction(query, expected_result): }, "query", ], + [ + { + "query": "mutation AddPet ($name: String!) {addPet(name: $name) {id name}}}" + }, + "mutation", + ], ], ) def test_graphql_operation_type_extraction(query, expected_result): From 2ede9de3dc4b7d43f0a823f5fa8dc65b67d9db40 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Fri, 14 Jul 2023 16:32:58 +0200 Subject: [PATCH 33/39] fix aiohttp and tests --- sentry_sdk/integrations/aiohttp.py | 21 ++-- tests/integrations/aiohttp/test_aiohttp.py | 110 ++++----------------- 2 files changed, 28 insertions(+), 103 deletions(-) diff --git a/sentry_sdk/integrations/aiohttp.py b/sentry_sdk/integrations/aiohttp.py index cd3316b8bf..5cbcc876ac 100644 --- a/sentry_sdk/integrations/aiohttp.py +++ b/sentry_sdk/integrations/aiohttp.py @@ -250,8 +250,9 @@ async def on_request_start(session, trace_config_ctx, params): params.headers[key] = value trace_config_ctx.span = span + trace_config_ctx.is_graphql_request = params.url.path == "/graphql" - if integration.capture_graphql_errors and params.url.path == "/graphql": + if integration.capture_graphql_errors and trace_config_ctx.is_graphql_request: trace_config_ctx.request_headers = params.headers async def on_request_chunk_sent(session, trace_config_ctx, params): @@ -260,17 +261,13 @@ async def on_request_chunk_sent(session, trace_config_ctx, params): if integration is None: return - if not hasattr(params, "url") or not hasattr(params, "method"): - # these are missing from params in earlier aiohttp versions - return - - if ( - integration.capture_graphql_errors - and params.url.path == "/graphql" - and params.method == "POST" - ): + if integration.capture_graphql_errors and trace_config_ctx.is_graphql_request: + trace_config_ctx.request_body = None with capture_internal_exceptions(): - trace_config_ctx.request_body = json.loads(params.chunk) + try: + trace_config_ctx.request_body = json.loads(params.chunk) + except json.JSONDecodeError: + return async def on_request_end(session, trace_config_ctx, params): # type: (ClientSession, SimpleNamespace, TraceRequestEndParams) -> None @@ -288,7 +285,7 @@ async def on_request_end(session, trace_config_ctx, params): if ( integration.capture_graphql_errors - and response.url.path == "/graphql" + and trace_config_ctx.is_graphql_request and response.method in ("GET", "POST") and response.status == 200 ): diff --git a/tests/integrations/aiohttp/test_aiohttp.py b/tests/integrations/aiohttp/test_aiohttp.py index dd294875ea..bf52c4fa91 100644 --- a/tests/integrations/aiohttp/test_aiohttp.py +++ b/tests/integrations/aiohttp/test_aiohttp.py @@ -1,6 +1,5 @@ import asyncio import json -import sys from contextlib import suppress from textwrap import dedent @@ -11,23 +10,36 @@ from sentry_sdk import capture_message, start_transaction from sentry_sdk.integrations.aiohttp import AioHttpIntegration +from sentry_sdk.utils import parse_version try: from unittest import mock # python 3.3 and above except ImportError: import mock # python < 3.3 +try: + from importlib.metadata import version # py 3.8+ + + AIOHTTP_VERSION = tuple(parse_version(version("aiohttp"))[:2]) + +except ImportError: + from pkg_resources import get_distribution + + AIOHTTP_VERSION = tuple(parse_version(get_distribution("aiohttp").version)[:2]) + -def min_python_version(major, minor, reason=None): +def min_aiohttp_version(major, minor, reason=None): if reason is None: - reason = "Requires Python {}.{} or higher".format(major, minor) - return pytest.mark.skipif(sys.version_info < (major, minor), reason=reason) + reason = "Requires aiohttp {}.{} or higher".format(major, minor) + return pytest.mark.skipif(AIOHTTP_VERSION < (major, minor), reason=reason) -def max_python_version(major, minor, reason=None): + +def max_aiohttp_version(major, minor, reason=None): if reason is None: - reason = "Requires Python {}.{} or lower".format(major, minor) - return pytest.mark.skipif(sys.version_info > (major, minor), reason=reason) + reason = "Requires aiohttp {}.{} or lower".format(major, minor) + + return pytest.mark.skipif(AIOHTTP_VERSION > (major, minor), reason=reason) @pytest.mark.asyncio @@ -550,7 +562,6 @@ async def handler(request): ) -@min_python_version(3, 8, "GraphQL aiohttp integration requires py>=3.8") @pytest.mark.asyncio async def test_graphql_get_client_error_captured( sentry_init, capture_events, aiohttp_raw_server, aiohttp_client @@ -600,7 +611,6 @@ async def handler(request): ) -@min_python_version(3, 8, "GraphQL aiohttp integration requires py>=3.8") @pytest.mark.asyncio async def test_graphql_post_client_error_captured( sentry_init, capture_events, aiohttp_client, aiohttp_raw_server @@ -660,85 +670,3 @@ async def handler(request): event["exception"]["values"][0]["value"] == "GraphQL request failed, name: AddPet, type: mutation" ) - - -@max_python_version(3, 7, "No GraphQL aiohttp integration for py<=3.7") -@pytest.mark.asyncio -async def test_graphql_get_client_error_not_captured( - sentry_init, capture_events, aiohttp_raw_server, aiohttp_client -): - """Test that firing GraphQL requests works, there will just be no event.""" - sentry_init(send_default_pii=True, integrations=[AioHttpIntegration()]) - - graphql_response = { - "data": None, - "errors": [ - { - "message": "some error", - "locations": [{"line": 2, "column": 3}], - "path": ["pet"], - } - ], - } - - async def handler(request): - return json_response(graphql_response) - - raw_server = await aiohttp_raw_server(handler) - events = capture_events() - - client = await aiohttp_client(raw_server) - response = await client.get( - "/graphql", params={"query": "query GetPet {pet{name}}"} - ) - - assert response.status == 200 - assert await response.json() == graphql_response - assert not events - - -@max_python_version(3, 7, "No GraphQL aiohttp integration for py<=3.7") -@pytest.mark.asyncio -async def test_graphql_post_client_error_not_captured( - sentry_init, capture_events, aiohttp_client, aiohttp_raw_server -): - """Test that firing GraphQL requests works, there will just be no event.""" - sentry_init(send_default_pii=True, integrations=[AioHttpIntegration()]) - - graphql_request = { - "query": dedent( - """ - mutation AddPet ($name: String!) { - addPet(name: $name) { - id - name - } - } - """ - ), - "variables": { - "name": "Lucy", - }, - } - graphql_response = { - "data": None, - "errors": [ - { - "message": "already have too many pets", - "locations": [{"line": 1, "column": 1}], - } - ], - } - - async def handler(request): - return json_response(graphql_response) - - raw_server = await aiohttp_raw_server(handler) - events = capture_events() - - client = await aiohttp_client(raw_server) - response = await client.post("/graphql", json=graphql_request) - - assert response.status == 200 - assert await response.json() == graphql_response - assert not events From 85e155e1eec767d8f4951fcb22fadf5d25a90d50 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Fri, 14 Jul 2023 17:26:04 +0200 Subject: [PATCH 34/39] fix 2.7 --- sentry_sdk/integrations/stdlib.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/sentry_sdk/integrations/stdlib.py b/sentry_sdk/integrations/stdlib.py index 9d0fd424b4..43049a06a7 100644 --- a/sentry_sdk/integrations/stdlib.py +++ b/sentry_sdk/integrations/stdlib.py @@ -88,7 +88,7 @@ def add_python_runtime_context(event, hint): def _install_httplib(): # type: () -> None real_putrequest = HTTPConnection.putrequest - real_send = HTTPConnection.send + real_endheaders = HTTPConnection.endheaders real_getresponse = HTTPConnection.getresponse def putrequest(self, method, url, *args, **kwargs): @@ -144,18 +144,19 @@ def putrequest(self, method, url, *args, **kwargs): return rv - def send(self, data, *args, **kwargs): - # type: (HTTPConnection, Any, *Any, **Any) -> Any + def endheaders(self, message_body=None, **kwargs): + # type: (HTTPConnection, Any, **Any) -> Any + rv = real_endheaders(self, message_body, **kwargs) + integration = Hub.current.get_integration(StdlibIntegration) if integration is None: - return real_send(self, data, *args, **kwargs) + return rv if integration.capture_graphql_errors and getattr( self, "_sentrysdk_is_graphql_request", False ): - self._sentry_request_body = data + self._sentry_request_body = message_body - rv = real_send(self, data, *args, **kwargs) return rv def getresponse(self, *args, **kwargs): @@ -221,7 +222,7 @@ def getresponse(self, *args, **kwargs): return rv HTTPConnection.putrequest = putrequest - HTTPConnection.send = send + HTTPConnection.endheaders = endheaders HTTPConnection.getresponse = getresponse @@ -246,7 +247,7 @@ def stdlib_processor( if _should_send_default_pii(): try: request_info["data"] = json.loads(request_body.decode()) - except JSONDecodeError: + except (JSONDecodeError, AttributeError): pass if response_body: From f8c1c64b6887641c6cf9532a64091ea64199a4c3 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Mon, 17 Jul 2023 12:37:49 +0200 Subject: [PATCH 35/39] add integration option tests --- tests/integrations/aiohttp/test_aiohttp.py | 86 +++++++++++++++++++ tests/integrations/httpx/test_httpx.py | 96 ++++++++++++++++++++++ tests/integrations/stdlib/test_httplib.py | 85 +++++++++++++++++++ 3 files changed, 267 insertions(+) diff --git a/tests/integrations/aiohttp/test_aiohttp.py b/tests/integrations/aiohttp/test_aiohttp.py index bf52c4fa91..755997842a 100644 --- a/tests/integrations/aiohttp/test_aiohttp.py +++ b/tests/integrations/aiohttp/test_aiohttp.py @@ -670,3 +670,89 @@ async def handler(request): event["exception"]["values"][0]["value"] == "GraphQL request failed, name: AddPet, type: mutation" ) + + +@pytest.mark.asyncio +async def test_graphql_no_get_errors_if_option_is_off( + sentry_init, capture_events, aiohttp_raw_server, aiohttp_client +): + sentry_init( + send_default_pii=True, + integrations=[AioHttpIntegration(capture_graphql_errors=False)], + ) + + graphql_response = { + "data": None, + "errors": [ + { + "message": "some error", + "locations": [{"line": 2, "column": 3}], + "path": ["pet"], + } + ], + } + + async def handler(request): + return json_response(graphql_response) + + raw_server = await aiohttp_raw_server(handler) + events = capture_events() + + client = await aiohttp_client(raw_server) + response = await client.get( + "/graphql", params={"query": "query GetPet {pet{name}}"} + ) + + assert response.status == 200 + assert await response.json() == graphql_response + + assert not events + + +@pytest.mark.asyncio +async def test_graphql_no_post_errors_if_option_is_off( + sentry_init, capture_events, aiohttp_client, aiohttp_raw_server +): + sentry_init( + send_default_pii=True, + integrations=[AioHttpIntegration(capture_graphql_errors=False)], + ) + + graphql_request = { + "query": dedent( + """ + mutation AddPet ($name: String!) { + addPet(name: $name) { + id + name + } + } + """ + ), + "variables": { + "name": "Lucy", + }, + } + graphql_response = { + "data": None, + "errors": [ + { + "message": "already have too many pets", + "locations": [{"line": 1, "column": 1}], + } + ], + } + + async def handler(request): + return json_response(graphql_response) + + raw_server = await aiohttp_raw_server(handler) + events = capture_events() + + client = await aiohttp_client(raw_server) + response = await client.post("/graphql", json=graphql_request) + + assert response.status == 200 + assert await response.json() == graphql_response + + assert not events diff --git a/tests/integrations/httpx/test_httpx.py b/tests/integrations/httpx/test_httpx.py index 38718951c6..25819ae0d0 100644 --- a/tests/integrations/httpx/test_httpx.py +++ b/tests/integrations/httpx/test_httpx.py @@ -420,3 +420,99 @@ def test_graphql_post_client_error_captured( event["exception"]["values"][0]["value"] == "GraphQL request failed, name: AddPet, type: mutation" ) + + +@pytest.mark.parametrize( + "httpx_client", + (httpx.Client(), httpx.AsyncClient()), +) +def test_graphql_no_get_errors_if_option_is_off( + sentry_init, capture_events, httpx_client, httpx_mock +): + sentry_init( + send_default_pii=True, + integrations=[HttpxIntegration(capture_graphql_errors=False)], + ) + + url = "http://example.com/graphql" + graphql_response = { + "data": None, + "errors": [ + { + "message": "some error", + "locations": [{"line": 2, "column": 3}], + "path": ["user"], + } + ], + } + params = {"query": "query QueryName {user{name}}"} + + httpx_mock.add_response(method="GET", json=graphql_response) + + events = capture_events() + + if asyncio.iscoroutinefunction(httpx_client.get): + response = asyncio.get_event_loop().run_until_complete( + httpx_client.get(url, params=params) + ) + else: + response = httpx_client.get(url, params=params) + + assert response.status_code == 200 + assert response.json() == graphql_response + + assert not events + + +@pytest.mark.parametrize( + "httpx_client", + (httpx.Client(), httpx.AsyncClient()), +) +def test_graphql_no_post_errors_if_option_is_off( + sentry_init, capture_events, httpx_client, httpx_mock +): + sentry_init( + send_default_pii=True, + integrations=[HttpxIntegration(capture_graphql_errors=False)], + ) + + url = "http://example.com/graphql" + graphql_request = { + "query": dedent( + """ + mutation AddPet ($name: String!) { + addPet(name: $name) { + id + name + } + } + """ + ), + "variables": { + "name": "Lucy", + }, + } + graphql_response = { + "data": None, + "errors": [ + { + "message": "already have too many pets", + "locations": [{"line": 1, "column": 1}], + } + ], + } + httpx_mock.add_response(method="POST", json=graphql_response) + + events = capture_events() + + if asyncio.iscoroutinefunction(httpx_client.post): + response = asyncio.get_event_loop().run_until_complete( + httpx_client.post(url, json=graphql_request) + ) + else: + response = httpx_client.post(url, json=graphql_request) + + assert response.status_code == 200 + assert response.json() == graphql_response + + assert not events diff --git a/tests/integrations/stdlib/test_httplib.py b/tests/integrations/stdlib/test_httplib.py index 2d7c87af69..10897e1e36 100644 --- a/tests/integrations/stdlib/test_httplib.py +++ b/tests/integrations/stdlib/test_httplib.py @@ -456,3 +456,88 @@ def do_POST(self): # noqa: N802 event["exception"]["values"][0]["value"] == "GraphQL request failed, name: AddPet, type: mutation" ) + + +def test_graphql_no_get_errors_if_option_is_off(sentry_init, capture_events): + sentry_init( + send_default_pii=True, + integrations=[StdlibIntegration(capture_graphql_errors=False)], + ) + + params = {"query": "query QueryName {user{name}}"} + graphql_response = { + "data": None, + "errors": [ + { + "message": "some error", + "locations": [{"line": 2, "column": 3}], + "path": ["user"], + } + ], + } + + events = capture_events() + + def do_GET(self): # noqa: N802 + self.send_response(200) + self.end_headers() + self.wfile.write(json.dumps(graphql_response).encode()) + + with mock.patch.object(MockServerRequestHandler, "do_GET", do_GET): + conn = HTTPConnection("localhost:{}".format(PORT)) + conn.request("GET", "/graphql?" + urlencode(params)) + response = conn.getresponse() + + # make sure the response can still be read() normally + assert response.read() == json.dumps(graphql_response).encode() + + assert not events + + +def test_graphql_no_post_errors_if_option_is_off(sentry_init, capture_events): + sentry_init( + send_default_pii=True, + integrations=[StdlibIntegration(capture_graphql_errors=False)], + ) + + graphql_request = { + "query": dedent( + """ + mutation AddPet ($name: String!) { + addPet(name: $name) { + id + name + } + } + """ + ), + "variables": { + "name": "Lucy", + }, + } + graphql_response = { + "data": None, + "errors": [ + { + "message": "already have too many pets", + "locations": [{"line": 1, "column": 1}], + } + ], + } + + events = capture_events() + + def do_POST(self): # noqa: N802 + self.send_response(200) + self.end_headers() + self.wfile.write(json.dumps(graphql_response).encode()) + + with mock.patch.object(MockServerRequestHandler, "do_POST", do_POST): + conn = HTTPConnection("localhost:{}".format(PORT)) + conn.request("POST", "/graphql", body=json.dumps(graphql_request).encode()) + response = conn.getresponse() + + # make sure the response can still be read() normally + assert response.read() == json.dumps(graphql_response).encode() + + assert not events From 4ba7a867073e3221afa7a88b830943e45f45cd43 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Mon, 17 Jul 2023 15:40:17 +0200 Subject: [PATCH 36/39] more tests --- tests/integrations/aiohttp/test_aiohttp.py | 67 +++++++++++++ tests/integrations/httpx/test_httpx.py | 77 +++++++++++++++ tests/integrations/stdlib/test_httplib.py | 106 +++++++++++++++++++++ 3 files changed, 250 insertions(+) diff --git a/tests/integrations/aiohttp/test_aiohttp.py b/tests/integrations/aiohttp/test_aiohttp.py index 755997842a..0d6f29b1fa 100644 --- a/tests/integrations/aiohttp/test_aiohttp.py +++ b/tests/integrations/aiohttp/test_aiohttp.py @@ -672,6 +672,73 @@ async def handler(request): ) +@pytest.mark.asyncio +async def test_graphql_get_client_no_errors_returned( + sentry_init, capture_events, aiohttp_raw_server, aiohttp_client +): + sentry_init(send_default_pii=True, integrations=[AioHttpIntegration()]) + + graphql_response = { + "data": None, + } + + async def handler(request): + return json_response(graphql_response) + + raw_server = await aiohttp_raw_server(handler) + events = capture_events() + + client = await aiohttp_client(raw_server) + response = await client.get( + "/graphql", params={"query": "query GetPet {pet{name}}"} + ) + + assert response.status == 200 + assert await response.json() == graphql_response + + assert not events + + +@pytest.mark.asyncio +async def test_graphql_post_client_no_errors_returned( + sentry_init, capture_events, aiohttp_client, aiohttp_raw_server +): + sentry_init(send_default_pii=True, integrations=[AioHttpIntegration()]) + + graphql_request = { + "query": dedent( + """ + mutation AddPet ($name: String!) { + addPet(name: $name) { + id + name + } + } + """ + ), + "variables": { + "name": "Lucy", + }, + } + graphql_response = { + "data": None, + } + + async def handler(request): + return json_response(graphql_response) + + raw_server = await aiohttp_raw_server(handler) + events = capture_events() + + client = await aiohttp_client(raw_server) + response = await client.post("/graphql", json=graphql_request) + + assert response.status == 200 + assert await response.json() == graphql_response + + assert not events + + @pytest.mark.asyncio async def test_graphql_no_get_errors_if_option_is_off( sentry_init, capture_events, aiohttp_raw_server, aiohttp_client diff --git a/tests/integrations/httpx/test_httpx.py b/tests/integrations/httpx/test_httpx.py index 25819ae0d0..bba75f7e3a 100644 --- a/tests/integrations/httpx/test_httpx.py +++ b/tests/integrations/httpx/test_httpx.py @@ -422,6 +422,83 @@ def test_graphql_post_client_error_captured( ) +@pytest.mark.parametrize( + "httpx_client", + (httpx.Client(), httpx.AsyncClient()), +) +def test_graphql_get_client_no_errors_returned( + sentry_init, capture_events, httpx_client, httpx_mock +): + sentry_init(send_default_pii=True, integrations=[HttpxIntegration()]) + + url = "http://example.com/graphql" + graphql_response = { + "data": None, + } + params = {"query": "query QueryName {user{name}}"} + + httpx_mock.add_response(method="GET", json=graphql_response) + + events = capture_events() + + if asyncio.iscoroutinefunction(httpx_client.get): + response = asyncio.get_event_loop().run_until_complete( + httpx_client.get(url, params=params) + ) + else: + response = httpx_client.get(url, params=params) + + assert response.status_code == 200 + assert response.json() == graphql_response + + assert not events + + +@pytest.mark.parametrize( + "httpx_client", + (httpx.Client(), httpx.AsyncClient()), +) +def test_graphql_post_client_no_errors_returned( + sentry_init, capture_events, httpx_client, httpx_mock +): + sentry_init(send_default_pii=True, integrations=[HttpxIntegration()]) + + url = "http://example.com/graphql" + graphql_request = { + "query": dedent( + """ + mutation AddPet ($name: String!) { + addPet(name: $name) { + id + name + } + } + """ + ), + "variables": { + "name": "Lucy", + }, + } + graphql_response = { + "data": None, + } + httpx_mock.add_response(method="POST", json=graphql_response) + + events = capture_events() + + if asyncio.iscoroutinefunction(httpx_client.post): + response = asyncio.get_event_loop().run_until_complete( + httpx_client.post(url, json=graphql_request) + ) + else: + response = httpx_client.post(url, json=graphql_request) + + assert response.status_code == 200 + assert response.json() == graphql_response + + assert not events + + @pytest.mark.parametrize( "httpx_client", (httpx.Client(), httpx.AsyncClient()), diff --git a/tests/integrations/stdlib/test_httplib.py b/tests/integrations/stdlib/test_httplib.py index 10897e1e36..39efe3d22f 100644 --- a/tests/integrations/stdlib/test_httplib.py +++ b/tests/integrations/stdlib/test_httplib.py @@ -458,6 +458,72 @@ def do_POST(self): # noqa: N802 ) +def test_graphql_get_client_no_errors_returned(sentry_init, capture_events): + sentry_init(send_default_pii=True, integrations=[StdlibIntegration()]) + + params = {"query": "query QueryName {user{name}}"} + graphql_response = { + "data": None, + } + + events = capture_events() + + def do_GET(self): # noqa: N802 + self.send_response(200) + self.end_headers() + self.wfile.write(json.dumps(graphql_response).encode()) + + with mock.patch.object(MockServerRequestHandler, "do_GET", do_GET): + conn = HTTPConnection("localhost:{}".format(PORT)) + conn.request("GET", "/graphql?" + urlencode(params)) + response = conn.getresponse() + + # make sure the response can still be read() normally + assert response.read() == json.dumps(graphql_response).encode() + + assert not events + + +def test_graphql_post_client_no_errors_returned(sentry_init, capture_events): + sentry_init(send_default_pii=True, integrations=[StdlibIntegration()]) + + graphql_request = { + "query": dedent( + """ + mutation AddPet ($name: String!) { + addPet(name: $name) { + id + name + } + } + """ + ), + "variables": { + "name": "Lucy", + }, + } + graphql_response = { + "data": None, + } + + events = capture_events() + + def do_POST(self): # noqa: N802 + self.send_response(200) + self.end_headers() + self.wfile.write(json.dumps(graphql_response).encode()) + + with mock.patch.object(MockServerRequestHandler, "do_POST", do_POST): + conn = HTTPConnection("localhost:{}".format(PORT)) + conn.request("POST", "/graphql", body=json.dumps(graphql_request).encode()) + response = conn.getresponse() + + # make sure the response can still be read() normally + assert response.read() == json.dumps(graphql_response).encode() + + assert not events + + def test_graphql_no_get_errors_if_option_is_off(sentry_init, capture_events): sentry_init( send_default_pii=True, @@ -541,3 +607,43 @@ def do_POST(self): # noqa: N802 assert response.read() == json.dumps(graphql_response).encode() assert not events + + +def test_graphql_non_json_response(sentry_init, capture_events): + sentry_init( + send_default_pii=True, + integrations=[StdlibIntegration()], + ) + + graphql_request = { + "query": dedent( + """ + mutation AddPet ($name: String!) { + addPet(name: $name) { + id + name + } + } + """ + ), + "variables": { + "name": "Lucy", + }, + } + + events = capture_events() + + def do_POST(self): # noqa: N802 + self.send_response(200) + self.end_headers() + self.wfile.write(b"not json") + + with mock.patch.object(MockServerRequestHandler, "do_POST", do_POST): + conn = HTTPConnection("localhost:{}".format(PORT)) + conn.request("POST", "/graphql", body=json.dumps(graphql_request).encode()) + response = conn.getresponse() + + # make sure the response can still be read() normally + assert response.read() == b"not json" + + assert not events From 9de27bb4c891392dcbc6e33ba3a68a6257adcdd0 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Mon, 17 Jul 2023 15:53:46 +0200 Subject: [PATCH 37/39] more tests --- sentry_sdk/integrations/aiohttp.py | 58 ++++++++++++---------- sentry_sdk/integrations/httpx.py | 6 ++- tests/integrations/aiohttp/test_aiohttp.py | 42 +++++++++++++++- tests/integrations/httpx/test_httpx.py | 45 +++++++++++++++++ 4 files changed, 122 insertions(+), 29 deletions(-) diff --git a/sentry_sdk/integrations/aiohttp.py b/sentry_sdk/integrations/aiohttp.py index 5cbcc876ac..4174171a9a 100644 --- a/sentry_sdk/integrations/aiohttp.py +++ b/sentry_sdk/integrations/aiohttp.py @@ -44,7 +44,7 @@ import asyncio from aiohttp import __version__ as AIOHTTP_VERSION - from aiohttp import ClientSession, TraceConfig + from aiohttp import ClientSession, ContentTypeError, TraceConfig from aiohttp.web import Application, HTTPException, UrlDispatcher, Response except ImportError: raise DidNotEnable("AIOHTTP not installed") @@ -291,34 +291,38 @@ async def on_request_end(session, trace_config_ctx, params): ): with hub.configure_scope() as scope: with capture_internal_exceptions(): - response_content = await response.json() - scope.add_event_processor( - _make_client_processor( - trace_config_ctx=trace_config_ctx, - response=response, - response_content=response_content, + try: + response_content = await response.json() + except ContentTypeError: + pass + else: + scope.add_event_processor( + _make_client_processor( + trace_config_ctx=trace_config_ctx, + response=response, + response_content=response_content, + ) ) - ) - if ( - response_content - and isinstance(response_content, dict) - and response_content.get("errors") - ): - try: - raise SentryGraphQLClientError - except SentryGraphQLClientError as ex: - event, hint = event_from_exception( - ex, - client_options=hub.client.options - if hub.client - else None, - mechanism={ - "type": AioHttpIntegration.identifier, - "handled": False, - }, - ) - hub.capture_event(event, hint=hint) + if ( + response_content + and isinstance(response_content, dict) + and response_content.get("errors") + ): + try: + raise SentryGraphQLClientError + except SentryGraphQLClientError as ex: + event, hint = event_from_exception( + ex, + client_options=hub.client.options + if hub.client + else None, + mechanism={ + "type": AioHttpIntegration.identifier, + "handled": False, + }, + ) + hub.capture_event(event, hint=hint) if trace_config_ctx.span is not None: span.finish() diff --git a/sentry_sdk/integrations/httpx.py b/sentry_sdk/integrations/httpx.py index 9f0de42ea2..0834d46d5f 100644 --- a/sentry_sdk/integrations/httpx.py +++ b/sentry_sdk/integrations/httpx.py @@ -241,7 +241,11 @@ def _capture_graphql_errors(hub, request, response): scope.add_event_processor(_make_request_processor(request, response)) with capture_internal_exceptions(): - response_content = response.json() + try: + response_content = response.json() + except JSONDecodeError: + return + if isinstance(response_content, dict) and response_content.get( "errors" ): diff --git a/tests/integrations/aiohttp/test_aiohttp.py b/tests/integrations/aiohttp/test_aiohttp.py index 0d6f29b1fa..79ed402554 100644 --- a/tests/integrations/aiohttp/test_aiohttp.py +++ b/tests/integrations/aiohttp/test_aiohttp.py @@ -6,7 +6,7 @@ import pytest from aiohttp import web from aiohttp.client import ServerDisconnectedError -from aiohttp.web import Request, json_response +from aiohttp.web import Request, Response, json_response from sentry_sdk import capture_message, start_transaction from sentry_sdk.integrations.aiohttp import AioHttpIntegration @@ -823,3 +823,43 @@ async def handler(request): assert await response.json() == graphql_response assert not events + + +@pytest.mark.asyncio +async def test_graphql_non_json_response( + sentry_init, capture_events, aiohttp_client, aiohttp_raw_server +): + sentry_init( + send_default_pii=True, + integrations=[AioHttpIntegration()], + ) + + graphql_request = { + "query": dedent( + """ + mutation AddPet ($name: String!) { + addPet(name: $name) { + id + name + } + } + """ + ), + "variables": { + "name": "Lucy", + }, + } + + async def handler(request): + return Response(body=b"not json") + + raw_server = await aiohttp_raw_server(handler) + events = capture_events() + + client = await aiohttp_client(raw_server) + response = await client.post("/graphql", json=graphql_request) + + assert response.status == 200 + assert await response.text() == "not json" + + assert not events diff --git a/tests/integrations/httpx/test_httpx.py b/tests/integrations/httpx/test_httpx.py index bba75f7e3a..ab2a5cce96 100644 --- a/tests/integrations/httpx/test_httpx.py +++ b/tests/integrations/httpx/test_httpx.py @@ -593,3 +593,48 @@ def test_graphql_no_post_errors_if_option_is_off( assert response.json() == graphql_response assert not events + + +@pytest.mark.parametrize( + "httpx_client", + (httpx.Client(), httpx.AsyncClient()), +) +def test_graphql_non_json_response( + sentry_init, capture_events, httpx_client, httpx_mock +): + sentry_init( + send_default_pii=True, + integrations=[HttpxIntegration()], + ) + + url = "http://example.com/graphql" + graphql_request = { + "query": dedent( + """ + mutation AddPet ($name: String!) { + addPet(name: $name) { + id + name + } + } + """ + ), + "variables": { + "name": "Lucy", + }, + } + httpx_mock.add_response(method="POST", text="not json") + + events = capture_events() + + if asyncio.iscoroutinefunction(httpx_client.post): + response = asyncio.get_event_loop().run_until_complete( + httpx_client.post(url, json=graphql_request) + ) + else: + response = httpx_client.post(url, json=graphql_request) + + assert response.text == "not json" + assert response.status_code == 200 + + assert not events From 64964528e9a588bd4c08c9e1c946e1d841d1fd83 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Mon, 17 Jul 2023 16:13:31 +0200 Subject: [PATCH 38/39] compat --- tests/integrations/httpx/test_httpx.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integrations/httpx/test_httpx.py b/tests/integrations/httpx/test_httpx.py index ab2a5cce96..50185b9603 100644 --- a/tests/integrations/httpx/test_httpx.py +++ b/tests/integrations/httpx/test_httpx.py @@ -623,7 +623,7 @@ def test_graphql_non_json_response( "name": "Lucy", }, } - httpx_mock.add_response(method="POST", text="not json") + httpx_mock.add_response(method="POST", content=b"not json") events = capture_events() From c58393d305674526470b14871367c556a5102139 Mon Sep 17 00:00:00 2001 From: Ivana Kellyerova Date: Mon, 17 Jul 2023 16:43:14 +0200 Subject: [PATCH 39/39] compat with old pytest-httpx --- tests/integrations/httpx/test_httpx.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/integrations/httpx/test_httpx.py b/tests/integrations/httpx/test_httpx.py index 50185b9603..8bae3ee3c4 100644 --- a/tests/integrations/httpx/test_httpx.py +++ b/tests/integrations/httpx/test_httpx.py @@ -623,7 +623,7 @@ def test_graphql_non_json_response( "name": "Lucy", }, } - httpx_mock.add_response(method="POST", content=b"not json") + httpx_mock.add_response(method="POST") events = capture_events() @@ -634,7 +634,6 @@ def test_graphql_non_json_response( else: response = httpx_client.post(url, json=graphql_request) - assert response.text == "not json" assert response.status_code == 200 assert not events