From 0ee7c5076828ec6e0ed484ccfcc4d0d28e81c5ad Mon Sep 17 00:00:00 2001 From: Ivana Kellyer Date: Mon, 23 Sep 2024 09:52:05 +0200 Subject: [PATCH 1/3] fix(django): Don't let RawPostDataException bubble up (#3553) --- sentry_sdk/integrations/_wsgi_common.py | 8 ++++++- tests/integrations/django/test_basic.py | 28 ++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/sentry_sdk/integrations/_wsgi_common.py b/sentry_sdk/integrations/_wsgi_common.py index 14a4c4aea4..c4f3f1c77e 100644 --- a/sentry_sdk/integrations/_wsgi_common.py +++ b/sentry_sdk/integrations/_wsgi_common.py @@ -152,7 +152,13 @@ def json(self): if not self.is_json(): return None - raw_data = self.raw_data() + try: + raw_data = self.raw_data() + except (RawPostDataException, ValueError): + # The body might have already been read, in which case this will + # fail + raw_data = None + if raw_data is None: return None diff --git a/tests/integrations/django/test_basic.py b/tests/integrations/django/test_basic.py index 45c25595f3..f02f8ee217 100644 --- a/tests/integrations/django/test_basic.py +++ b/tests/integrations/django/test_basic.py @@ -3,6 +3,7 @@ import re import pytest from functools import partial +from unittest.mock import patch from werkzeug.test import Client @@ -10,6 +11,7 @@ from django.contrib.auth.models import User from django.core.management import execute_from_command_line from django.db.utils import OperationalError, ProgrammingError, DataError +from django.http.request import RawPostDataException try: from django.urls import reverse @@ -20,7 +22,11 @@ from sentry_sdk._compat import PY310 from sentry_sdk import capture_message, capture_exception from sentry_sdk.consts import SPANDATA -from sentry_sdk.integrations.django import DjangoIntegration, _set_db_data +from sentry_sdk.integrations.django import ( + DjangoIntegration, + DjangoRequestExtractor, + _set_db_data, +) from sentry_sdk.integrations.django.signals_handlers import _get_receiver_name from sentry_sdk.integrations.executing import ExecutingIntegration from sentry_sdk.tracing import Span @@ -740,6 +746,26 @@ def test_read_request(sentry_init, client, capture_events): assert "data" not in event["request"] +def test_request_body_already_read(sentry_init, client, capture_events): + sentry_init(integrations=[DjangoIntegration()]) + + events = capture_events() + + class MockExtractor(DjangoRequestExtractor): + def raw_data(self): + raise RawPostDataException + + with patch("sentry_sdk.integrations.django.DjangoRequestExtractor", MockExtractor): + client.post( + reverse("post_echo"), data=b'{"hey": 42}', content_type="application/json" + ) + + (event,) = events + + assert event["message"] == "hi" + assert "data" not in event["request"] + + def test_template_tracing_meta(sentry_init, client, capture_events): sentry_init(integrations=[DjangoIntegration()]) events = capture_events() From 25ab10cdbc556e949b37daf95c77711604bfbdf4 Mon Sep 17 00:00:00 2001 From: Daniel Szoke <7881302+szokeasaurusrex@users.noreply.github.com> Date: Mon, 23 Sep 2024 10:11:19 +0200 Subject: [PATCH 2/3] fix(aiohttp): Handle invalid responses (#3554) If the request handler returns an invalid response (e.g. `None`), our SDK triggers an error because we try to access the invalid response's `status` attribute. Wrap this with a `try` block to handle the `AttributeError` and ensure the SDK does not break the app. --- sentry_sdk/integrations/aiohttp.py | 12 +++++++++++- tests/integrations/aiohttp/test_aiohttp.py | 21 +++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/sentry_sdk/integrations/aiohttp.py b/sentry_sdk/integrations/aiohttp.py index a447b67f38..6a738f3af0 100644 --- a/sentry_sdk/integrations/aiohttp.py +++ b/sentry_sdk/integrations/aiohttp.py @@ -139,7 +139,17 @@ async def sentry_app_handle(self, request, *args, **kwargs): # have no way to tell. Do not set span status. reraise(*_capture_exception()) - transaction.set_http_status(response.status) + try: + # A valid response handler will return a valid response with a status. But, if the handler + # returns an invalid response (e.g. None), the line below will raise an AttributeError. + # Even though this is likely invalid, we need to handle this case to ensure we don't break + # the application. + response_status = response.status + except AttributeError: + pass + else: + transaction.set_http_status(response_status) + return response Application._handle = sentry_app_handle diff --git a/tests/integrations/aiohttp/test_aiohttp.py b/tests/integrations/aiohttp/test_aiohttp.py index 43e3bec546..be372b6643 100644 --- a/tests/integrations/aiohttp/test_aiohttp.py +++ b/tests/integrations/aiohttp/test_aiohttp.py @@ -596,3 +596,24 @@ async def hello(request): (event,) = events assert event["contexts"]["trace"]["origin"] == "auto.http.aiohttp" assert event["spans"][0]["origin"] == "auto.http.aiohttp" + + +@pytest.mark.asyncio +@pytest.mark.parametrize("invalid_response", (None, "invalid")) +async def test_invalid_response( + sentry_init, aiohttp_client, capture_events, invalid_response +): + sentry_init(integrations=[AioHttpIntegration()]) + + async def handler(_): + return invalid_response + + app = web.Application() + app.router.add_get("/", handler) + + client = await aiohttp_client(app) + + # Invalid response should result on a ServerDisconnectedError in the client side, not an internal server error. + # Important to note that the ServerDisconnectedError indicates we have no error server-side. + with pytest.raises(ServerDisconnectedError): + await client.get("/") From 26b86a5e256a54ed83060863a350f46c8522645e Mon Sep 17 00:00:00 2001 From: Burak Yigit Kaya Date: Mon, 23 Sep 2024 09:21:34 +0100 Subject: [PATCH 3/3] fix: Fix breadcrumb timestamp casting and its tests (#3546) These tests were failing for me locally as the timestamps were without tzinfo and all were assumed UTC whereas my local timezone is BST at the moment. This patch fixes the tests along with faulty/incomplete breadcrumb timestamp parsing logic on py3.7 and py3.8. --------- Co-authored-by: Anton Pirker Co-authored-by: Ivana Kellyer --- sentry_sdk/scope.py | 5 +++-- sentry_sdk/utils.py | 22 ++++++++++++++++--- tests/test_basics.py | 39 +++++++++++++++++++++------------- tests/test_utils.py | 50 ++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 96 insertions(+), 20 deletions(-) diff --git a/sentry_sdk/scope.py b/sentry_sdk/scope.py index adae8dc888..0c0482904e 100644 --- a/sentry_sdk/scope.py +++ b/sentry_sdk/scope.py @@ -968,7 +968,7 @@ def start_transaction( transaction=None, instrumenter=INSTRUMENTER.SENTRY, custom_sampling_context=None, - **kwargs + **kwargs, ): # type: (Optional[Transaction], str, Optional[SamplingContext], Unpack[TransactionKwargs]) -> Union[Transaction, NoOpSpan] """ @@ -1324,7 +1324,8 @@ def _apply_breadcrumbs_to_event(self, event, hint, options): crumb["timestamp"] = datetime_from_isoformat(crumb["timestamp"]) event["breadcrumbs"]["values"].sort(key=lambda crumb: crumb["timestamp"]) - except Exception: + except Exception as err: + logger.debug("Error when sorting breadcrumbs", exc_info=err) pass def _apply_user_to_event(self, event, hint, options): diff --git a/sentry_sdk/utils.py b/sentry_sdk/utils.py index 38ab7e3618..44cb98bfed 100644 --- a/sentry_sdk/utils.py +++ b/sentry_sdk/utils.py @@ -239,13 +239,29 @@ def format_timestamp(value): return utctime.strftime("%Y-%m-%dT%H:%M:%S.%fZ") +ISO_TZ_SEPARATORS = frozenset(("+", "-")) + + def datetime_from_isoformat(value): # type: (str) -> datetime try: - return datetime.fromisoformat(value) - except AttributeError: + result = datetime.fromisoformat(value) + except (AttributeError, ValueError): # py 3.6 - return datetime.strptime(value, "%Y-%m-%dT%H:%M:%S.%f") + timestamp_format = ( + "%Y-%m-%dT%H:%M:%S.%f" if "." in value else "%Y-%m-%dT%H:%M:%S" + ) + if value.endswith("Z"): + value = value[:-1] + "+0000" + + if value[-6] in ISO_TZ_SEPARATORS: + timestamp_format += "%z" + value = value[:-3] + value[-2:] + elif value[-5] in ISO_TZ_SEPARATORS: + timestamp_format += "%z" + + result = datetime.strptime(value, timestamp_format) + return result.astimezone(timezone.utc) def event_hint_with_exc_info(exc_info=None): diff --git a/tests/test_basics.py b/tests/test_basics.py index 6f77353c8a..74dfe1955a 100644 --- a/tests/test_basics.py +++ b/tests/test_basics.py @@ -8,6 +8,7 @@ import pytest from sentry_sdk.client import Client +from sentry_sdk.utils import datetime_from_isoformat from tests.conftest import patch_start_tracing_child import sentry_sdk @@ -397,11 +398,12 @@ def test_breadcrumbs(sentry_init, capture_events): def test_breadcrumb_ordering(sentry_init, capture_events): sentry_init() events = capture_events() + now = datetime.datetime.now(datetime.timezone.utc).replace(microsecond=0) timestamps = [ - datetime.datetime.now() - datetime.timedelta(days=10), - datetime.datetime.now() - datetime.timedelta(days=8), - datetime.datetime.now() - datetime.timedelta(days=12), + now - datetime.timedelta(days=10), + now - datetime.timedelta(days=8), + now - datetime.timedelta(days=12), ] for timestamp in timestamps: @@ -417,10 +419,7 @@ def test_breadcrumb_ordering(sentry_init, capture_events): assert len(event["breadcrumbs"]["values"]) == len(timestamps) timestamps_from_event = [ - datetime.datetime.strptime( - x["timestamp"].replace("Z", ""), "%Y-%m-%dT%H:%M:%S.%f" - ) - for x in event["breadcrumbs"]["values"] + datetime_from_isoformat(x["timestamp"]) for x in event["breadcrumbs"]["values"] ] assert timestamps_from_event == sorted(timestamps) @@ -428,11 +427,24 @@ def test_breadcrumb_ordering(sentry_init, capture_events): def test_breadcrumb_ordering_different_types(sentry_init, capture_events): sentry_init() events = capture_events() + now = datetime.datetime.now(datetime.timezone.utc) timestamps = [ - datetime.datetime.now() - datetime.timedelta(days=10), - datetime.datetime.now() - datetime.timedelta(days=8), - datetime.datetime.now() - datetime.timedelta(days=12), + now - datetime.timedelta(days=10), + now - datetime.timedelta(days=8), + now.replace(microsecond=0) - datetime.timedelta(days=12), + now - datetime.timedelta(days=9), + now - datetime.timedelta(days=13), + now.replace(microsecond=0) - datetime.timedelta(days=11), + ] + + breadcrumb_timestamps = [ + timestamps[0], + timestamps[1].isoformat(), + datetime.datetime.strftime(timestamps[2], "%Y-%m-%dT%H:%M:%S") + "Z", + datetime.datetime.strftime(timestamps[3], "%Y-%m-%dT%H:%M:%S.%f") + "+00:00", + datetime.datetime.strftime(timestamps[4], "%Y-%m-%dT%H:%M:%S.%f") + "+0000", + datetime.datetime.strftime(timestamps[5], "%Y-%m-%dT%H:%M:%S.%f") + "-0000", ] for i, timestamp in enumerate(timestamps): @@ -440,7 +452,7 @@ def test_breadcrumb_ordering_different_types(sentry_init, capture_events): message="Authenticated at %s" % timestamp, category="auth", level="info", - timestamp=timestamp if i % 2 == 0 else timestamp.isoformat(), + timestamp=breadcrumb_timestamps[i], ) capture_exception(ValueError()) @@ -448,10 +460,7 @@ def test_breadcrumb_ordering_different_types(sentry_init, capture_events): assert len(event["breadcrumbs"]["values"]) == len(timestamps) timestamps_from_event = [ - datetime.datetime.strptime( - x["timestamp"].replace("Z", ""), "%Y-%m-%dT%H:%M:%S.%f" - ) - for x in event["breadcrumbs"]["values"] + datetime_from_isoformat(x["timestamp"]) for x in event["breadcrumbs"]["values"] ] assert timestamps_from_event == sorted(timestamps) diff --git a/tests/test_utils.py b/tests/test_utils.py index 4df343a357..c46cac7f9f 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -12,6 +12,7 @@ from sentry_sdk.utils import ( Components, Dsn, + datetime_from_isoformat, env_to_bool, format_timestamp, get_current_thread_meta, @@ -61,6 +62,55 @@ def _normalize_distribution_name(name): return re.sub(r"[-_.]+", "-", name).lower() +@pytest.mark.parametrize( + ("input_str", "expected_output"), + ( + ( + "2021-01-01T00:00:00.000000Z", + datetime(2021, 1, 1, tzinfo=timezone.utc), + ), # UTC time + ( + "2021-01-01T00:00:00.000000", + datetime(2021, 1, 1, tzinfo=datetime.now().astimezone().tzinfo), + ), # No TZ -- assume UTC + ( + "2021-01-01T00:00:00Z", + datetime(2021, 1, 1, tzinfo=timezone.utc), + ), # UTC - No milliseconds + ( + "2021-01-01T00:00:00.000000+00:00", + datetime(2021, 1, 1, tzinfo=timezone.utc), + ), + ( + "2021-01-01T00:00:00.000000-00:00", + datetime(2021, 1, 1, tzinfo=timezone.utc), + ), + ( + "2021-01-01T00:00:00.000000+0000", + datetime(2021, 1, 1, tzinfo=timezone.utc), + ), + ( + "2021-01-01T00:00:00.000000-0000", + datetime(2021, 1, 1, tzinfo=timezone.utc), + ), + ( + "2020-12-31T00:00:00.000000+02:00", + datetime(2020, 12, 31, tzinfo=timezone(timedelta(hours=2))), + ), # UTC+2 time + ( + "2020-12-31T00:00:00.000000-0200", + datetime(2020, 12, 31, tzinfo=timezone(timedelta(hours=-2))), + ), # UTC-2 time + ( + "2020-12-31T00:00:00-0200", + datetime(2020, 12, 31, tzinfo=timezone(timedelta(hours=-2))), + ), # UTC-2 time - no milliseconds + ), +) +def test_datetime_from_isoformat(input_str, expected_output): + assert datetime_from_isoformat(input_str) == expected_output, input_str + + @pytest.mark.parametrize( "env_var_value,strict,expected", [