diff --git a/sentry_sdk/_types.py b/sentry_sdk/_types.py index b28c7260ce..dc380d19f8 100644 --- a/sentry_sdk/_types.py +++ b/sentry_sdk/_types.py @@ -76,6 +76,22 @@ def removed_because_over_size_limit(cls, value=""): }, ) + @classmethod + def removed_because_body_consumed_and_not_cached(cls, value=""): + # type: (Any) -> AnnotatedValue + """The actual value was removed because the underlying stream has been consumed, without caching the value.""" + return AnnotatedValue( + value=value, + metadata={ + "rem": [ # Remark + [ + "!consumed", # Because the original stream is unavailable + "x", # The fields original value was removed + ] + ] + }, + ) + @classmethod def substituted_because_contains_sensitive_data(cls): # type: () -> AnnotatedValue diff --git a/sentry_sdk/integrations/fastapi.py b/sentry_sdk/integrations/fastapi.py index 1473cbcab7..2235199d5c 100644 --- a/sentry_sdk/integrations/fastapi.py +++ b/sentry_sdk/integrations/fastapi.py @@ -11,13 +11,14 @@ from typing import TYPE_CHECKING if TYPE_CHECKING: - from typing import Any, Callable, Dict + from typing import Any, Callable, Dict, Optional from sentry_sdk._types import Event try: from sentry_sdk.integrations.starlette import ( StarletteIntegration, StarletteRequestExtractor, + _patch_request, ) except DidNotEnable: raise DidNotEnable("Starlette is not installed") @@ -103,38 +104,54 @@ async def _sentry_app(*args, **kwargs): return await old_app(*args, **kwargs) request = args[0] + _patch_request(request) _set_transaction_name_and_source( sentry_sdk.get_current_scope(), integration.transaction_style, request ) sentry_scope = sentry_sdk.get_isolation_scope() - extractor = StarletteRequestExtractor(request) - info = await extractor.extract_request_info() + sentry_scope._name = FastApiIntegration.identifier - def _make_request_event_processor(req, integration): - # type: (Any, Any) -> Callable[[Event, Dict[str, Any]], Event] + def _make_cookies_event_processor(cookies): + # type: (Optional[Dict[str, Any]]) -> Callable[[Event, Dict[str, Any]], Event] def event_processor(event, hint): # type: (Event, Dict[str, Any]) -> Event + if cookies and should_send_default_pii(): + event.setdefault("request", {})["cookies"] = deepcopy(cookies) + + return event + + return event_processor - # Extract information from request - request_info = event.get("request", {}) - if info: - if "cookies" in info and should_send_default_pii(): - request_info["cookies"] = info["cookies"] - if "data" in info: - request_info["data"] = info["data"] - event["request"] = deepcopy(request_info) + def _make_request_body_event_processor(info): + # type: (Optional[Dict[str, Any]]) -> Callable[[Event, Dict[str, Any]], Event] + def event_processor(event, hint): + # type: (Event, Dict[str, Any]) -> Event + if info and "data" in info: + event.setdefault("request", {})["data"] = deepcopy(info["data"]) return event return event_processor - sentry_scope._name = FastApiIntegration.identifier - sentry_scope.add_event_processor( - _make_request_event_processor(request, integration) - ) + extractor = StarletteRequestExtractor(request) + cookies = extractor.extract_cookies_from_request() + sentry_scope.add_event_processor(_make_cookies_event_processor(cookies)) + + try: + response = await old_app(*args, **kwargs) + except Exception as exception: + info = await extractor.extract_request_info() + sentry_scope.add_event_processor( + _make_request_body_event_processor(info) + ) + + raise exception + + info = await extractor.extract_request_info() + sentry_scope.add_event_processor(_make_request_body_event_processor(info)) - return await old_app(*args, **kwargs) + return response return _sentry_app diff --git a/sentry_sdk/integrations/graphene.py b/sentry_sdk/integrations/graphene.py index 00a8d155d4..00c474a276 100644 --- a/sentry_sdk/integrations/graphene.py +++ b/sentry_sdk/integrations/graphene.py @@ -20,7 +20,7 @@ if TYPE_CHECKING: from collections.abc import Generator - from typing import Any, Dict, Union + from typing import Any, Dict, Union, Callable from graphene.language.source import Source # type: ignore from graphql.execution import ExecutionResult from graphql.type import GraphQLSchema @@ -48,7 +48,7 @@ def _patch_graphql(): def _sentry_patched_graphql_sync(schema, source, *args, **kwargs): # type: (GraphQLSchema, Union[str, Source], Any, Any) -> ExecutionResult scope = sentry_sdk.get_isolation_scope() - scope.add_event_processor(_event_processor) + scope.add_event_processor(_make_event_processor(source)) with graphql_span(schema, source, kwargs): result = old_graphql_sync(schema, source, *args, **kwargs) @@ -75,7 +75,7 @@ async def _sentry_patched_graphql_async(schema, source, *args, **kwargs): return await old_graphql_async(schema, source, *args, **kwargs) scope = sentry_sdk.get_isolation_scope() - scope.add_event_processor(_event_processor) + scope.add_event_processor(_make_event_processor(source)) with graphql_span(schema, source, kwargs): result = await old_graphql_async(schema, source, *args, **kwargs) @@ -99,16 +99,22 @@ async def _sentry_patched_graphql_async(schema, source, *args, **kwargs): graphene_schema.graphql = _sentry_patched_graphql_async -def _event_processor(event, hint): - # type: (Event, Dict[str, Any]) -> Event - if should_send_default_pii(): - request_info = event.setdefault("request", {}) - request_info["api_target"] = "graphql" +def _make_event_processor(source): + # type: (Any) -> Callable[[Event, dict[str, Any]], Event] + def _event_processor(event, hint): + # type: (Event, Dict[str, Any]) -> Event + if should_send_default_pii(): + request_info = event.setdefault("request", {}) + request_info["api_target"] = "graphql" + if isinstance(source, str): + request_info.setdefault("data", {})["query"] = source # type: ignore - elif event.get("request", {}).get("data"): - del event["request"]["data"] + elif event.get("request", {}).get("data"): + del event["request"]["data"] - return event + return event + + return _event_processor @contextmanager diff --git a/sentry_sdk/integrations/starlette.py b/sentry_sdk/integrations/starlette.py index f1a0e360bb..029b5b8a09 100644 --- a/sentry_sdk/integrations/starlette.py +++ b/sentry_sdk/integrations/starlette.py @@ -36,15 +36,26 @@ from typing import TYPE_CHECKING if TYPE_CHECKING: - from typing import Any, Awaitable, Callable, Container, Dict, Optional, Tuple, Union - + from typing import ( + Any, + Awaitable, + Callable, + Container, + Dict, + Optional, + Tuple, + Union, + Protocol, + TypeVar, + ) + from types import CoroutineType from sentry_sdk._types import Event, HttpStatusCodeRange try: import starlette # type: ignore from starlette import __version__ as STARLETTE_VERSION from starlette.applications import Starlette # type: ignore - from starlette.datastructures import UploadFile # type: ignore + from starlette.datastructures import UploadFile, FormData # type: ignore from starlette.middleware import Middleware # type: ignore from starlette.middleware.authentication import ( # type: ignore AuthenticationMiddleware, @@ -55,6 +66,16 @@ except ImportError: raise DidNotEnable("Starlette is not installed") +if TYPE_CHECKING: + from contextlib import AbstractAsyncContextManager + + T_co = TypeVar("T_co", covariant=True) + + class AwaitableOrContextManager( + Awaitable[T_co], AbstractAsyncContextManager[T_co], Protocol[T_co] + ): ... + + try: # Starlette 0.20 from starlette.middleware.exceptions import ExceptionMiddleware # type: ignore @@ -420,6 +441,35 @@ def _is_async_callable(obj): ) +def _patch_request(request): + # type: (Request) -> None + _original_body = request.body + _original_json = request.json + _original_form = request.form + + @functools.wraps(_original_body) + def sentry_body(): + # type: () -> CoroutineType[Any, Any, bytes] + request.scope.setdefault("state", {})["sentry_sdk.is_body_cached"] = True + return _original_body() + + @functools.wraps(_original_json) + def sentry_json(): + # type: () -> CoroutineType[Any, Any, Any] + request.scope.setdefault("state", {})["sentry_sdk.is_body_cached"] = True + return _original_json() + + @functools.wraps(_original_form) + def sentry_form(*args, **kwargs): + # type: (*Any, **Any) -> AwaitableOrContextManager[FormData] + request.scope.setdefault("state", {})["sentry_sdk.is_body_cached"] = True + return _original_form(*args, **kwargs) + + request.body = sentry_body + request.json = sentry_json + request.form = sentry_form + + def patch_request_response(): # type: () -> None old_request_response = starlette.routing.request_response @@ -440,6 +490,7 @@ async def _sentry_async_func(*args, **kwargs): return await old_func(*args, **kwargs) request = args[0] + _patch_request(request) _set_transaction_name_and_source( sentry_sdk.get_current_scope(), @@ -448,33 +499,54 @@ async def _sentry_async_func(*args, **kwargs): ) sentry_scope = sentry_sdk.get_isolation_scope() - extractor = StarletteRequestExtractor(request) - info = await extractor.extract_request_info() + sentry_scope._name = StarletteIntegration.identifier - def _make_request_event_processor(req, integration): - # type: (Any, Any) -> Callable[[Event, dict[str, Any]], Event] + def _make_cookies_event_processor(cookies): + # type: (Optional[Dict[str, Any]]) -> Callable[[Event, Dict[str, Any]], Event] def event_processor(event, hint): # type: (Event, Dict[str, Any]) -> Event + if cookies and should_send_default_pii(): + event.setdefault("request", {})["cookies"] = deepcopy( + cookies + ) - # Add info from request to event - request_info = event.get("request", {}) - if info: - if "cookies" in info: - request_info["cookies"] = info["cookies"] - if "data" in info: - request_info["data"] = info["data"] - event["request"] = deepcopy(request_info) + return event + + return event_processor + + def _make_request_body_event_processor(info): + # type: (Optional[Dict[str, Any]]) -> Callable[[Event, Dict[str, Any]], Event] + def event_processor(event, hint): + # type: (Event, Dict[str, Any]) -> Event + if info and "data" in info: + event.setdefault("request", {})["data"] = deepcopy( + info["data"] + ) return event return event_processor - sentry_scope._name = StarletteIntegration.identifier + extractor = StarletteRequestExtractor(request) + cookies = extractor.extract_cookies_from_request() + sentry_scope.add_event_processor(_make_cookies_event_processor(cookies)) + + try: + response = await old_func(*args, **kwargs) + except Exception as exception: + info = await extractor.extract_request_info() + sentry_scope.add_event_processor( + _make_request_body_event_processor(info) + ) + + raise exception + + info = await extractor.extract_request_info() sentry_scope.add_event_processor( - _make_request_event_processor(request, integration) + _make_request_body_event_processor(info) ) - return await old_func(*args, **kwargs) + return response func = _sentry_async_func @@ -621,6 +693,18 @@ async def extract_request_info(self): request_info["data"] = AnnotatedValue.removed_because_over_size_limit() return request_info + # Avoid hangs by not parsing body when ASGI stream is consumed + is_body_cached = ( + "state" in self.request.scope + and "sentry_sdk.is_body_cached" in self.request.scope["state"] + and self.request.scope["state"]["sentry_sdk.is_body_cached"] + ) + if not is_body_cached: + request_info["data"] = ( + AnnotatedValue.removed_because_body_consumed_and_not_cached() + ) + return request_info + # Add JSON body, if it is a JSON request json = await self.json() if json: diff --git a/tests/integrations/fastapi/test_fastapi.py b/tests/integrations/fastapi/test_fastapi.py index a69978ded4..30d10440b7 100644 --- a/tests/integrations/fastapi/test_fastapi.py +++ b/tests/integrations/fastapi/test_fastapi.py @@ -1,3 +1,4 @@ +import asyncio import json import logging import pytest @@ -11,7 +12,7 @@ from fastapi.middleware.trustedhost import TrustedHostMiddleware import sentry_sdk -from sentry_sdk import capture_message +from sentry_sdk import capture_message, capture_exception from sentry_sdk.feature_flags import add_feature_flag from sentry_sdk.integrations.asgi import SentryAsgiMiddleware from sentry_sdk.integrations.fastapi import FastApiIntegration @@ -24,6 +25,8 @@ from tests.integrations.conftest import parametrize_test_configurable_status_codes from tests.integrations.starlette import test_starlette +BODY_JSON = {"some": "json", "for": "testing", "nested": {"numbers": 123}} + def fastapi_app_factory(): app = FastAPI() @@ -220,9 +223,121 @@ def test_active_thread_id(sentry_init, capture_envelopes, teardown_profiling, en assert str(data["active"]) == trace_context["data"]["thread.id"] +def test_cookies_available(sentry_init, capture_events): + sentry_init( + integrations=[StarletteIntegration(), FastApiIntegration()], + send_default_pii=True, + ) + + app = FastAPI() + + @app.post("/exception") + async def _exception(request: Request): + logging.critical("Oh no!") + return {"error": "Oh no!"} + + events = capture_events() + + client = TestClient(app) + + try: + client.post( + "/exception", + json=BODY_JSON, + cookies={ + "tasty_cookie": "strawberry", + "yummy_cookie": "choco", + }, + ) + except ZeroDivisionError: + capture_exception() + + event = events[0] + assert event["request"]["cookies"] == { + "tasty_cookie": "strawberry", + "yummy_cookie": "choco", + } + + +def test_request_body_not_cached_with_exception(sentry_init, capture_events): + sentry_init( + integrations=[StarletteIntegration(), FastApiIntegration()], + send_default_pii=True, + ) + + app = FastAPI() + + @app.post("/exception") + async def _exception(request: Request): + 1 / 0 + return {"error": "Oh no!"} + + events = capture_events() + + client = TestClient(app) + + try: + client.post( + "/exception", + json=BODY_JSON, + cookies={ + "tasty_cookie": "strawberry", + "yummy_cookie": "choco", + }, + ) + except ZeroDivisionError: + capture_exception() + + event = events[0] + assert event["request"]["cookies"] == { + "tasty_cookie": "strawberry", + "yummy_cookie": "choco", + } + assert event["request"]["data"] == "" + + +def test_request_body_cached_with_exception(sentry_init, capture_events): + sentry_init( + integrations=[StarletteIntegration(), FastApiIntegration()], + send_default_pii=True, + ) + + app = FastAPI() + + @app.post("/exception") + async def _exception(request: Request): + await request.json() + 1 / 0 + return {"error": "Oh no!"} + + events = capture_events() + + client = TestClient(app) + + try: + client.post( + "/exception", + json=BODY_JSON, + cookies={ + "tasty_cookie": "strawberry", + "yummy_cookie": "choco", + }, + ) + except ZeroDivisionError: + capture_exception() + + event = events[0] + assert event["request"]["cookies"] == { + "tasty_cookie": "strawberry", + "yummy_cookie": "choco", + } + assert event["request"]["data"] == BODY_JSON + + @pytest.mark.asyncio async def test_original_request_not_scrubbed(sentry_init, capture_events): sentry_init( + default_integrations=False, integrations=[StarletteIntegration(), FastApiIntegration()], traces_sample_rate=1.0, ) diff --git a/tests/integrations/starlette/test_starlette.py b/tests/integrations/starlette/test_starlette.py index bc445bf8f2..554e78e980 100644 --- a/tests/integrations/starlette/test_starlette.py +++ b/tests/integrations/starlette/test_starlette.py @@ -11,7 +11,7 @@ import pytest -from sentry_sdk import capture_message, get_baggage, get_traceparent +from sentry_sdk import capture_message, get_baggage, get_traceparent, capture_exception from sentry_sdk.integrations.asgi import SentryAsgiMiddleware from sentry_sdk.integrations.starlette import ( StarletteIntegration, @@ -26,6 +26,7 @@ AuthenticationError, SimpleUser, ) +from starlette.requests import Request from starlette.exceptions import HTTPException from starlette.middleware import Middleware from starlette.middleware.authentication import AuthenticationMiddleware @@ -435,6 +436,7 @@ async def test_starletterequestextractor_extract_request_info(sentry_init): side_effect = [_mock_receive(msg) for msg in JSON_RECEIVE_MESSAGES] starlette_request._receive = mock.Mock(side_effect=side_effect) + starlette_request.scope["state"] = {"sentry_sdk.is_body_cached": True} extractor = StarletteRequestExtractor(starlette_request) request_info = await extractor.extract_request_info() @@ -447,6 +449,37 @@ async def test_starletterequestextractor_extract_request_info(sentry_init): assert request_info["data"] == BODY_JSON +@pytest.mark.asyncio +async def test_starletterequestextractor_extract_request_info_not_cached(sentry_init): + sentry_init( + send_default_pii=True, + integrations=[StarletteIntegration()], + ) + scope = SCOPE.copy() + scope["headers"] = [ + [b"content-type", b"application/json"], + [b"content-length", str(len(json.dumps(BODY_JSON))).encode()], + [b"cookie", b"yummy_cookie=choco; tasty_cookie=strawberry"], + ] + + starlette_request = starlette.requests.Request(scope) + + # Mocking async `_receive()` that works in Python 3.7+ + side_effect = [_mock_receive(msg) for msg in JSON_RECEIVE_MESSAGES] + starlette_request._receive = mock.Mock(side_effect=side_effect) + + extractor = StarletteRequestExtractor(starlette_request) + + request_info = await extractor.extract_request_info() + + assert request_info + assert request_info["cookies"] == { + "tasty_cookie": "strawberry", + "yummy_cookie": "choco", + } + assert request_info["data"].metadata == {"rem": [["!consumed", "x"]]} + + @pytest.mark.asyncio async def test_starletterequestextractor_extract_request_info_no_pii(sentry_init): sentry_init( @@ -466,6 +499,7 @@ async def test_starletterequestextractor_extract_request_info_no_pii(sentry_init side_effect = [_mock_receive(msg) for msg in JSON_RECEIVE_MESSAGES] starlette_request._receive = mock.Mock(side_effect=side_effect) + starlette_request.scope["state"] = {"sentry_sdk.is_body_cached": True} extractor = StarletteRequestExtractor(starlette_request) request_info = await extractor.extract_request_info() @@ -941,8 +975,129 @@ def test_active_thread_id(sentry_init, capture_envelopes, teardown_profiling, en assert str(data["active"]) == trace_context["data"]["thread.id"] +def test_cookies_available(sentry_init, capture_events): + sentry_init( + integrations=[StarletteIntegration()], + send_default_pii=True, + ) + + events = capture_events() + + async def _exception(request: Request): + logging.critical("Oh no!") + return starlette.responses.JSONResponse({"status": "Oh no!"}) + + app = starlette.applications.Starlette( + routes=[ + starlette.routing.Route("/exception", _exception, methods=["POST"]), + ], + ) + + client = TestClient(app) + + client.post( + "/exception", + json=BODY_JSON, + cookies={ + "tasty_cookie": "strawberry", + "yummy_cookie": "choco", + }, + ) + + event = events[0] + assert event["request"]["cookies"] == { + "tasty_cookie": "strawberry", + "yummy_cookie": "choco", + } + + +def test_request_body_not_cached_exception(sentry_init, capture_events): + sentry_init( + integrations=[StarletteIntegration()], + send_default_pii=True, + ) + + events = capture_events() + + async def _exception(request): + 1 / 0 + return starlette.responses.JSONResponse({"status": "Oh no!"}) + + app = starlette.applications.Starlette( + routes=[ + starlette.routing.Route("/exception", _exception, methods=["POST"]), + ], + ) + + client = TestClient(app) + + try: + client.post( + "/exception", + json=BODY_JSON, + cookies={ + "tasty_cookie": "strawberry", + "yummy_cookie": "choco", + }, + ) + except ZeroDivisionError: + capture_exception() + + event = events[0] + assert event["request"]["cookies"] == { + "tasty_cookie": "strawberry", + "yummy_cookie": "choco", + } + assert event["request"]["data"] == "" + + +def test_request_body_cached_exception(sentry_init, capture_events): + sentry_init( + integrations=[StarletteIntegration()], + send_default_pii=True, + ) + + events = capture_events() + + async def _exception(request): + await request.json() + 1 / 0 + return starlette.responses.JSONResponse({"status": "Oh no!"}) + + app = starlette.applications.Starlette( + routes=[ + starlette.routing.Route("/exception", _exception, methods=["POST"]), + ], + ) + + client = TestClient(app) + + try: + client.post( + "/exception", + json=BODY_JSON, + cookies={ + "tasty_cookie": "strawberry", + "yummy_cookie": "choco", + }, + ) + except ZeroDivisionError: + capture_exception() + + event = events[0] + assert event["request"]["cookies"] == { + "tasty_cookie": "strawberry", + "yummy_cookie": "choco", + } + assert event["request"]["data"] == BODY_JSON + + def test_original_request_not_scrubbed(sentry_init, capture_events): - sentry_init(integrations=[StarletteIntegration()]) + sentry_init( + default_integrations=False, + integrations=[StarletteIntegration()], + traces_sample_rate=1.0, + ) events = capture_events()