Repository navigation
fix: Read normalized rate limit headers - #3883
Conversation
Use lowercase keys when reading package:http response headers so category-specific rate limits are not missed. Fixes #3882 Co-Authored-By: Junior <noreply@github.com> Co-Authored-By: Gino Buenaflor <giancarlo.buenaflor@sentry.io>
Semver Impact of This PR🟢 Patch (bug fixes) 📋 Changelog PreviewThis is how your changes will appear in the changelog. FeaturesFlutter
Fixes
Dependencies
Internal Changes
🤖 This preview updates automatically when you update the PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3883 +/- ##
==========================================
+ Coverage 85.94% 87.82% +1.87%
==========================================
Files 240 346 +106
Lines 8767 12854 +4087
==========================================
+ Hits 7535 11289 +3754
- Misses 1232 1565 +333
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
Android Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 9993fa9 | 389.44 ms | 390.85 ms | 1.41 ms |
| c741aa5 | 360.67 ms | 367.86 ms | 7.19 ms |
| cdf371b | 367.64 ms | 377.02 ms | 9.38 ms |
| ebccb5b | 363.79 ms | 371.10 ms | 7.31 ms |
| 6287dc4 | 390.30 ms | 399.12 ms | 8.82 ms |
| 51520fc | 351.89 ms | 349.79 ms | -2.10 ms |
| 3fe9e80 | 362.67 ms | 376.19 ms | 13.52 ms |
| d8f332d | 365.31 ms | 349.98 ms | -15.33 ms |
| 819c1e7 | 449.80 ms | 442.98 ms | -6.82 ms |
| 29e8ebe | 389.91 ms | 395.76 ms | 5.84 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 9993fa9 | 14.55 MiB | 15.87 MiB | 1.31 MiB |
| c741aa5 | 14.55 MiB | 15.87 MiB | 1.31 MiB |
| cdf371b | 13.93 MiB | 15.18 MiB | 1.25 MiB |
| ebccb5b | 13.93 MiB | 15.18 MiB | 1.25 MiB |
| 6287dc4 | 13.93 MiB | 15.18 MiB | 1.25 MiB |
| 51520fc | 13.93 MiB | 15.18 MiB | 1.25 MiB |
| 3fe9e80 | 14.55 MiB | 15.87 MiB | 1.31 MiB |
| d8f332d | 14.55 MiB | 15.87 MiB | 1.31 MiB |
| 819c1e7 | 13.93 MiB | 15.00 MiB | 1.06 MiB |
| 29e8ebe | 13.93 MiB | 15.06 MiB | 1.13 MiB |
iOS Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| d0371bb | 1258.85 ms | 1259.44 ms | 0.60 ms |
| dc53d48 | 1252.06 ms | 1255.61 ms | 3.55 ms |
| c26ed0a | 1244.11 ms | 1263.85 ms | 19.75 ms |
| 81f83eb | 1259.53 ms | 1273.39 ms | 13.86 ms |
| 40c8f93 | 1234.27 ms | 1261.98 ms | 27.71 ms |
| dbd526b | 1244.78 ms | 1259.02 ms | 14.24 ms |
| 73dca78 | 1246.65 ms | 1265.42 ms | 18.76 ms |
| 737ef5d | 1255.65 ms | 1252.24 ms | -3.41 ms |
| 3801d52 | 1267.76 ms | 1266.10 ms | -1.65 ms |
| 5367726 | 1211.00 ms | 1216.73 ms | 5.73 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| d0371bb | 5.73 MiB | 6.18 MiB | 464.41 KiB |
| dc53d48 | 5.65 MiB | 6.09 MiB | 446.96 KiB |
| c26ed0a | 5.53 MiB | 5.97 MiB | 453.76 KiB |
| 81f83eb | 7.86 MiB | 9.44 MiB | 1.58 MiB |
| 40c8f93 | 5.53 MiB | 6.00 MiB | 479.94 KiB |
| dbd526b | 7.86 MiB | 9.44 MiB | 1.58 MiB |
| 73dca78 | 7.86 MiB | 9.44 MiB | 1.58 MiB |
| 737ef5d | 5.73 MiB | 6.18 MiB | 466.16 KiB |
| 3801d52 | 5.73 MiB | 6.17 MiB | 455.54 KiB |
| 5367726 | 5.73 MiB | 6.17 MiB | 455.87 KiB |
There was a problem hiding this comment.
Pull request overview
This PR fixes rate-limit header parsing in the Dart HTTP transport by reading package:http response headers via their normalized lowercase keys, preventing missed headers (notably under HTTP/2+ semantics) and avoiding unintended all-category rate limiting.
Changes:
- Read
retry-afterandx-sentry-rate-limitsfromResponse.headersusing lowercase keys inHttpTransport. - Update transport tests to provide lowercase header names, matching
package:http’s header-map contract.
Standards
No issues found.
Standards summary: 0 findings; worst issue: none.
Spec
Matches the PR description and Issue #3882: ensures rate-limit headers are read case-insensitively (via normalized lowercase keys) and updates tests accordingly.
Spec summary: 0 findings; worst issue: none.
Correctness
The change aligns with package:http behavior (normalized lowercase header keys) and fixes previously fragile header lookups without introducing new logic paths.
Correctness summary: 0 findings; worst issue: none.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/dart/lib/src/transport/http_transport.dart | Switch header lookups to lowercase keys for correct rate-limit parsing. |
| packages/dart/test/transport/http_transport_test.dart | Adjust tests to use lowercase response header names to match package:http behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
buenaflor
left a comment
There was a problem hiding this comment.
LGTM, failing CI is not because of this PR
Read
package:httpresponse headers using their normalized lowercase keys. This preserves category-specific Sentry rate limits instead of falling back to an all-category limit when header names arrive lowercased.The transport tests now supply lowercase
retry-afterandx-sentry-rate-limitsheaders, matching the response map contract and covering both lookup paths.Automated Dart tests were not run because this environment does not include Dart or FVM.
git diff --checkpasses.Fixes #3882
Requested by Gino Buenaflor via Junior.
--
View Junior Session in Sentry