diff --git a/README.md b/README.md index 5e55247..158fd5f 100644 --- a/README.md +++ b/README.md @@ -132,5 +132,16 @@ public class BodyLoggerOptions /// Controls storage of client IP addresses https://learn.microsoft.com/en-us/azure/azure-monitor/app/ip-collection?tabs=net /// public bool DisableIpMasking { get; set; } = false; + + /// + /// Controls if the middleware should catch and rethrow exceptions to allow logging of request bodies + /// even if downstream middlewares or handlers throw. + /// + /// + /// In some edge cases this might interfere with custom exception handlers or other middlewares catching exceptions. + /// If you enable this feature, make sure that the body logger middleware is registered as early as possible + /// on host creation. + /// + public bool EnableBodyLoggingOnExceptions { get; set; } = false; } ``` diff --git a/src/ApplicationInsightsRequestLogging/BodyLoggerMiddleware.cs b/src/ApplicationInsightsRequestLogging/BodyLoggerMiddleware.cs index e225d0d..13ab507 100644 --- a/src/ApplicationInsightsRequestLogging/BodyLoggerMiddleware.cs +++ b/src/ApplicationInsightsRequestLogging/BodyLoggerMiddleware.cs @@ -31,8 +31,28 @@ public async Task InvokeAsync(HttpContext context, RequestDelegate next) _bodyReader.PrepareResponseBodyReading(context); } - // hand over to the next middleware and wait for the call to return - await next(context); + if (_options.EnableBodyLoggingOnExceptions) + { + // hand over to the next middleware and wait for the call to return + try + { + await next(context); + } + catch (Exception) + { + if (_options.HttpVerbs.Contains(context.Request.Method)) + { + _telemetryWriter.Write(context, _options.RequestBodyPropertyKey, + _sensitiveDataFilter.RemoveSensitiveData(requestBody)); + } + + throw; + } + } + else + { + await next(context); + } if (_options.HttpVerbs.Contains(context.Request.Method)) { diff --git a/src/ApplicationInsightsRequestLogging/Options/BodyLoggerOptions.cs b/src/ApplicationInsightsRequestLogging/Options/BodyLoggerOptions.cs index 88dbc87..539f7b6 100644 --- a/src/ApplicationInsightsRequestLogging/Options/BodyLoggerOptions.cs +++ b/src/ApplicationInsightsRequestLogging/Options/BodyLoggerOptions.cs @@ -56,6 +56,17 @@ public BodyLoggerOptions() /// public bool DisableIpMasking { get; set; } = false; + /// + /// Controls if the middleware should catch and rethrow exceptions to allow logging of request bodies + /// even if downstream middlewares or handlers throw. + /// + /// + /// In some edge cases this might interfere with custom exception handlers or other middlewares catching exceptions. + /// If you enable this feature, make sure that the body logger middleware is registered as early as possible + /// on host creation. + /// + public bool EnableBodyLoggingOnExceptions { get; set; } = false; + public List PropertyNamesWithSensitiveData { get; set; } = new List() { "password", diff --git a/test/ApplicationInsightsRequestLoggingTests/BodyLoggerMiddlewareTests.cs b/test/ApplicationInsightsRequestLoggingTests/BodyLoggerMiddlewareTests.cs index aa372a5..b3d932b 100644 --- a/test/ApplicationInsightsRequestLoggingTests/BodyLoggerMiddlewareTests.cs +++ b/test/ApplicationInsightsRequestLoggingTests/BodyLoggerMiddlewareTests.cs @@ -29,6 +29,97 @@ public void BodyLoggerMiddleware_Should_Throw_If_Ctor_Params_Null() // Assert action.Should().Throw(); } + + [Fact] + public async void BodyLoggerMiddleware_Should_Not_LogRequestBody_If_Downstream_Exception_And_Disabled() + { + // Arrange + var telemetryWriter = new Mock(); + + using var host = await new HostBuilder() + .ConfigureWebHost(webBuilder => + { + webBuilder + .UseTestServer() + .ConfigureServices(services => + { + services.AddTransient(); + services.AddTransient(); + services.AddSingleton(telemetryWriter.Object); + services.AddOptions().Configure(options => + { + options.EnableBodyLoggingOnExceptions = false; + }); + services.AddTransient(); + }) + .Configure(app => + { + app.UseMiddleware(); + app.Run( _ => throw new Exception("downstream exception")); + }); + }) + .StartAsync(); + + // Act + try + { + await host.GetTestClient().PostAsync("/", new StringContent("Hello from client")); + } + catch (Exception) + { + //ignore errors thrown by test client + } + + // Assert + telemetryWriter.VerifyNoOtherCalls(); + + } + + [Fact] + public async void BodyLoggerMiddleware_Should_LogRequestBody_If_Downstream_Exception_And_Enabled() + { + // Arrange + var telemetryWriter = new Mock(); + + using var host = await new HostBuilder() + .ConfigureWebHost(webBuilder => + { + webBuilder + .UseTestServer() + .ConfigureServices(services => + { + services.AddTransient(); + services.AddTransient(); + services.AddSingleton(telemetryWriter.Object); + services.AddOptions().Configure(options => + { + options.EnableBodyLoggingOnExceptions = true; + }); + services.AddTransient(); + }) + .Configure(app => + { + app.UseMiddleware(); + app.Run( _ => throw new Exception("downstream exception")); + }); + }) + .StartAsync(); + + // Act + try + { + await host.GetTestClient().PostAsync("/", new StringContent("Hello from client")); + } + catch (Exception) + { + //ignore errors thrown by test client + } + + // Assert + telemetryWriter.Verify(x => x.Write(It.IsAny(), "RequestBody", "Hello from client"), Times.Once); + + } + [Fact] public async void BodyLoggerMiddleware_Should_Send_Data_To_AppInsights() diff --git a/test/ManualTests/Controllers/TestController.cs b/test/ManualTests/Controllers/TestController.cs index 4332edd..9d1222e 100644 --- a/test/ManualTests/Controllers/TestController.cs +++ b/test/ManualTests/Controllers/TestController.cs @@ -17,12 +17,11 @@ public ActionResult MaxBytesTest([FromForm] string data) { return Ok(data); } + + [HttpPost("throw")] + public ActionResult ThrowException([FromBody] TestData data) + { + throw new InvalidOperationException("Test exception!"); + } } } - -public class TestData -{ - public string Name { get; set; } - public string Blog { get; set; } - public string Topics { get; set; } -} \ No newline at end of file diff --git a/test/ManualTests/ManualTests.csproj b/test/ManualTests/ManualTests.csproj index 92ea18d..dff8447 100644 --- a/test/ManualTests/ManualTests.csproj +++ b/test/ManualTests/ManualTests.csproj @@ -2,7 +2,7 @@ net6.0 - enable + disable enable diff --git a/test/ManualTests/Model/TestData.cs b/test/ManualTests/Model/TestData.cs new file mode 100644 index 0000000..895b5e6 --- /dev/null +++ b/test/ManualTests/Model/TestData.cs @@ -0,0 +1,8 @@ +public class TestData +{ + public string Name { get; set; } + + public string Blog { get; set; } + + public string Topics { get; set; } +} \ No newline at end of file diff --git a/test/ManualTests/Program.cs b/test/ManualTests/Program.cs index d4f3959..90ac36c 100644 --- a/test/ManualTests/Program.cs +++ b/test/ManualTests/Program.cs @@ -12,14 +12,16 @@ app.Run(); -void ConfigureConfiguration(ConfigurationManager configuration) {} +void ConfigureConfiguration(ConfigurationManager configuration) { } void ConfigureServices(IServiceCollection services) { services.AddApplicationInsightsTelemetry(); services.AddAppInsightsHttpBodyLogging(o => { o.HttpCodes.Add(StatusCodes.Status200OK); + o.HttpCodes.Add(StatusCodes.Status204NoContent); o.DisableIpMasking = true; + o.EnableBodyLoggingOnExceptions = true; }); services.AddControllers(); services.AddEndpointsApiExplorer(); diff --git a/test/ManualTests/appsettings.json b/test/ManualTests/appsettings.json index 10f68b8..03c7fb9 100644 --- a/test/ManualTests/appsettings.json +++ b/test/ManualTests/appsettings.json @@ -5,5 +5,8 @@ "Microsoft.AspNetCore": "Warning" } }, - "AllowedHosts": "*" + "AllowedHosts": "*", + "ApplicationInsights": { + "ConnectionString": "" + } }