Skip to content

Conversation

@shellmayr
Copy link
Member

Closes TET-1208

@linear
Copy link

linear bot commented Oct 30, 2025

@@ -1,33 +1,35 @@
import asyncio
Copy link
Member Author

Choose a reason for hiding this comment

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

Organized some imports here

@codecov
Copy link

codecov bot commented Oct 30, 2025

Codecov Report

❌ Patch coverage is 76.19048% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.91%. Comparing base (2d49b74) to head (767fd62).
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
sentry_sdk/integrations/openai_agents/utils.py 76.47% 2 Missing and 2 partials ⚠️
...k/integrations/openai_agents/spans/invoke_agent.py 75.00% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5049      +/-   ##
==========================================
- Coverage   83.93%   83.91%   -0.03%     
==========================================
  Files         179      179              
  Lines       17893    17906      +13     
  Branches     3181     3183       +2     
==========================================
+ Hits        15019    15026       +7     
- Misses       1909     1912       +3     
- Partials      965      968       +3     
Files with missing lines Coverage Δ
...k/integrations/openai_agents/spans/invoke_agent.py 86.11% <75.00%> (-1.77%) ⬇️
sentry_sdk/integrations/openai_agents/utils.py 86.17% <76.47%> (-1.93%) ⬇️

... and 1 file with indirect coverage changes

@shellmayr shellmayr marked this pull request as ready for review October 31, 2025 09:34
@shellmayr shellmayr requested a review from a team as a code owner October 31, 2025 09:34
Comment on lines +117 to +121
serialized_str = safe_serialize(message)
try:
serialized_message = json.loads(serialized_str)
except (json.JSONDecodeError, TypeError):
continue
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do you serialize and deserialize here?

Copy link
Member Author

Choose a reason for hiding this comment

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

I haven't found a better way to be sure that whatever is in the messages is actually serialize-able down the line. I.e. if there is an object that is passed in the messages, and the object is not serializeable, it will fail. Do we have a better way of doing this?

Copy link
Contributor

Choose a reason for hiding this comment

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

what exactly fails down the line?

I'm just trying to understand why we need this. Do we want to ensure we only have standard JSON types like strings, integers and so on in the dictionaries?

Because then we can just directly check the types in the dictionary.

{
"role": GEN_AI_ALLOWED_MESSAGE_ROLES.TOOL,
"content": [message],
"content": [serialized_message],
Copy link

Choose a reason for hiding this comment

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

Bug: Bug

The safe_serialize and json.loads pattern can result in serialized_message being a non-dictionary type. This causes AttributeError or TypeError when attempting to access dictionary-like properties, as the try/except block is too narrow. This can lead to messages being silently skipped or the function crashing.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants