Skip to content

Do not retry 4xx uploads #6618

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
+8 −0
Open

Do not retry 4xx uploads #6618

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
b4f4bd7
Do not retry 4xx uploads
dfed Oct 30, 2025
6426997
Changelog
dfed Oct 30, 2025
05fd99a
Correct PR number
dfed Oct 30, 2025
cf32770
exclude 429
dfed Oct 30, 2025
b81276c
record lost event
dfed Oct 30, 2025
580e603
don't rely on error
dfed Oct 30, 2025
496d7bf
do not double record
dfed Oct 30, 2025
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@
- Add layer class filtering for views used in multiple contexts (e.g., SwiftUI._UIGraphicsView)
- Improve transform calculations for views with custom anchor points
- Fix axis-aligned transform detection for optimized opaque view clipping
- Fix issue where a too-large breadcrumb would prevent future errors and messages from being uploaded (#6618)

### Improvements

Expand Down
7 changes: 7 additions & 0 deletions Sources/Sentry/SentryHttpTransport.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,13 @@ - (void)sendEnvelope:(SentryEnvelope *)envelope
return;
}

if (response.statusCode >= 400 && response.statusCode < 500 && response.statusCode != 429) {
SENTRY_LOG_DEBUG(@"Received 4xx response code: %li", (long)response.statusCode);
Comment on lines +417 to +418
Copy link
Author

@dfed dfed Oct 30, 2025

Choose a reason for hiding this comment

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

happy to make this specific to 400 status codes if we don't want to go broader, but generally speaking 4xx is "client screwed up and shouldn't try again"

Copy link
Member

@philipphofmann philipphofmann Oct 31, 2025

Choose a reason for hiding this comment

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

Yes, I agree that 400 usually means the client shouldn't retry again, but I still have to double-check if they might not have a good reason to keep the envelope.

The somewhat good news is that the SDK will delete the faulty envelope once the cache is full. The cache has a default max size of 100 envelopes. So when you log more than 100 errors, the SDK will send out all the other errors. Of course this isn't ideal, but at least the other envelopes end up in Sentry eventually.

Copy link
Author

@dfed dfed Oct 31, 2025

Choose a reason for hiding this comment

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

Sweet. Happy to update this PR. Let me know what you find!

Copy link
Member

@philipphofmann philipphofmann Nov 20, 2025

Choose a reason for hiding this comment

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

Hey @dfed, sorry that it took so long. So yes, we must drop envelopes when getting 400 and also 500 status codes and we must also record a client report with the reason send_error. When we get a 429, we must also drop the envelope, but we must not record a client report, as the backend already does this, and otherwise we would be double-counting.

We record client reports here

sentry-cocoa/Sources/Sentry/SentryHttpTransport.m

Lines 429 to 442 in f0d57ae

- (void)recordLostEventFor:(NSArray<SentryEnvelopeItem *> *)items
{
for (SentryEnvelopeItem *item in items) {
NSString *itemType = item.header.type;
// We don't want to record a lost event when it's a client report.
// It's fine to drop it silently.
if ([itemType isEqualToString:SentryEnvelopeItemTypes.clientReport]) {
continue;
}
SentryDataCategory category = sentryDataCategoryForEnvelopItemType(itemType);
[self recordLostEvent:category reason:kSentryDiscardReasonNetworkError];
[self recordLostSpans:item reason:kSentryDiscardReasonNetworkError];
}
}

We don't have the send_error category yet, so we need to add it here

sentry-cocoa/Sources/Sentry/include/SentryDiscardReason.h

Lines 3 to 21 in f0d57ae

/**
* A reason that defines why events were lost, see
* https://develop.sentry.dev/sdk/client-reports/#envelope-item-payload.
*/
typedef NS_ENUM(NSUInteger, SentryDiscardReason) {
kSentryDiscardReasonBeforeSend = 0,
kSentryDiscardReasonEventProcessor = 1,
kSentryDiscardReasonSampleRate = 2,
kSentryDiscardReasonNetworkError = 3,
kSentryDiscardReasonQueueOverflow = 4,
kSentryDiscardReasonCacheOverflow = 5,
kSentryDiscardReasonRateLimitBackoff = 6,
kSentryDiscardReasonInsufficientData = 7
};
static DEPRECATED_MSG_ATTRIBUTE(
"Use nameForSentryDiscardReason() instead.") NSString *_Nonnull const SentryDiscardReasonNames[]
= { @"before_send", @"event_processor", @"sample_rate", @"network_error", @"queue_overflow",
@"cache_overflow", @"ratelimit_backoff", @"insufficient_data" };

More info on client reports is here: https://develop.sentry.dev/sdk/telemetry/client-reports/ and this PR updates the develop docs on this topic getsentry/sentry-docs#15571

@dfed, I leave it up to you. Feel free to implement the changes and also add tests for it, or we can take it from here and push the PR over the finish line.

Copy link
Author

@dfed dfed Nov 20, 2025

Choose a reason for hiding this comment

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

Happy to let y'all take it over the line. Feel free to close this PR or push to it: whatever makes your lives easier.

// 4xx means the payload is bad and will not succeed on retry. Drop it on the floor and enable sending the next one.
[weakSelf deleteEnvelopeAndSendNext:envelopePath];
return;
}

SENTRY_LOG_DEBUG(@"Received non-200 response code: %li", (long)response.statusCode);
[weakSelf finishedSending];
}];
Expand Down