fix(asgi): Stop duplicating scope["root_path"] in URLs#6579
fix(asgi): Stop duplicating scope["root_path"] in URLs#6579alexander-alderman-webb wants to merge 15 commits into
scope["root_path"] in URLs#6579Conversation
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
Codecov Results 📊✅ 90267 passed | ❌ 24 failed | ⏭️ 6124 skipped | Total: 96415 | Pass Rate: 93.62% | Execution Time: 334m 48s 📊 Comparison with Base Branch
➕ New Tests (24)View new tests
❌ Failed Tests
|
| File | Patch % | Lines |
|---|---|---|
| sentry_sdk/integrations/quart.py | 100.00% |
Coverage diff
@@ Coverage Diff @@
## main #PR +/-##
==========================================
+ Coverage 89.85% 89.85% —%
==========================================
Files 192 192 —
Lines 23758 23762 +4
Branches 8204 8204 —
==========================================
+ Hits 21347 21351 +4
- Misses 2411 2411 —
- Partials 1344 1346 +2Generated by Codecov Action
alexander-alderman-webb
changed the title
scope["root_path"] in URLs
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
wahajahmed010
left a comment
wahajahmed010
left a comment
There was a problem hiding this comment.
Review: #6579
Summary: Clean fix for ASGI URL duplication when scope["root_path"] is already included in scope["path"]. The approach is sound — thread a path_includes_root_path boolean through the URL construction pipeline.
✅ What works well
- Backward compatible — default is
True, preserving existing behavior for frameworks that don't include root_path in path - Framework-specific handling — correctly version-gated for Starlette (>=0.33), Quart (>=0.19), and hardcoded for Litestar/Starlite
- Test coverage — all 6 integrations tested (Django ASGI, FastAPI, Litestar, Quart, Starlette, Starlite), both span streaming and static lifecycle modes
- Clean threading — parameter flows through
_get_url→_get_request_data→_get_request_attributes→_get_transaction_name_and_source→_get_segment_name_and_source
⚠️ Potential issues
- Django ASGI test — The test sets
comm.scope["root_path"] = "/root"but Django defaults topath_includes_root_path=True. Does Django ASGI actually include root_path in path? If not, this test might pass but produce incorrect URLs in production. Worth verifying Django's ASGI behavior. - No test for
path_includes_root_path=False— The Starlette/FastAPI tests only exercise theTruepath (Starlette >= 0.33). The old behavior (False) has no dedicated test. - Edge case: empty path — When
path_includes_root_path=Trueandscope["path"]is empty, the URL would be just scheme+host with no path. Unlikely but worth noting.
Verdict
Solid PR. The core logic is correct and well-structured. The main risk is the Django default assumption — worth double-checking before merging.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a96d284. Configure here.
| # From Django 5.1 onwards, ASGI request.path is taken directly from scope["path"] without prepending scope["root_path"]. | ||
| # Assume that scope["path"] includes scope["root_path"] for these versions, as otherwise request.path is also incorrect. | ||
| # https://github.com/django/django/commit/bcd255cd5ca0a1e686d276cca71f45ec400d84a2 | ||
| path_includes_root_path=DJANGO_VERSION >= (5, 1), |
There was a problem hiding this comment.
Channels middleware missing path flag
Medium Severity
The Channels (<3) SentryAsgiMiddleware still relies on the new default path_includes_root_path=True, while the main Django ASGI handler sets it from DJANGO_VERSION. On Django before 5.1, mounted root_path can be omitted from reported request URLs and span attributes.
Reviewed by Cursor Bugbot for commit a96d284. Configure here.
| # From Django 5.1 onwards, ASGI request.path is taken directly from scope["path"] without prepending scope["root_path"]. | ||
| # Assume that scope["path"] includes scope["root_path"] for these versions, as otherwise request.path is also incorrect. | ||
| # https://github.com/django/django/commit/bcd255cd5ca0a1e686d276cca71f45ec400d84a2 | ||
| path_includes_root_path=DJANGO_VERSION >= (5, 1), |
There was a problem hiding this comment.
test_request_url fails on Django < 5.1 due to incorrect root_path setup
The new test_request_url test sets scope["path"] = "/root/nomessage" with scope["root_path"] = "/root" (path already includes root_path), but only skips for Django < 3.0. On Django < 5.1, path_includes_root_path=False causes the URL to be constructed as root_path + path = "/root/root/nomessage", which will fail the assertion == "/root/nomessage". Consider adding @pytest.mark.skipif(django.VERSION < (5, 1), ...) or adding a separate test case for the pre-5.1 path format.
Evidence
- Line 103 sets
path_includes_root_path=DJANGO_VERSION >= (5, 1), so Django < 5.1 getsFalse. _get_urlwithpath_includes_root_path=Falseconstructs URL asroot_path + path(/root+/root/nomessage=/root/root/nomessage).tests/integrations/django/asgi/test_asgi.py:1078setscomm.scope["root_path"] = "/root"with request path/root/nomessage— a Django 5.1+ style where path already contains root_path.- The test asserts
url.full == "/root/nomessage"but has noskipif django.VERSION < (5, 1)guard, onlyskipif django.VERSION < (3, 0). - Test would fail on Django 3.x–4.x where the code path produces
/root/root/nomessage.
Also found at 1 additional location
tests/integrations/django/asgi/test_asgi.py:1093-1101
Identified by Warden code-review · GNZ-9Q4
3 participants
Description
Add the
path_includes_root_pathboolean parameter to_get_url()and related functions.If the parameter is
True(the default),scope["path"]is considered to includescope["root_path"], wherescopeis the ASGI scope. Otherwise, the existing behavior that prependsscope["root_path"]when forming the url is preserved.Add tests to each ASGI-based integration. Some are compliant with the ASGI spec in which
scope["path"]includesscope["root_path"], while others are not. In particular, Starlite, LiterStar and old versions of Starlette and Quart are not compliant with the spec. In these cases,path_includes_root_path=Falseis set.Issues
Closes #6577
Reminders
uv run ruff.feat:,fix:,ref:,meta:)