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
7 changes: 6 additions & 1 deletion sentry_sdk/scope.py
Original file line number Diff line number Diff line change
Expand Up @@ -891,6 +891,7 @@ def user(self, value: "Optional[Dict[str, Any]]") -> None:
def set_user(self, value: "Optional[Dict[str, Any]]") -> None:
"""Sets a user for the scope."""
self._user = value

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Bug: The set_user method stores the user dictionary without filtering None values, causing regular events to include null attributes, which contradicts the PR's goal.
Severity: MEDIUM

Suggested Fix

Filter None values from the user dictionary within the set_user method before storing it in self._user. This ensures all subsequent uses of self._user, including in _apply_user_to_event, will have the correctly filtered data. Alternatively, modify _apply_user_to_event to filter the dictionary before adding it to the event.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.

Location: sentry_sdk/scope.py#L893

Potential issue: The `set_user` method stores the user dictionary in `self._user`
without filtering out keys that have `None` values. Subsequently, the
`_apply_user_to_event` method copies this dictionary directly to the event payload. As a
result, if a user is set with a dictionary containing `None` values (e.g.,
`scope.set_user({"username": None})`), the resulting Sentry event will still contain
`null` for that attribute. This behavior is inconsistent, as the filtering logic was
only added for the telemetry/span path in `_apply_user_attributes_to_telemetry`, but not
for the regular event path.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This is the existing behaviour. Leaving as-is because the string-ifying of 'None' happens on attributes-based telemetry (i.e. streamed spans)


session = self.get_isolation_scope()._session
if session is not None:
session.update(user=value)
Expand Down Expand Up @@ -1753,7 +1754,11 @@ def _apply_user_attributes_to_telemetry(
("user.email", "email"),
("user.ip_address", "ip_address"),
):
if user_attribute in self._user and attribute_name not in attributes:
if (
user_attribute in self._user
and attribute_name not in attributes
and self._user[user_attribute] is not None
):
attributes[attribute_name] = self._user[user_attribute]

def _drop(self, cause: "Any", ty: str) -> "Optional[Any]":
Expand Down
37 changes: 37 additions & 0 deletions tests/test_scope.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,43 @@ def test_scope_flags_copy():
]


def test_set_user(sentry_init, capture_events):
sentry_init()
events = capture_events()

sentry_sdk.get_isolation_scope().set_user({"id": "42", "email": "bob@example.com"})
capture_exception(NameError())
assert events[-1]["user"] == {"id": "42", "email": "bob@example.com"}

sentry_sdk.get_isolation_scope().set_user(None)
capture_exception(NameError())
assert "user" not in events[-1]


def test_set_user_none_values_are_dropped_when_copying_to_attributes(
sentry_init, capture_items
):
sentry_init(
traces_sample_rate=1.0,
send_default_pii=True,
_experiments={"trace_lifecycle": "stream"},
)
items = capture_items("span")

with sentry_sdk.traces.start_span(name="test_segment"):
sentry_sdk.get_isolation_scope().set_user(
{"email": "ada@beans.com", "username": None}
)
capture_exception(NameError())

sentry_sdk.flush()

segment = items[0].payload

assert "user.name" not in segment["attributes"]
assert segment["attributes"]["user.email"] == "ada@beans.com"


def test_merging(sentry_init, capture_events):
sentry_init()

Expand Down
Loading