Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 5 additions & 2 deletions src/Sentry/Internal/Http/RetryAfterHandler.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
namespace Sentry.Internal.Http;

/// <summary>
/// Retry After Handler which short-circuit requests following an HTTP 429.
/// Retry After Handler which short-circuits requests following an HTTP 429 that carries no per-category
/// rate limits. Responses that do carry them are left to the transport to apply per envelope item.
/// </summary>
/// <seealso href="https://tools.ietf.org/html/rfc6585#section-4" />
/// <seealso href="https://develop.sentry.dev/sdk/overview/#writing-an-sdk"/>
Expand Down Expand Up @@ -62,7 +63,9 @@ protected override async Task<HttpResponseMessage> SendAsync(

var response = await base.SendAsync(request, cancellationToken).ConfigureAwait(false);

if (response.StatusCode == TooManyRequests)
// 429 responses carrying a `X-Sentry-Rate-Limits` header are handled separately.
// See https://github.com/getsentry/sentry-dotnet/pull/5482
if (response.StatusCode == TooManyRequests && !response.Headers.Contains("X-Sentry-Rate-Limits"))
{
var retryAfterTimestamp = GetRetryAfterTimestamp(response);
_ = Interlocked.Exchange(ref _retryAfterUtcTicks, retryAfterTimestamp.UtcTicks);
Expand Down
43 changes: 43 additions & 0 deletions test/Sentry.Tests/Internals/Http/HttpTransportTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1072,4 +1072,47 @@ public async Task SendEnvelopeAsync_Response429WithTextMessage_LogsWarning()
e.Level == SentryLevel.Error &&
e.Message.Contains("Sentry rejected the envelope"));
}

[Fact]
public async Task SendEnvelopeAsync_TransactionRateLimited_ErrorEnvelopeStillSent()
{
// Arrange
// Note this goes through DefaultSentryHttpClientFactory, so that the RetryAfterHandler is part of the
// pipeline, as it is in production. See https://github.com/getsentry/sentry-dotnet/issues/3947
var requestCount = 0;
using var httpHandler = new FakeHttpMessageHandler(() =>
{
requestCount++;
return requestCount == 1
? SentryResponses.GetRateLimitResponse(
"60:transaction;profile;span:organization:transaction_usage_exceeded, " +
"60:transaction:project:project_quota_transaction_usage_exceeded")
: SentryResponses.GetOkResponse();
});

var options = new SentryOptions
{
Dsn = ValidDsn,
DiagnosticLogger = _testOutputLogger,
Debug = true,
CreateHttpMessageHandler = () => httpHandler
};

var httpTransport = new HttpTransport(
options,
new DefaultSentryHttpClientFactory().Create(options),
null,
clock: _fakeClock);

// Act
// Transactions are over quota...
var transaction = new SentryTransaction("test", "test.op") { IsSampled = true };
await httpTransport.SendEnvelopeAsync(Envelope.FromTransaction(transaction));

// ...but errors are not, so this one should still go out
await httpTransport.SendEnvelopeAsync(Envelope.FromEvent(new SentryEvent()));

// Assert
requestCount.Should().Be(2);
}
}
42 changes: 42 additions & 0 deletions test/Sentry.Tests/Internals/Http/RetryAfterHandlerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,48 @@ public async Task SendAsync_TooManyRequestsWithoutRetryAfterHeader_RetryAfterNot
Assert.True(_fixture.StubHandler.SendAsyncCalled);
}

[Fact]
public async Task SendAsync_TooManyRequestsWithCategoryRateLimits_RetryAfterNotSet()
{
// Per-category limits are applied by the transport, per envelope item. Backing off globally here would
// stop us sending categories that aren't rate limited at all. See https://github.com/getsentry/sentry-dotnet/issues/3947
var expected = new HttpResponseMessage(TooManyRequests);
expected.Headers.Add("X-Sentry-Rate-Limits", "60:transaction;profile;span:organization:transaction_usage_exceeded");
expected.Headers.RetryAfter = new RetryConditionHeaderValue(TimeSpan.FromSeconds(60));
_fixture.StubHandler.SendAsyncFunc = (_, _) => expected;

var invoker = _fixture.GetInvoker();
var actual = await invoker.SendAsync(new HttpRequestMessage(HttpMethod.Get, "/"), None);

Assert.Equal(expected, actual);
Assert.Equal(0, _fixture.Sut.RetryAfterUtcTicks);
Assert.True(_fixture.StubHandler.SendAsyncCalled);
}

[Fact]
public async Task SendAsync_TooManyRequestsWithCategoryRateLimits_SecondRequestIsNotThrottled()
{
var rateLimited = new HttpResponseMessage(TooManyRequests);
rateLimited.Headers.Add("X-Sentry-Rate-Limits", "60:transaction;profile;span:organization:transaction_usage_exceeded");
_fixture.StubHandler.SendAsyncFunc = (_, _) => rateLimited;

var invoker = _fixture.GetInvoker();

// First call: rate limited for transactions only
_ = await invoker.SendAsync(new HttpRequestMessage(HttpMethod.Get, "/"), None);
Assert.True(_fixture.StubHandler.SendAsyncCalled);

// Change the response: OK
var expected = new HttpResponseMessage(HttpStatusCode.OK);
_fixture.StubHandler.SendAsyncFunc = (_, _) => expected;
_fixture.StubHandler.SendAsyncCalled = false;

var actual = await invoker.SendAsync(new HttpRequestMessage(HttpMethod.Get, "/"), None);

Assert.Equal(expected, actual);
Assert.True(_fixture.StubHandler.SendAsyncCalled);
}

[Fact]
public async Task SendAsync_TooManyRequestsWithRetryAfterHeaderDate_RetryAfterSet()
{
Expand Down
Loading