diff --git a/CHANGELOG.md b/CHANGELOG.md index 7dcf22728d..b9cc99f776 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,8 @@ # Changelog -## vNext +## Unreleased +- Handle non-json error response messages on HttpTransport. (#690) @lucas-zimerman - Fix deadlock on missing ConfigureAwait into foreach loops. (#694) @lucas-zimerman ## 3.0.0-alpha.8 diff --git a/src/Sentry/Internal/Http/HttpTransport.cs b/src/Sentry/Internal/Http/HttpTransport.cs index 8c90213608..a0144d286f 100644 --- a/src/Sentry/Internal/Http/HttpTransport.cs +++ b/src/Sentry/Internal/Http/HttpTransport.cs @@ -111,25 +111,41 @@ public async Task SendEnvelopeAsync(Envelope envelope, CancellationToken cancell } else if (_options.DiagnosticLogger?.IsEnabled(SentryLevel.Error) == true) { - var responseJson = await response.Content.ReadAsJsonAsync(cancellationToken).ConfigureAwait(false); - - var errorMessage = - responseJson.GetPropertyOrNull("detail")?.GetString() - ?? DefaultErrorMessage; - - var errorCauses = - responseJson.GetPropertyOrNull("causes")?.EnumerateArray().Select(j => j.GetString()).ToArray() - ?? Array.Empty(); - - _options.DiagnosticLogger?.Log( - SentryLevel.Error, - "Sentry rejected the envelope {0}. Status code: {1}. Error detail: {2}. Error causes: {3}.", - null, - processedEnvelope.TryGetEventId(), - response.StatusCode, - errorMessage, - string.Join(", ", errorCauses) - ); + if (response.Content.Headers.ContentType?.MediaType is "application/json") + { + var responseJson = await response.Content.ReadAsJsonAsync(cancellationToken).ConfigureAwait(false); + + var errorMessage = + responseJson.GetPropertyOrNull("detail")?.GetString() + ?? DefaultErrorMessage; + + var errorCauses = + responseJson.GetPropertyOrNull("causes")?.EnumerateArray().Select(j => j.GetString()).ToArray() + ?? Array.Empty(); + + _options.DiagnosticLogger?.Log( + SentryLevel.Error, + "Sentry rejected the envelope {0}. Status code: {1}. Error detail: {2}. Error causes: {3}.", + null, + processedEnvelope.TryGetEventId(), + response.StatusCode, + errorMessage, + string.Join(", ", errorCauses) + ); + } + else + { + var responseString = await response.Content.ReadAsStringAsync().ConfigureAwait(false); + + _options.DiagnosticLogger?.Log( + SentryLevel.Error, + "Sentry rejected the envelope {0}. Status code: {1}. Error detail: {2}.", + null, + processedEnvelope.TryGetEventId(), + response.StatusCode, + responseString + ); + } } } diff --git a/test/Sentry.Testing/SentryResponses.cs b/test/Sentry.Testing/SentryResponses.cs index 9a63f54dee..59368c19c8 100644 --- a/test/Sentry.Testing/SentryResponses.cs +++ b/test/Sentry.Testing/SentryResponses.cs @@ -1,6 +1,7 @@ using System; using System.Net; using System.Net.Http; +using System.Text; using System.Text.Json; namespace Sentry.Testing @@ -20,15 +21,21 @@ public static HttpResponseMessage GetOkResponse() Content = GetOkContent() }; - public static HttpResponseMessage GetErrorResponse(HttpStatusCode code, string detail, string[] causes = null) + public static HttpResponseMessage GetJsonErrorResponse(HttpStatusCode code, string detail, string[] causes = null) { var responseContent = causes != null ? JsonSerializer.Serialize(new {detail, causes}) : JsonSerializer.Serialize(new {detail}); - return new HttpResponseMessage(code) {Content = new StringContent(responseContent)}; + return new HttpResponseMessage(code) {Content = new StringContent(responseContent, Encoding.UTF8, "application/json") }; } + public static HttpResponseMessage GetTextErrorResponse(HttpStatusCode code, string detail) + => new HttpResponseMessage(code) + { + Content = new StringContent(detail) + }; + public static HttpResponseMessage GetRateLimitResponse(string rateLimitHeaderValue) { return new((HttpStatusCode)429) diff --git a/test/Sentry.Tests/Internals/Http/HttpTransportTests.cs b/test/Sentry.Tests/Internals/Http/HttpTransportTests.cs index def15ad573..b0d461a238 100644 --- a/test/Sentry.Tests/Internals/Http/HttpTransportTests.cs +++ b/test/Sentry.Tests/Internals/Http/HttpTransportTests.cs @@ -52,7 +52,7 @@ await httpHandler } [Fact] - public async Task SendEnvelopeAsync_ResponseNotOkWithMessage_LogsError() + public async Task SendEnvelopeAsync_ResponseNotOkWithJsonMessage_LogsError() { // Arrange const HttpStatusCode expectedCode = HttpStatusCode.BadGateway; @@ -63,7 +63,7 @@ public async Task SendEnvelopeAsync_ResponseNotOkWithMessage_LogsError() var httpHandler = Substitute.For(); httpHandler.VerifiableSendAsync(Arg.Any(), Arg.Any()) - .Returns(_ => SentryResponses.GetErrorResponse(expectedCode, expectedMessage, expectedCauses)); + .Returns(_ => SentryResponses.GetJsonErrorResponse(expectedCode, expectedMessage, expectedCauses)); var logger = new AccumulativeDiagnosticLogger(); @@ -94,6 +94,46 @@ public async Task SendEnvelopeAsync_ResponseNotOkWithMessage_LogsError() ).Should().BeTrue(); } + [Fact] + public async Task SendEnvelopeAsync_ResponseNotOkWithStringMessage_LogsError() + { + // Arrange + const HttpStatusCode expectedCode = HttpStatusCode.RequestEntityTooLarge; + const string expectedMessage = "413 Request Entity Too Large"; + + var httpHandler = Substitute.For(); + + _ = httpHandler.VerifiableSendAsync(Arg.Any(), Arg.Any()) + .Returns(_ => SentryResponses.GetTextErrorResponse(expectedCode, expectedMessage)); + + var logger = new AccumulativeDiagnosticLogger(); + + var httpTransport = new HttpTransport( + new SentryOptions + { + Dsn = DsnSamples.ValidDsnWithSecret, + Debug = true, + DiagnosticLogger = logger + }, + new HttpClient(httpHandler) + ); + + var envelope = Envelope.FromEvent(new SentryEvent()); + + // Act + await httpTransport.SendEnvelopeAsync(envelope); + + // Assert + _ = logger.Entries.Any(e => + e.Level == SentryLevel.Error && + e.Message == "Sentry rejected the envelope {0}. Status code: {1}. Error detail: {2}." && + e.Exception == null && + e.Args[0].ToString() == envelope.TryGetEventId().ToString() && + e.Args[1].ToString() == expectedCode.ToString() && + e.Args[2].ToString() == expectedMessage + ).Should().BeTrue(); + } + [Fact] public async Task SendEnvelopeAsync_ResponseNotOkNoMessage_LogsError() { @@ -103,7 +143,7 @@ public async Task SendEnvelopeAsync_ResponseNotOkNoMessage_LogsError() var httpHandler = Substitute.For(); httpHandler.VerifiableSendAsync(Arg.Any(), Arg.Any()) - .Returns(_ => SentryResponses.GetErrorResponse(expectedCode, null)); + .Returns(_ => SentryResponses.GetJsonErrorResponse(expectedCode, null)); var logger = new AccumulativeDiagnosticLogger();