From 7652c099858f453f176ce6222c801c2b7ecc1838 Mon Sep 17 00:00:00 2001 From: Thibaud Desodt Date: Thu, 28 Feb 2019 21:31:01 +0100 Subject: [PATCH 1/4] Expose a new channel to notify exceptions to be logged by module in a less intrusive way than doing `HttpContext.AddError` fixes #48 --- .../Extensions/SerilogWebRequestExtensions.cs | 93 +++++++++++++++++++ .../Classic/WebRequestLoggingHandler.cs | 4 +- .../SerilogWeb.Classic.csproj | 1 + .../SerilogWebRequestExtensionsTests.cs | 52 +++++++++++ .../WebRequestLoggingHandlerTests.cs | 23 +++++ 5 files changed, 172 insertions(+), 1 deletion(-) create mode 100644 src/SerilogWeb.Classic/Classic/Extensions/SerilogWebRequestExtensions.cs create mode 100644 test/SerilogWeb.Classic.Tests/Extensions/SerilogWebRequestExtensionsTests.cs diff --git a/src/SerilogWeb.Classic/Classic/Extensions/SerilogWebRequestExtensions.cs b/src/SerilogWeb.Classic/Classic/Extensions/SerilogWebRequestExtensions.cs new file mode 100644 index 0000000..0d6b7c3 --- /dev/null +++ b/src/SerilogWeb.Classic/Classic/Extensions/SerilogWebRequestExtensions.cs @@ -0,0 +1,93 @@ +using System; +using System.Collections; +using System.Collections.Generic; +using System.Web; + +namespace SerilogWeb.Classic.Extensions +{ + /// + /// + /// + public static class SerilogWebRequestExtensions + { + /// + /// The key under which we store Exceptions to be logged by SerilogWeb.Classic in HttpContext.Current.Items + /// + private const string SerilogWebErrorKey = "SerilogWebClassic_Errors"; + + /// + /// Adds an error so that it can be logged by the SerilogWeb module at the end of the request process + /// + /// the HttpContextBase + /// the Exception to log + public static void AddSerilogWebError(this HttpContextBase self, Exception exception) + { + if (self == null) throw new ArgumentNullException(nameof(self)); + AddSerilogWebErrorInternal(self.Items, exception); + + } + + /// + /// Adds an error so that it can be logged by the SerilogWeb module at the end of the request process + /// + /// the HttpContext + /// the Exception to log + public static void AddSerilogWebError(this HttpContext self, Exception exception) + { + if (self == null) throw new ArgumentNullException(nameof(self)); + AddSerilogWebErrorInternal(self.Items, exception); + } + + private static void AddSerilogWebErrorInternal(IDictionary items, Exception ex) + { + Stack errors = null; + if (items.Contains(SerilogWebErrorKey)) + { + errors = items[SerilogWebErrorKey] as Stack; + } + + errors = errors ?? new Stack(); + + errors.Push(ex); + items[SerilogWebErrorKey] = errors; + } + + + /// + /// Retrieves the last error stored through or null + /// + /// the HttpContextBase + public static Exception GetLastSerilogWebError(this HttpContextBase self) + { + if (self == null) throw new ArgumentNullException(nameof(self)); + return GetLastSerilogWebErrorInternal(self.Items); + } + + /// + /// Retrieves the last error stored through or null + /// + /// the HttpContextBase + public static Exception GetLastSerilogWebError(this HttpContext self) + { + if (self == null) throw new ArgumentNullException(nameof(self)); + return GetLastSerilogWebErrorInternal(self.Items); + } + + private static Exception GetLastSerilogWebErrorInternal(IDictionary items) + { + if (!items.Contains(SerilogWebErrorKey)) + { + return null; + } + + var errors = items[SerilogWebErrorKey] as Stack; + + if (errors == null || errors.Count == 0) + { + return null; + } + + return errors.Peek(); + } + } +} diff --git a/src/SerilogWeb.Classic/Classic/WebRequestLoggingHandler.cs b/src/SerilogWeb.Classic/Classic/WebRequestLoggingHandler.cs index 6a3806a..6c4e6ab 100644 --- a/src/SerilogWeb.Classic/Classic/WebRequestLoggingHandler.cs +++ b/src/SerilogWeb.Classic/Classic/WebRequestLoggingHandler.cs @@ -3,6 +3,7 @@ using System.Linq; using Serilog; using Serilog.Events; +using SerilogWeb.Classic.Extensions; namespace SerilogWeb.Classic { @@ -41,7 +42,8 @@ internal void OnLogRequest(SerilogWebClassicConfiguration configuration) if (request == null || configuration.RequestFilter(_application.Context)) return; - var error = _application.Server.GetLastError(); + var error = _application.Context.GetLastSerilogWebError() ?? _application.Server.GetLastError(); + var level = error != null || _application.Response.StatusCode >= 500 ? LogEventLevel.Error : configuration.RequestLoggingLevel; if (level == LogEventLevel.Error && error == null && _application.Context.AllErrors != null) diff --git a/src/SerilogWeb.Classic/SerilogWeb.Classic.csproj b/src/SerilogWeb.Classic/SerilogWeb.Classic.csproj index 7cf308a..d3bfad2 100644 --- a/src/SerilogWeb.Classic/SerilogWeb.Classic.csproj +++ b/src/SerilogWeb.Classic/SerilogWeb.Classic.csproj @@ -55,6 +55,7 @@ + diff --git a/test/SerilogWeb.Classic.Tests/Extensions/SerilogWebRequestExtensionsTests.cs b/test/SerilogWeb.Classic.Tests/Extensions/SerilogWebRequestExtensionsTests.cs new file mode 100644 index 0000000..c8039d9 --- /dev/null +++ b/test/SerilogWeb.Classic.Tests/Extensions/SerilogWebRequestExtensionsTests.cs @@ -0,0 +1,52 @@ +using System; +using SerilogWeb.Classic.Extensions; +using SerilogWeb.Classic.Tests.Support; +using Xunit; + +namespace SerilogWeb.Classic.Tests.Extensions +{ + public class SerilogWebRequestExtensionsTests + { + [Fact] + public void AddSerilogWebErrorCanBeRetrievedOnHttpContextBase() + { + var app = new FakeHttpApplication(); + var context = app.Context; + + var original = new Exception("It failed"); + context.AddSerilogWebError(original); + + var returned = context.GetLastSerilogWebError(); + + Assert.Same(original, returned); + } + + [Fact] + public void GetLastSerilogWebErrorReturnsNullWhenThereIsNoError() + { + var app = new FakeHttpApplication(); + var context = app.Context; + + var returned = context.GetLastSerilogWebError(); + + Assert.Null(returned); + } + + [Fact] + public void GetLastSerilogWebErrorReturnsLastAddedError() + { + var app = new FakeHttpApplication(); + var context = app.Context; + + var first = new Exception("It failed once"); + context.AddSerilogWebError(first); + + var second = new Exception("It failed twice"); + context.AddSerilogWebError(second); + + var returned = context.GetLastSerilogWebError(); + + Assert.Same(second, returned); + } + } +} diff --git a/test/SerilogWeb.Classic.Tests/WebRequestLoggingHandlerTests.cs b/test/SerilogWeb.Classic.Tests/WebRequestLoggingHandlerTests.cs index 4d1489e..71cc944 100644 --- a/test/SerilogWeb.Classic.Tests/WebRequestLoggingHandlerTests.cs +++ b/test/SerilogWeb.Classic.Tests/WebRequestLoggingHandlerTests.cs @@ -5,6 +5,7 @@ using Serilog; using Serilog.Core; using Serilog.Events; +using SerilogWeb.Classic.Extensions; using SerilogWeb.Classic.Tests.Support; using Xunit; @@ -475,5 +476,27 @@ public void RequestWithoutServerLastErrorButStatusCode500AreLoggedAsErrorWithLas Assert.Equal(LogEventLevel.Error, LastEvent.Level); Assert.Same(secondError, LastEvent.Exception); } + + [Fact] + public void RequestWithSerilogWebErrorAreLoggedAsError() + { + var error = new InvalidOperationException("Epic fail #1", new NotImplementedException()); + TestContext.SimulateRequest( + (req) => { }, + () => + { + App.Context.AddSerilogWebError(error); + + + Assert.Null(App.Server.GetLastError()); + Assert.NotNull(App.Context.GetLastSerilogWebError()); + + return new FakeHttpResponse(); + }); + + Assert.NotNull(LastEvent); + Assert.Equal(LogEventLevel.Error, LastEvent.Level); + Assert.Same(error, LastEvent.Exception); + } } } From 83bdfaf7148536fcd093780af2537895c52cf6b6 Mon Sep 17 00:00:00 2001 From: Thibaud Desodt Date: Thu, 28 Feb 2019 21:33:13 +0100 Subject: [PATCH 2/4] Bump version to 5.0 Because that feels like a big enough change --- appveyor.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/appveyor.yml b/appveyor.yml index 3f8de69..4520753 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -1,7 +1,7 @@ version: '{build}' image: Visual Studio 2017 build_script: -- ps: ./Build.ps1 -majorMinor "4.2" -patch "$env:APPVEYOR_BUILD_VERSION" -customLogger "C:\Program Files\AppVeyor\BuildAgent\Appveyor.MSBuildLogger.dll" +- ps: ./Build.ps1 -majorMinor "5.0" -patch "$env:APPVEYOR_BUILD_VERSION" -customLogger "C:\Program Files\AppVeyor\BuildAgent\Appveyor.MSBuildLogger.dll" artifacts: - path: SerilogWeb.*.nupkg deploy: From 770b4963084e09310468038f5c83bd71b804e1aa Mon Sep 17 00:00:00 2001 From: Thibaud Desodt Date: Thu, 28 Feb 2019 21:37:05 +0100 Subject: [PATCH 3/4] Update referenced Serilog version to v2.7.1 We might as well update it. v2.8 is still a bit fresh, and we don't want to force undesired package updates ... --- src/SerilogWeb.Classic/SerilogWeb.Classic.csproj | 2 +- src/SerilogWeb.Classic/packages.config | 2 +- test/SerilogWeb.Classic.Tests/SerilogWeb.Classic.Tests.csproj | 2 +- test/SerilogWeb.Test/SerilogWeb.Test.csproj | 2 +- test/SerilogWeb.Test/packages.config | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/SerilogWeb.Classic/SerilogWeb.Classic.csproj b/src/SerilogWeb.Classic/SerilogWeb.Classic.csproj index d3bfad2..7bdcfee 100644 --- a/src/SerilogWeb.Classic/SerilogWeb.Classic.csproj +++ b/src/SerilogWeb.Classic/SerilogWeb.Classic.csproj @@ -41,7 +41,7 @@ - ..\..\packages\Serilog.2.6.0\lib\net45\Serilog.dll + ..\..\packages\Serilog.2.7.1\lib\net45\Serilog.dll diff --git a/src/SerilogWeb.Classic/packages.config b/src/SerilogWeb.Classic/packages.config index 359529e..767fd96 100644 --- a/src/SerilogWeb.Classic/packages.config +++ b/src/SerilogWeb.Classic/packages.config @@ -1,4 +1,4 @@  - + \ No newline at end of file diff --git a/test/SerilogWeb.Classic.Tests/SerilogWeb.Classic.Tests.csproj b/test/SerilogWeb.Classic.Tests/SerilogWeb.Classic.Tests.csproj index e8abacc..1012327 100644 --- a/test/SerilogWeb.Classic.Tests/SerilogWeb.Classic.Tests.csproj +++ b/test/SerilogWeb.Classic.Tests/SerilogWeb.Classic.Tests.csproj @@ -7,7 +7,7 @@ - + diff --git a/test/SerilogWeb.Test/SerilogWeb.Test.csproj b/test/SerilogWeb.Test/SerilogWeb.Test.csproj index fbe41ad..05d21d7 100644 --- a/test/SerilogWeb.Test/SerilogWeb.Test.csproj +++ b/test/SerilogWeb.Test/SerilogWeb.Test.csproj @@ -50,7 +50,7 @@ - ..\..\packages\Serilog.2.6.0\lib\net45\Serilog.dll + ..\..\packages\Serilog.2.7.1\lib\net45\Serilog.dll ..\..\packages\Serilog.Settings.AppSettings.2.1.2\lib\net45\Serilog.Settings.AppSettings.dll diff --git a/test/SerilogWeb.Test/packages.config b/test/SerilogWeb.Test/packages.config index bfcfccd..1a1830c 100644 --- a/test/SerilogWeb.Test/packages.config +++ b/test/SerilogWeb.Test/packages.config @@ -2,7 +2,7 @@ - + \ No newline at end of file From d61c7769d77e9aa4bf577406a0e9679c14e0fbdd Mon Sep 17 00:00:00 2001 From: Thibaud Desodt Date: Thu, 28 Feb 2019 21:53:02 +0100 Subject: [PATCH 4/4] Replace deprecated licenseUrl with license in nuspec file --- src/SerilogWeb.Classic/SerilogWeb.Classic.nuspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/SerilogWeb.Classic/SerilogWeb.Classic.nuspec b/src/SerilogWeb.Classic/SerilogWeb.Classic.nuspec index 3df9912..1785e98 100644 --- a/src/SerilogWeb.Classic/SerilogWeb.Classic.nuspec +++ b/src/SerilogWeb.Classic/SerilogWeb.Classic.nuspec @@ -7,7 +7,7 @@ Logs details of System.Web HTTP requests through Serilog. en-US https://github.com/serilog-web/classic - https://www.apache.org/licenses/LICENSE-2.0 + Apache-2.0 https://serilog-web.github.io/pages/images/serilog-web.png serilog logging aspnet