Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions sentry_sdk/integrations/bottle.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
)
from sentry_sdk.integrations._wsgi_common import RequestExtractor
from sentry_sdk.integrations.wsgi import SentryWsgiMiddleware
from sentry_sdk.tracing import SOURCE_FOR_STYLE
from sentry_sdk.traces import SOURCE_FOR_STYLE as SEGMENT_SOURCE_FOR_STYLE
from sentry_sdk.tracing import SOURCE_FOR_STYLE as TRANSACTION_SOURCE_FOR_STYLE
from sentry_sdk.tracing_utils import has_span_streaming_enabled
Comment thread
ericapisani marked this conversation as resolved.
from sentry_sdk.utils import (
capture_internal_exceptions,
ensure_integration_enabled,
Expand Down Expand Up @@ -102,6 +104,11 @@
)
res = old_handle(self, environ)

if has_span_streaming_enabled(sentry_sdk.get_client().options):
_set_segment_name_and_source(
transaction_style=integration.transaction_style
)

Check warning on line 110 in sentry_sdk/integrations/bottle.py

View check run for this annotation

@sentry/warden / warden: code-review

Segment name not updated when route handler raises an unhandled exception

Because `_set_segment_name_and_source` is called after `old_handle` returns normally, any unhandled exception propagating out of the route callback (e.g. `ZeroDivisionError`) causes the segment name to never be updated to the matched route, leaving it with the WSGI middleware's default name. Wrapping the call in a `try/finally` block would ensure it runs in both success and error paths.
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Post-handle naming can fail requests

Medium Severity

After a successful old_handle, span-streaming runs _set_segment_name_and_source on the critical response path. That helper only catches RuntimeError, and the call is not wrapped in capture_internal_exceptions. Any other exception while reading bottle_request.route or calling set_transaction_name can abort the request after the handler already returned a response.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 5753f41. Configure here.


return res

Bottle._handle = _patched_handle
Expand Down Expand Up @@ -163,6 +170,25 @@
return file.content_length


def _set_segment_name_and_source(transaction_style: str) -> None:
try:
if transaction_style == "url":
name = bottle_request.route.rule or "bottle request"
else:
name = (
bottle_request.route.name
or transaction_from_function(bottle_request.route.callback)
or "bottle request"
)

sentry_sdk.get_current_scope().set_transaction_name(
name,
source=SEGMENT_SOURCE_FOR_STYLE[transaction_style],
)
except RuntimeError:

Check warning on line 188 in sentry_sdk/integrations/bottle.py

View check run for this annotation

@sentry/warden / warden: find-bugs

`_set_segment_name_and_source` only catches `RuntimeError`, but can raise `KeyError` for unmatched routes, discarding the already-computed response

When a request hits a 404 (no matched route), `bottle_request.route` accesses `environ['bottle.route']` which is unset, raising `KeyError`. Since the `except RuntimeError: pass` doesn't catch `KeyError`, the exception propagates through `_patched_handle` after `res = old_handle(...)` has already returned a valid response—discarding it and causing an unintended 500 error for the client when span streaming is enabled.
pass


def _set_transaction_name_and_source(
event: "Event", transaction_style: str, request: "Any"
) -> None:
Expand All @@ -185,7 +211,9 @@
pass

event["transaction"] = name
event["transaction_info"] = {"source": SOURCE_FOR_STYLE[transaction_style]}
event["transaction_info"] = {
"source": TRANSACTION_SOURCE_FOR_STYLE[transaction_style]
}


def _make_request_event_processor(
Expand Down
259 changes: 255 additions & 4 deletions tests/integrations/bottle/test_bottle.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from werkzeug.test import Client
from werkzeug.wrappers import Response

import sentry_sdk
from sentry_sdk import capture_message
from sentry_sdk.integrations.bottle import BottleIntegration
from sentry_sdk.integrations.logging import LoggingIntegration
Expand Down Expand Up @@ -462,23 +463,37 @@
assert not events


@pytest.mark.parametrize("span_streaming", [True, False])
def test_span_origin(
sentry_init,
get_client,
capture_events,
capture_items,
span_streaming,
):
sentry_init(
integrations=[BottleIntegration()],
traces_sample_rate=1.0,
_experiments={"trace_lifecycle": "stream" if span_streaming else "static"},
)
events = capture_events()

if span_streaming:
items = capture_items("span")
else:
events = capture_events()

client = get_client()
client.get("/message")

(_, event) = events

assert event["contexts"]["trace"]["origin"] == "auto.http.bottle"
if span_streaming:
sentry_sdk.flush()
spans = [item.payload for item in items]
segment = spans[-1]
assert segment["is_segment"] is True
assert segment["attributes"]["sentry.origin"] == "auto.http.bottle"
else:
(_, event) = events
assert event["contexts"]["trace"]["origin"] == "auto.http.bottle"


@pytest.mark.parametrize("raise_error", [True, False])
Expand Down Expand Up @@ -556,3 +571,239 @@

(event,) = events
assert event["exception"]["values"][0]["type"] == "ZeroDivisionError"


def test_span_streaming_basic(sentry_init, capture_items):
sentry_init(
integrations=[BottleIntegration()],
traces_sample_rate=1.0,
_experiments={"trace_lifecycle": "stream"},
)
items = capture_items("span")

app = Bottle()

@app.route("/message")
def hi():
return "ok"

client = Client(app)
client.get("/message")

sentry_sdk.flush()

spans = [item.payload for item in items]
assert len(spans) == 1

segment = spans[0]

# Segment span (root, created by WSGI middleware)
assert segment["is_segment"] is True
assert "parent_span_id" not in segment
assert segment["status"] == "ok"
assert segment["attributes"]["sentry.op"] == "http.server"
assert segment["attributes"]["sentry.origin"] == "auto.http.bottle"
assert segment["attributes"]["http.request.method"] == "GET"
assert segment["attributes"]["http.response.status_code"] == 200
assert segment["name"].endswith("hi")


@pytest.mark.parametrize(
"url,transaction_style,expected_name,expected_source",
[
("/message", "endpoint", "hi", "component"),
("/message", "url", "/message", "route"),
("/message/123456", "url", "/message/<message_id>", "route"),
("/message-named-route", "endpoint", "hi", "component"),
],
)
def test_span_streaming_transaction_style(
sentry_init,
capture_items,
url,
transaction_style,
expected_name,
expected_source,
):
sentry_init(
integrations=[BottleIntegration(transaction_style=transaction_style)],
traces_sample_rate=1.0,
_experiments={"trace_lifecycle": "stream"},
)
items = capture_items("span")

app = Bottle()

@app.route("/message")
def hi():
return "ok"

@app.route("/message/<message_id>")
def hi_with_id(message_id):
return "ok"

@app.route("/message-named-route", name="hi")
def named_hi():
return "ok"

client = Client(app)
client.get(url)

sentry_sdk.flush()

spans = [item.payload for item in items]
assert len(spans) == 1

segment = spans[0]

assert segment["is_segment"] is True

assert segment["name"].endswith(expected_name)
assert segment["attributes"]["sentry.span.source"] == expected_source


def test_span_streaming_with_error(sentry_init, capture_items):
sentry_init(
integrations=[BottleIntegration()],
traces_sample_rate=1.0,
_experiments={"trace_lifecycle": "stream"},
)
items = capture_items("event", "span")

app = Bottle()

@app.route("/error")
def error():
1 / 0

client = Client(app)
try:
client.get("/error")
except ZeroDivisionError:
pass

sentry_sdk.flush()

events = [item.payload for item in items if item.type == "event"]
spans = [item.payload for item in items if item.type == "span"]
assert len(events) == 1
assert len(spans) == 1

error_event = events[0]
segment = spans[0]

# Confirm the same trace is shared
assert segment["trace_id"] == error_event["contexts"]["trace"]["trace_id"]

# Span hierarchy
assert segment["is_segment"] is True
assert "parent_span_id" not in segment

# Error event span_id points to the segment span (where the exception was raised)
assert error_event["contexts"]["trace"]["span_id"] == segment["span_id"]

# Span status
assert segment["status"] == "error"

# Bottle mechanism on the error event
assert error_event["exception"]["values"][0]["mechanism"]["type"] == "bottle"
assert error_event["exception"]["values"][0]["mechanism"]["handled"] is False


@pytest.mark.parametrize(
"status_code,expected_span_status",
[
(200, "ok"),
(404, "error"),
(500, "error"),
],
)
def test_span_streaming_http_error_status(
sentry_init,
capture_items,
status_code,
expected_span_status,
):
sentry_init(
integrations=[BottleIntegration()],
traces_sample_rate=1.0,
_experiments={"trace_lifecycle": "stream"},
)
items = capture_items("span")

app = Bottle()

@app.route("/")
def handle():
return HTTPResponse(status=status_code, body="response")

client = Client(app)
client.get("/")

sentry_sdk.flush()

spans = [item.payload for item in items]
assert len(spans) == 1

segment = spans[0]

assert segment["is_segment"] is True

assert segment["status"] == expected_span_status
assert segment["attributes"]["http.response.status_code"] == status_code


@pytest.mark.parametrize("raise_error", [True, False])
@pytest.mark.parametrize(
("integration_kwargs", "status_code", "should_capture"),
(
({}, 500, True),
({}, 400, False),
({"failed_request_status_codes": set()}, 500, False),
({"failed_request_status_codes": {404, *range(500, 600)}}, 404, True),
({"failed_request_status_codes": {404, *range(500, 600)}}, 400, False),
),
)
def test_span_streaming_failed_request_status_codes(
sentry_init,
capture_items,
integration_kwargs,
status_code,
should_capture,
raise_error,
):
sentry_init(
integrations=[BottleIntegration(**integration_kwargs)],
traces_sample_rate=1.0,
_experiments={"trace_lifecycle": "stream"},
)
items = capture_items("event", "span")

app = Bottle()

@app.route("/")
def handle():
response = HTTPResponse(status=status_code)
if raise_error:
raise response
return response

client = Client(app, Response)
client.get("/")

sentry_sdk.flush()

events = [item.payload for item in items if item.type == "event"]
spans = [item.payload for item in items if item.type == "span"]
assert len(spans) == 1

segment = spans[0]

assert segment["is_segment"] is True

if should_capture:
assert len(events) == 1
assert events[0]["exception"]["values"][0]["type"] == "HTTPResponse"

Check warning on line 806 in tests/integrations/bottle/test_bottle.py

View check run for this annotation

@sentry/warden / warden: find-bugs

test_span_streaming_failed_request_status_codes asserts `handled=True` for raised HTTPResponse but implementation always uses `handled=False` for raised exceptions

The assertion `mechanism["handled"] is True` is unconditional across both `raise_error=True` and `raise_error=False`, but the `wrapped_callback` in bottle.py captures all raised exceptions (including `HTTPResponse`, which extends `BottleException(Exception)`) with `handled=False`; only a _returned_ `HTTPResponse` reaches the `handled=True` path.
assert events[0]["exception"]["values"][0]["mechanism"]["handled"] is True

Check warning on line 807 in tests/integrations/bottle/test_bottle.py

View check run for this annotation

@sentry/warden / warden: code-review

`mechanism["handled"]` asserted `True` for raised HTTPResponse, but implementation sets `False`

When `raise_error=True`, the `HTTPResponse` is raised and caught in the `except Exception` branch of `wrapped_callback`, which calls `_capture_exception(exception, handled=False)`. The assertion on line 807 unconditionally expects `handled=True` for all `should_capture` cases, so every `raise_error=True` parametrize combination where `should_capture=True` will fail.
else:
assert len(events) == 0
Loading