Return response body in user facing logs when encountering 5xx responses

RELEASENOTES.md

@@ -0,0 +1 @@
+- BBS2GH: When a Bitbucket Server API request fails with a 5xx response, the response body is now included in the standard log output (alongside the generic error message) to make troubleshooting easier without needing to inspect the verbose log.

src/Octoshift/Services/BbsClient.cs

@@ -85,7 +85,7 @@ private async Task<string> SendAsync(HttpMethod httpMethod, string url, object b
         }
 
         using var payload = body?.ToJson().ToStringContent();
-        var response = httpMethod.ToString() switch
+        using var response = httpMethod.ToString() switch
         {
             "GET" => await _httpClient.GetAsync(url),
             "DELETE" => await _httpClient.DeleteAsync(url),
@@ -97,6 +97,16 @@ private async Task<string> SendAsync(HttpMethod httpMethod, string url, object b
         var content = await response.Content.ReadAsStringAsync();
         _log.LogVerbose($"RESPONSE ({response.StatusCode}): {content}");
 
+        if (!response.IsSuccessStatusCode && (int)response.StatusCode >= 500)
+        {
+            var serverErrorException = new HttpRequestException(
+                $"Response status code does not indicate success: {(int)response.StatusCode} ({response.ReasonPhrase}).",
+                null,
+                response.StatusCode);
+            serverErrorException.Data["ResponseBody"] = content;
+            throw serverErrorException;
+        }
+
         response.EnsureSuccessStatusCode();
 
         return content;

src/Octoshift/Services/OctoLogger.cs

@@ -146,7 +146,18 @@ public virtual void LogError(Exception ex)
         lock (_mutex)
         {
             var verboseMessage = ex is HttpRequestException httpEx ? $"[HTTP ERROR {(int?)httpEx.StatusCode}] {ex}" : ex.ToString();
-            var logMessage = Verbose ? verboseMessage : ex is OctoshiftCliException ? ex.Message : GENERIC_ERROR_MESSAGE;
+            var responseBody = ex.Data["ResponseBody"] as string;
+            if (responseBody is not null)
+            {
+                verboseMessage = $"{verboseMessage}{Environment.NewLine}Response: {responseBody}";
+            }
+            var logMessage = Verbose
+                ? verboseMessage
+                : ex is OctoshiftCliException
+                    ? ex.Message
+                    : responseBody is not null
+                        ? $"{GENERIC_ERROR_MESSAGE}{Environment.NewLine}Response: {responseBody}"
+                        : GENERIC_ERROR_MESSAGE;
 
             var output = Redact(FormatMessage(logMessage, LogLevel.ERROR));
 

src/OctoshiftCLI.Tests/Octoshift/Services/BbsClientTests.cs

@@ -442,19 +442,28 @@ public async Task GetAllAsync_Throws_HttpRequestException_On_Non_Success_Respons
         const string url = "http://example.com/resource";
 
         var firstResponseContent = new { isLastPage = false, nextPageStart = 2, values = Array.Empty<object>() }.ToJson();
-        using var firstResponse = new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent(firstResponseContent) };
-
-        using var secondResponse = new HttpResponseMessage(HttpStatusCode.InternalServerError);
 
         var handlerMock = new Mock<HttpMessageHandler>();
 
         // first request
         const string firstRequestUrl = $"{url}?start=0&limit=100";
-        MockHttpHandler(req => req.Method == HttpMethod.Get && req.RequestUri!.ToString() == firstRequestUrl, firstResponse, handlerMock);
+        handlerMock
+            .Protected()
+            .Setup<Task<HttpResponseMessage>>(
+                "SendAsync",
+                ItExpr.Is<HttpRequestMessage>(req => req.Method == HttpMethod.Get && req.RequestUri!.ToString() == firstRequestUrl),
+                ItExpr.IsAny<CancellationToken>())
+            .ReturnsAsync(() => new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent(firstResponseContent) });
 
-        // second request
+        // second request - return a fresh response each time so retries don't reuse a disposed instance
         const string secondRequestUrl = $"{url}?start=2&limit=100";
-        MockHttpHandler(req => req.Method == HttpMethod.Get && req.RequestUri!.ToString() == secondRequestUrl, secondResponse, handlerMock);
+        handlerMock
+            .Protected()
+            .Setup<Task<HttpResponseMessage>>(
+                "SendAsync",
+                ItExpr.Is<HttpRequestMessage>(req => req.Method == HttpMethod.Get && req.RequestUri!.ToString() == secondRequestUrl),
+                ItExpr.IsAny<CancellationToken>())
+            .ReturnsAsync(() => new HttpResponseMessage(HttpStatusCode.InternalServerError));
 
         using var httpClient = new HttpClient(handlerMock.Object);
         var bbsClient = new BbsClient(_mockOctoLogger.Object, httpClient, null, _retryPolicy);
@@ -466,6 +475,34 @@ await bbsClient
             .ThrowExactlyAsync<HttpRequestException>();
     }
 
+    [Fact]
+    public async Task SendAsync_Includes_Response_Body_In_Exception_Data_On_5xx()
+    {
+        // Arrange
+        const string url = "http://example.com/resource";
+        const string responseBody = "{\"errors\":[{\"message\":\"Could not start migration job\"}]}";
+
+        using var response = new HttpResponseMessage(HttpStatusCode.ServiceUnavailable)
+        {
+            Content = new StringContent(responseBody)
+        };
+
+        var handlerMock = MockHttpHandler(req => req.Method == HttpMethod.Post, response);
+
+        using var httpClient = new HttpClient(handlerMock.Object);
+        var bbsClient = new BbsClient(_mockOctoLogger.Object, httpClient, null, _retryPolicy);
+
+        // Act
+        var thrown = await bbsClient
+            .Invoking(async x => await x.PostAsync(url, _rawRequestBody))
+            .Should()
+            .ThrowExactlyAsync<HttpRequestException>();
+
+        // Assert
+        thrown.Which.StatusCode.Should().Be(HttpStatusCode.ServiceUnavailable);
+        thrown.Which.Data["ResponseBody"].Should().Be(responseBody);
+    }
+
     [Fact]
     public async Task GetAllAsync_Overrides_Pagination_Query_Params_In_Request_Url()
     {

src/OctoshiftCLI.Tests/Octoshift/Services/OctoLoggerTests.cs

@@ -240,6 +240,30 @@ public void LogError_With_Exception_Should_Write_To_Console_Error()
         _consoleError.Should().NotBeNull();
     }
 
+    [Fact]
+    public void LogError_For_Exception_With_ResponseBody_Should_Include_Response_Body_In_Non_Verbose_Mode()
+    {
+        // Arrange
+        const string genericErrorMessage = "An unexpected error happened. Please see the logs for details.";
+        const string responseBody = "{\"errors\":[{\"message\":\"Could not start migration job\"}]}";
+        var httpException = new HttpRequestException("Response status code does not indicate success: 503 (Service Unavailable).", null, HttpStatusCode.ServiceUnavailable);
+        httpException.Data["ResponseBody"] = responseBody;
+
+        // Act
+        _octoLogger.LogError(httpException);
+
+        // Assert
+        _consoleOutput.Should().BeNull();
+
+        _consoleError.Should().Contain(genericErrorMessage);
+        _consoleError.Should().Contain($"Response: {responseBody}");
+
+        _logOutput.Should().Contain(genericErrorMessage);
+        _logOutput.Should().Contain($"Response: {responseBody}");
+
+        _verboseLogOutput.Should().Contain(responseBody);
+    }
+
     [Fact]
     public void LogInformation_Should_Write_To_Console_Out()
     {