Intercepts unhandled exceptions, returns somewhat descriptive message…

… to caller (#710) * Intercepts unhandled exceptions, returns somewhat descriptive message to caller * Fixes tests * Tweaks error handler and expands tests
Altinn · Jul 10, 2024 · 57ab8c4 · 57ab8c4
1 parent e07f546
commit 57ab8c4
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 25 deletions.
diff --git a/src/Altinn.App.Api/Controllers/ExternalApiController.cs b/src/Altinn.App.Api/Controllers/ExternalApiController.cs
@@ -63,14 +63,18 @@ [FromQuery] Dictionary<string, string> queryParams
 
             return Ok(externalApiData);
         }
-        catch (HttpRequestException ex)
+        catch (Exception ex)
         {
-            _logger.LogError(ex, "Error when calling external api.");
-            if (ex.StatusCode.HasValue)
+            const string genericErrorDescription = "An error occurred when calling external api";
+            _logger.LogError(ex, genericErrorDescription);
+
+            if (ex is HttpRequestException { StatusCode: not null } httpEx)
             {
-                return StatusCode((int)ex.StatusCode.Value, ex.Message);
+                var errorMessage = !string.IsNullOrWhiteSpace(ex.Message) ? ex.Message : genericErrorDescription;
+                return StatusCode((int)httpEx.StatusCode.Value, errorMessage);
             }
-            return StatusCode((int)HttpStatusCode.InternalServerError, "An error occurred when calling external api.");
+
+            return StatusCode((int)HttpStatusCode.InternalServerError, $"{genericErrorDescription}: {ex.Message}");
         }
     }
 }
diff --git a/test/Altinn.App.Api.Tests/Controllers/ExternalApiControllerTests.cs b/test/Altinn.App.Api.Tests/Controllers/ExternalApiControllerTests.cs
@@ -38,10 +38,10 @@ public async Task Get_ShouldReturnOkResult_WhenExternalApiDataIsFound()
         var result = await controller.Get(1, Guid.NewGuid(), externalApiId, queryParams);
 
         // Assert
-        result.Should().BeOfType<OkObjectResult>();
         var okResult = result as OkObjectResult;
-        okResult?.StatusCode.Should().Be((int)HttpStatusCode.OK);
-        okResult?.Value.Should().Be(externalApiData);
+        Assert.NotNull(okResult);
+        okResult.StatusCode.Should().Be((int)HttpStatusCode.OK);
+        okResult.Value.Should().Be(externalApiData);
     }
 
     [Fact]
@@ -61,53 +61,66 @@ public async Task Get_ShouldReturnNotFoundResult_WhenExternalApiDataIsNotFound()
         var result = await controller.Get(1, Guid.NewGuid(), externalApiId, queryParams);
 
         // Assert
-        result.Should().BeOfType<BadRequestObjectResult>();
         var objectResult = result as BadRequestObjectResult;
-        objectResult?.StatusCode.Should().Be((int)HttpStatusCode.BadRequest);
-        objectResult?.Value.Should().Be($"External api with id '{externalApiId}' not found.");
+        Assert.NotNull(objectResult);
+        objectResult.StatusCode.Should().Be((int)HttpStatusCode.BadRequest);
+        objectResult.Value.Should().Be($"External api with id '{externalApiId}' not found.");
     }
 
     [Fact]
-    public async Task Get_ShouldThrowException_WhenExternalApiServiceThrowsException()
+    public async Task Get_ShouldReturnHttpStatus500_WhenExternalApiServiceThrowsUnhandledException()
     {
         //Arrange
         string externalApiId = "apiId";
         Dictionary<string, string> queryParams = [];
 
         _externalApiServiceMock
             .Setup(s => s.GetExternalApiData(externalApiId, It.IsAny<InstanceIdentifier>(), queryParams))
-            .Throws<Exception>();
+            .ThrowsAsync(new Exception("Error message"));
 
         var controller = new ExternalApiController(_loggerMock.Object, _externalApiServiceMock.Object);
 
-        // Act & Assert
-        await Assert.ThrowsAsync<Exception>(
-            async () => await controller.Get(1, Guid.NewGuid(), externalApiId, queryParams)
-        );
+        // Act
+        var result = await controller.Get(1, Guid.NewGuid(), externalApiId, queryParams);
+
+        // Assert
+        var objectResult = result as ObjectResult;
+        Assert.NotNull(objectResult);
+        objectResult.StatusCode.Should().Be((int)HttpStatusCode.InternalServerError);
+        objectResult.Value.Should().Match(x => ((string)x).EndsWith("Error message"));
     }
 
-    [Fact]
-    public async Task Get_ShouldReturnStatusCode_WhenExternalApiServiceThrowsHttpRequestException()
+    [Theory]
+    [InlineData("Error message")]
+    [InlineData("")]
+    public async Task Get_ShouldReturnStatusCode_AndFallbackToDefaultMessage_WhenExternalApiServiceThrowsHttpRequestException(
+        string errorMessage
+    )
     {
         // Arrange
         string externalApiId = "apiId";
         Dictionary<string, string> queryParams = [];
 
         _externalApiServiceMock
             .Setup(s => s.GetExternalApiData(externalApiId, It.IsAny<InstanceIdentifier>(), queryParams))
-            .ThrowsAsync(
-                new HttpRequestException("Error message", new HttpRequestException(), HttpStatusCode.BadRequest)
-            );
+            .ThrowsAsync(new HttpRequestException(errorMessage, new HttpRequestException(), HttpStatusCode.BadRequest));
 
         var controller = new ExternalApiController(_loggerMock.Object, _externalApiServiceMock.Object);
 
         // Act
         var result = await controller.Get(1, Guid.NewGuid(), externalApiId, queryParams);
 
         // Assert
-        result.Should().BeOfType<ObjectResult>();
         var objectResult = result as ObjectResult;
-        objectResult?.StatusCode.Should().Be((int)HttpStatusCode.BadRequest);
-        objectResult?.Value.Should().Be("Error message");
+        Assert.NotNull(objectResult);
+        objectResult.StatusCode.Should().Be((int)HttpStatusCode.BadRequest);
+
+        objectResult
+            .Value.Should()
+            .Be(
+                string.IsNullOrWhiteSpace(errorMessage)
+                    ? "An error occurred when calling external api"
+                    : "Error message"
+            );
     }
 }