diff --git a/CHANGELOG.md b/CHANGELOG.md index 67344b931c..5eb188085c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Unreleased +- Use a default value of 60 seconds if a `Retry-After` header is not present. ([#1537](https://github.com/getsentry/sentry-dotnet/pull/1537)) ### Fixes - Ignore zero properties for MemoryInfo ([#1531](https://github.com/getsentry/sentry-dotnet/pull/1531)) diff --git a/src/Sentry/Internal/Http/RetryAfterHandler.cs b/src/Sentry/Internal/Http/RetryAfterHandler.cs index a645bfc8f5..2682476088 100644 --- a/src/Sentry/Internal/Http/RetryAfterHandler.cs +++ b/src/Sentry/Internal/Http/RetryAfterHandler.cs @@ -19,6 +19,7 @@ internal class RetryAfterHandler : DelegatingHandler private readonly ISystemClock _clock; private const HttpStatusCode TooManyRequests = (HttpStatusCode)429; + internal static readonly TimeSpan DefaultRetryAfterDelay = TimeSpan.FromSeconds(60); private long _retryAfterUtcTicks; internal long RetryAfterUtcTicks => _retryAfterUtcTicks; @@ -63,31 +64,40 @@ protected override async Task SendAsync( var response = await base.SendAsync(request, cancellationToken).ConfigureAwait(false); - if (response.StatusCode == TooManyRequests && response.Headers != null) + if (response.StatusCode == TooManyRequests) { - if (response.Headers.RetryAfter != null) + var retryAfterTimestamp = GetRetryAfterTimestamp(response); + _ = Interlocked.Exchange(ref _retryAfterUtcTicks, retryAfterTimestamp.UtcTicks); + } + + return response; + } + + private DateTimeOffset GetRetryAfterTimestamp(HttpResponseMessage response) + { + if (response.Headers.RetryAfter != null) + { + if (response.Headers.RetryAfter.Delta is { } delta) { - if (response.Headers.RetryAfter.Delta != null) - { - var retryAfterUtc = _clock.GetUtcNow() + response.Headers.RetryAfter.Delta.Value; - _ = Interlocked.Exchange(ref _retryAfterUtcTicks, retryAfterUtc.UtcTicks); - } - else if (response.Headers.RetryAfter.Date != null) - { - _ = Interlocked.Exchange(ref _retryAfterUtcTicks, response.Headers.RetryAfter.Date.Value.UtcTicks); - } + return _clock.GetUtcNow() + delta; } - // Sentry was sending floating point numbers which are not handled by RetryConditionHeaderValue - // To be compatible with older versions of sentry on premise: https://github.com/getsentry/sentry/issues/7919 - else if (response.Headers.TryGetValues("Retry-After", out var values) - && double.TryParse(values?.FirstOrDefault(), out var retryAfterSeconds)) + + if (response.Headers.RetryAfter.Date is { } date) { - var retryAfterSpan = TimeSpan.FromSeconds(retryAfterSeconds); - _ = Interlocked.Exchange(ref _retryAfterUtcTicks, _clock.GetUtcNow().AddTicks(retryAfterSpan.Ticks).UtcTicks); + return date; } } - return response; + // Sentry was sending floating point numbers which are not handled by RetryConditionHeaderValue + // To be compatible with older versions of sentry on premise: https://github.com/getsentry/sentry/issues/7919 + if (response.Headers.TryGetValues("Retry-After", out var values) + && double.TryParse(values.FirstOrDefault(), out var retryAfterSeconds)) + { + return _clock.GetUtcNow().AddTicks((long)(retryAfterSeconds * TimeSpan.TicksPerSecond)); + } + + // No retry header was sent. Use the default retry delay. + return _clock.GetUtcNow() + DefaultRetryAfterDelay; } } } diff --git a/test/Sentry.Tests/Internals/Http/RetryAfterHandlerTests.cs b/test/Sentry.Tests/Internals/Http/RetryAfterHandlerTests.cs index de36d6fb5e..3b4f569fe6 100644 --- a/test/Sentry.Tests/Internals/Http/RetryAfterHandlerTests.cs +++ b/test/Sentry.Tests/Internals/Http/RetryAfterHandlerTests.cs @@ -54,7 +54,7 @@ public async Task SendAsync_TooManyRequestsWithoutRetryAfterHeader_RetryAfterNot var actual = await invoker.SendAsync(new HttpRequestMessage(HttpMethod.Get, "/"), None); Assert.Equal(expected, actual); - Assert.Equal(0, _fixture.Sut.RetryAfterUtcTicks); + Assert.Equal((_fixture.TimeReturned + RetryAfterHandler.DefaultRetryAfterDelay).UtcTicks, _fixture.Sut.RetryAfterUtcTicks); Assert.True(_fixture.StubHandler.SendAsyncCalled); } @@ -104,7 +104,8 @@ public async Task SendAsync_TooManyRequestsWithRetryAfterHeaderFloat_RetryAfterS var actual = await invoker.SendAsync(new HttpRequestMessage(HttpMethod.Get, "/"), None); Assert.Equal(expected, actual); - Assert.Equal((_fixture.TimeReturned + TimeSpan.FromSeconds(floating)).UtcTicks, _fixture.Sut.RetryAfterUtcTicks); + var expectedTime = _fixture.TimeReturned.AddTicks((long)(floating * TimeSpan.TicksPerSecond)); + Assert.Equal(expectedTime.UtcTicks, _fixture.Sut.RetryAfterUtcTicks); Assert.True(_fixture.StubHandler.SendAsyncCalled); } @@ -127,7 +128,8 @@ public async Task SendAsync_TooManyRequestsWithRetryAfterHeaderFloat_SecondReque var actual = await invoker.SendAsync(new HttpRequestMessage(HttpMethod.Get, "/"), None); Assert.Equal(TooManyRequests, actual.StatusCode); - Assert.Equal((_fixture.TimeReturned + TimeSpan.FromSeconds(floating)).UtcTicks, _fixture.Sut.RetryAfterUtcTicks); + var expectedTime = _fixture.TimeReturned.AddTicks((long)(floating * TimeSpan.TicksPerSecond)); + Assert.Equal(expectedTime.UtcTicks, _fixture.Sut.RetryAfterUtcTicks); Assert.False(_fixture.StubHandler.SendAsyncCalled); }