diff --git a/CHANGELOG.md b/CHANGELOG.md index a415874c1f..78d8179867 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixes - OTel activities that are marked as not recorded are no longer sent to Sentry ([#3890](https://github.com/getsentry/sentry-dotnet/pull/3890)) +- Fixed envelopes with oversized attachments getting stuck in __processing ([#3938](https://github.com/getsentry/sentry-dotnet/pull/3938)) - Unknown stack frames in profiles on .NET 8+ ([#3942](https://github.com/getsentry/sentry-dotnet/pull/3942)) - Deduplicate profiling stack frames ([#3941](https://github.com/getsentry/sentry-dotnet/pull/3941)) diff --git a/src/Sentry/Internal/DiscardReason.cs b/src/Sentry/Internal/DiscardReason.cs index d132f761ba..11a35fa2a3 100644 --- a/src/Sentry/Internal/DiscardReason.cs +++ b/src/Sentry/Internal/DiscardReason.cs @@ -4,6 +4,7 @@ namespace Sentry.Internal; { // See https://develop.sentry.dev/sdk/client-reports/ for list public static DiscardReason BeforeSend = new("before_send"); + public static DiscardReason BufferOverflow = new("buffer_overflow"); public static DiscardReason CacheOverflow = new("cache_overflow"); public static DiscardReason EventProcessor = new("event_processor"); public static DiscardReason NetworkError = new("network_error"); diff --git a/src/Sentry/Internal/Http/CachingTransport.cs b/src/Sentry/Internal/Http/CachingTransport.cs index f6d4c9b856..b5ed2cb34a 100644 --- a/src/Sentry/Internal/Http/CachingTransport.cs +++ b/src/Sentry/Internal/Http/CachingTransport.cs @@ -300,13 +300,22 @@ private async Task ProcessCacheAsync(CancellationToken cancellation) } } - private static bool IsNetworkError(Exception exception) => + private static bool IsNetworkUnavailableError(Exception exception) => exception switch { + // TODO: Could we make this more specific? Lots of these errors are unrelated to network availability. HttpRequestException or WebException or IOException or SocketException => true, _ => false }; + private static bool IsRejectedByServer(Exception ex) + { + // When envelopes are too big, the server will reset the connection as soon as the maximum size is exceeded + // (it doesn't wait for us to finish sending the whole envelope). + return ex is SocketException { ErrorCode: 32 /* Broken pipe */ } + || (ex.InnerException is { } innerException && IsRejectedByServer(innerException)); + } + private async Task InnerProcessCacheAsync(string file, CancellationToken cancellation) { if (_options.NetworkStatusListener is { Online: false } listener) @@ -346,8 +355,15 @@ private async Task InnerProcessCacheAsync(string file, CancellationToken cancell // Let the worker catch, log, wait a bit and retry. throw; } - catch (Exception ex) when (IsNetworkError(ex)) + catch (Exception ex) when (IsRejectedByServer(ex)) + { + _options.ClientReportRecorder.RecordDiscardedEvents(DiscardReason.BufferOverflow, envelope); + _options.LogError(ex, "Failed to send cached envelope: {0}. The envelope is likely too big and will be discarded.", file); + } + catch (Exception ex) when (IsNetworkUnavailableError(ex)) { + // TODO: Envelopes could end up in an infinite loop here. We should consider implementing some + // kind backoff strategy and a retry limit... then drop the envelopes if the limit is exceeded. if (_options.NetworkStatusListener is PollingNetworkStatusListener pollingListener) { pollingListener.Online = false; diff --git a/test/Sentry.Tests/Internals/Http/CachingTransportTests.cs b/test/Sentry.Tests/Internals/Http/CachingTransportTests.cs index 05df93f02b..e903b7f3ca 100644 --- a/test/Sentry.Tests/Internals/Http/CachingTransportTests.cs +++ b/test/Sentry.Tests/Internals/Http/CachingTransportTests.cs @@ -711,6 +711,43 @@ public async Task TransportResumesWhenNetworkComesBackOnline() envelopes.Should().NotBeEmpty(); } + [Fact] + public async Task FlushAsync_RejectedByServer_DiscardsEnvelope() + { + // Arrange + var listener = Substitute.For(); + listener.Online.Returns(_ => true); + + using var cacheDirectory = new TempDirectory(); + var options = new SentryOptions + { + Dsn = ValidDsn, + DiagnosticLogger = _logger, + Debug = true, + CacheDirectoryPath = cacheDirectory.Path, + NetworkStatusListener = listener, + ClientReportRecorder = Substitute.For() + }; + + using var envelope = Envelope.FromEvent(new SentryEvent()); + + var innerTransport = Substitute.For(); + innerTransport.SendEnvelopeAsync(Arg.Any(), Arg.Any()) + .Returns(_ => throw new SocketException(32 /* Bad pipe exception */)); + await using var transport = CachingTransport.Create(innerTransport, options, startWorker: false); + + // Act + await transport.SendEnvelopeAsync(envelope); + await transport.FlushAsync(); + + // Assert + foreach (var item in envelope.Items) + { + options.ClientReportRecorder.Received(1) + .RecordDiscardedEvent(DiscardReason.BufferOverflow, item.DataCategory); + } + } + [Fact] public async Task DoesntWriteSentAtHeaderToCacheFile() {