Do not retry 4xx uploads
:scroll: Description
If an upload receives a 4xx error, Sentry should assume that a retry will not succeed and drop it on the floor. Currently Sentry will retry the upload (seemingly forever), which means that a 4xx error on upload will prevent future uploads from succeeding.
:bulb: Motivation and Context
This PR represents the shortest-path resolution to #6612. I'd prefer a solution where a 4xx error didn't cause a silent failure, or a solution where breadcrumb were automatically truncated. But this change is simple and unsticks existing clients, which seems like a good thing.
:green_heart: How did you test it?
I didn't.
:pencil: Checklist
You have to check all boxes before merging:
- [ ] I added tests to verify the changes.
- [x] No new PII added or SDK only sends newly added PII if
sendDefaultPIIis enabled. - [x] I updated the docs if needed.
- [x] I updated the wizard if needed.
- [ ] Review from the native team if needed.
- [x] No breaking change or entry added to the changelog.
- [x] No breaking change for hybrid SDKs or communicated to hybrid SDKs.
@philipphofmann are you going to pick this up?
@philprime, yes, I plan to next week. I mean, you or anybody else can also pick it up if you have bandwidth.
I manually tested capturing a message with the iOS-Swift sample app
let message = Data(repeating: 0, count: 1024*1024*2) // 2 MB message
let string = message.base64EncodedString()
let eventId = SentrySDK.capture(message: string)
And the transport successfully deleted the envelope when getting a 400 from Relay.
Doing the same thing on the main branch, led to filling up the chache until the faulty envelope with the too large message finally gets deleted due to cache overflow.
Codecov Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 84.922%. Comparing base (8bed6db) to head (4af25a0).
:warning: Report is 1 commits behind head on main.
:white_check_mark: All tests successful. No failed tests found.
Additional details and impacted files
@@ Coverage Diff @@
## main #6618 +/- ##
=============================================
+ Coverage 84.885% 84.922% +0.036%
=============================================
Files 457 457
Lines 27616 27624 +8
Branches 12140 12150 +10
=============================================
+ Hits 23442 23459 +17
+ Misses 4131 4126 -5
+ Partials 43 39 -4
| Files with missing lines | Coverage Δ | |
|---|---|---|
| Sources/Sentry/SentryDiscardReasonMapper.m | 100.000% <100.000%> (ø) |
|
| Sources/Sentry/SentryHttpTransport.m | 96.744% <100.000%> (+0.093%) |
:arrow_up: |
... and 4 files with indirect coverage changes
Continue to review full report in Codecov by Sentry.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 8bed6db...4af25a0. Read the comment docs.
Back to draft because some tests are failing.
Thank you all for getting this over the line! Excited for this 🚀