Skip to content
Open
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
4 changes: 4 additions & 0 deletions sentry_sdk/integrations/openai.py
Original file line number Diff line number Diff line change
Expand Up @@ -613,6 +613,8 @@
nonlocal ttft
count_tokens_manually = True
for x in old_iterator:
span.set_data(SPANDATA.GEN_AI_RESPONSE_MODEL, x.model)

Check warning on line 616 in sentry_sdk/integrations/openai.py

View workflow job for this annotation

GitHub Actions / warden: code-review

Unprotected attribute access on streaming chunk may cause runtime error

The `x.model` access on line 616 is placed outside the `capture_internal_exceptions()` block, unlike other model accesses in this file (e.g., lines 761, 810 which are inside the block, or line 472 which uses `hasattr` guard). If a streaming chunk lacks the `model` attribute or it's malformed, an `AttributeError` will propagate to the user's application instead of being silently logged by Sentry's internal error handler.
Copy link
Contributor

Choose a reason for hiding this comment

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

Unprotected attribute access on streaming chunk may cause runtime error

The x.model access on line 616 is placed outside the capture_internal_exceptions() block, unlike other model accesses in this file (e.g., lines 761, 810 which are inside the block, or line 472 which uses hasattr guard). If a streaming chunk lacks the model attribute or it's malformed, an AttributeError will propagate to the user's application instead of being silently logged by Sentry's internal error handler.

Verification

Read openai.py lines 590-670 and 740-815. Verified pattern: line 472 uses if hasattr(response, 'model') guard, lines 761 and 810 place model access inside capture_internal_exceptions(). The new code at line 616 has neither protection and could raise AttributeError to user code.

Suggested fix: Move the set_data call inside the existing capture_internal_exceptions() block

Suggested change
span.set_data(SPANDATA.GEN_AI_RESPONSE_MODEL, x.model)
if hasattr(x, "model"):
span.set_data(SPANDATA.GEN_AI_RESPONSE_MODEL, x.model)

Identified by Warden code-review · 26J-GUA

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link

Choose a reason for hiding this comment

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

Ungarded streamed model access can raise

Medium Severity

x.model is read directly in the streaming iterators before capture_internal_exceptions(). If a streamed chunk does not expose model (or uses a dict-like/event variant), this raises and interrupts iteration, making instrumentation break the caller’s stream instead of failing safely.

Additional Locations (1)

Fix in Cursor Fix in Web

Copy link
Contributor Author

Choose a reason for hiding this comment

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


with capture_internal_exceptions():
if hasattr(x, "choices"):
choice_index = 0
Expand Down Expand Up @@ -657,6 +659,8 @@
nonlocal ttft
count_tokens_manually = True
async for x in old_iterator:
span.set_data(SPANDATA.GEN_AI_RESPONSE_MODEL, x.model)
Copy link
Contributor

Choose a reason for hiding this comment

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

Accessing x.model without defensive check could raise AttributeError and break streaming

The new code accesses x.model directly without checking if the attribute exists, and this call is placed outside the capture_internal_exceptions() block. If a streaming chunk doesn't have a model attribute (which could happen with certain API responses or edge cases), an AttributeError will propagate to the user's application and break their streaming logic. The rest of the code in this function uses defensive checks like hasattr(x, "choices") before accessing attributes.

Verification

Read the full function context at lines 658-686. Verified that the synchronous version at line 616 has the same pattern (also potentially problematic). Confirmed that capture_internal_exceptions() is used elsewhere in the file to protect against attribute access errors. Checked that set_data is a simple dictionary assignment that won't throw, so the issue is specifically with x.model access.

Suggested fix: Wrap the model attribute access inside the existing capture_internal_exceptions block or add a hasattr check

Suggested change
span.set_data(SPANDATA.GEN_AI_RESPONSE_MODEL, x.model)
if hasattr(x, "model"):
span.set_data(SPANDATA.GEN_AI_RESPONSE_MODEL, x.model)
Also found at 1 additional location
  • sentry_sdk/integrations/openai.py:616-616

Identified by Warden code-review · CWR-SCT

Copy link
Contributor

Choose a reason for hiding this comment

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

Fix attempt detected (commit f12cf5d)

The commit added unprotected x.model access outside the capture_internal_exceptions() block at both lines 616 and 662, directly introducing the exact issue reported: an AttributeError could propagate if a streaming chunk lacks the model attribute.

The original issue appears unresolved. Please review and try again.

Evaluated by Warden

Copy link
Contributor Author

Choose a reason for hiding this comment

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


with capture_internal_exceptions():
if hasattr(x, "choices"):
choice_index = 0
Expand Down
8 changes: 8 additions & 0 deletions tests/integrations/openai/test_openai.py
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,8 @@ def test_streaming_chat_completion_no_prompts(
span = tx["spans"][0]
assert span["op"] == "gen_ai.chat"

assert span["data"][SPANDATA.GEN_AI_RESPONSE_MODEL] == "model-id"

assert SPANDATA.GEN_AI_SYSTEM_INSTRUCTIONS not in span["data"]
assert SPANDATA.GEN_AI_REQUEST_MESSAGES not in span["data"]
assert SPANDATA.GEN_AI_RESPONSE_TEXT not in span["data"]
Expand Down Expand Up @@ -656,6 +658,8 @@ def test_streaming_chat_completion(sentry_init, capture_events, messages, reques
},
]

assert span["data"][SPANDATA.GEN_AI_RESPONSE_MODEL] == "model-id"

assert "hello" in span["data"][SPANDATA.GEN_AI_REQUEST_MESSAGES]
assert "hello world" in span["data"][SPANDATA.GEN_AI_RESPONSE_TEXT]

Expand Down Expand Up @@ -762,6 +766,8 @@ async def test_streaming_chat_completion_async_no_prompts(
span = tx["spans"][0]
assert span["op"] == "gen_ai.chat"

assert span["data"][SPANDATA.GEN_AI_RESPONSE_MODEL] == "model-id"

assert SPANDATA.GEN_AI_SYSTEM_INSTRUCTIONS not in span["data"]
assert SPANDATA.GEN_AI_REQUEST_MESSAGES not in span["data"]
assert SPANDATA.GEN_AI_RESPONSE_TEXT not in span["data"]
Expand Down Expand Up @@ -897,6 +903,8 @@ async def test_streaming_chat_completion_async(
span = tx["spans"][0]
assert span["op"] == "gen_ai.chat"

assert span["data"][SPANDATA.GEN_AI_RESPONSE_MODEL] == "model-id"

param_id = request.node.callspec.id
if "blocks" in param_id:
assert json.loads(span["data"][SPANDATA.GEN_AI_SYSTEM_INSTRUCTIONS]) == [
Expand Down
Loading