Conversation
cf2d25c to
bd37ba6
Compare
| @pytest_mark_django_db_decorator() | ||
| @pytest.mark.skipif( | ||
| django.VERSION < (3, 0), reason="Django ASGI support shipped in 3.0" | ||
| ) |
There was a problem hiding this comment.
Test skipif condition requires newer Django version
Medium Severity
The test test_user_pii_in_asgi_with_auth has a skipif condition checking for django.VERSION < (3, 0), but the test view async_mylogin uses User.objects.acreate_user() (added in Django 4.1) and alogin (added in Django 5.1). This mismatch means the test will fail with an ImportError when run on Django versions 3.0 through 5.0, since the skipif condition won't exclude those versions but the required APIs don't exist.
🔬 Verification Test
Why verification test was not possible: This test failure would manifest only when running the test suite against specific Django versions (3.x, 4.x, 5.0). The test would fail with an ImportError: cannot import name 'alogin' from 'django.contrib.auth' when Django < 5.1 is used. Verifying this would require setting up multiple Django version environments which is outside the scope of this review.
Additional Locations (1)
|
|
||
| def _set_user_info(request: "ASGIRequest", event: "Event") -> None: | ||
| user_info = getattr(request, "_sentry_user_info", {}) | ||
| event.setdefault("user", user_info) |
There was a problem hiding this comment.
Async user info doesn't merge with existing user data
Low Severity
The new async _set_user_info uses event.setdefault("user", user_info) which only sets the user dict if no "user" key exists. In contrast, the sync version in sentry_sdk/integrations/django/__init__.py uses event.setdefault("user", {}) then user_info.setdefault() on individual fields, which merges request user data into any existing user dict. This means if scope.set_user({"id": "custom"}) is called before the event processor runs, the sync version would still add email and username from the authenticated user, but the async version adds nothing. This behavioral inconsistency could cause missing user fields in async contexts.
🔬 Verification Test
Why verification test was not possible: This requires testing the interaction between scope.set_user() and the event processor in an async Django context, which would need a full integration test environment with Django ASGI running. The behavioral difference is clear from comparing the sync implementation at line 584-605 in __init__.py (which calls setdefault on individual fields) versus the async implementation (which calls setdefault on the entire user dict).
bd37ba6 to
a908d9d
Compare
a908d9d to
6c0c2a7
Compare
Description
For async views in django, we're supposed to use the async version
auserto fetch user info.Issues