From 6fa78e53d015fc4ebb0b8e186af74b2c3050f9bb Mon Sep 17 00:00:00 2001 From: m0sa Date: Fri, 22 Jun 2018 00:45:59 +0200 Subject: [PATCH 01/11] initial stab at ILogger integration, seems like we're getting duplicate entries now (host as well as middleware logging the exception --- .../ExceptionalBuilderExtensions.cs | 10 +++- .../ExceptionalLogger.cs | 55 +++++++++++++++++++ .../ExceptionalLoggerProvider.cs | 27 +++++++++ .../ExceptionalServiceExtensions.cs | 6 ++ ...tackExchange.Exceptional.AspNetCore.csproj | 2 +- 5 files changed, 97 insertions(+), 3 deletions(-) create mode 100644 src/StackExchange.Exceptional.AspNetCore/ExceptionalLogger.cs create mode 100644 src/StackExchange.Exceptional.AspNetCore/ExceptionalLoggerProvider.cs diff --git a/src/StackExchange.Exceptional.AspNetCore/ExceptionalBuilderExtensions.cs b/src/StackExchange.Exceptional.AspNetCore/ExceptionalBuilderExtensions.cs index 07563978..1710d862 100644 --- a/src/StackExchange.Exceptional.AspNetCore/ExceptionalBuilderExtensions.cs +++ b/src/StackExchange.Exceptional.AspNetCore/ExceptionalBuilderExtensions.cs @@ -1,4 +1,6 @@ -using StackExchange.Exceptional; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using StackExchange.Exceptional; using System; namespace Microsoft.AspNetCore.Builder @@ -16,7 +18,11 @@ public static class ExceptionalBuilderExtensions public static IApplicationBuilder UseExceptional(this IApplicationBuilder builder) { _ = builder ?? throw new ArgumentNullException(nameof(builder)); + + var loggerFactory = builder.ApplicationServices.GetRequiredService(); + loggerFactory.AddProvider(builder.ApplicationServices.GetRequiredService()); + return builder.UseMiddleware(); } } -} \ No newline at end of file +} diff --git a/src/StackExchange.Exceptional.AspNetCore/ExceptionalLogger.cs b/src/StackExchange.Exceptional.AspNetCore/ExceptionalLogger.cs new file mode 100644 index 00000000..07d63385 --- /dev/null +++ b/src/StackExchange.Exceptional.AspNetCore/ExceptionalLogger.cs @@ -0,0 +1,55 @@ +using System; +using System.Collections.Generic; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; + +namespace StackExchange.Exceptional +{ + class ExceptionalLogger : ILogger + { + private readonly string _category; + private readonly IHttpContextAccessor _httpContextAccessor; + private readonly IOptions _settings; + + public ExceptionalLogger(string category, IOptions settings, IHttpContextAccessor httpContextAccessor = null) + { + _category = category; + _settings = settings; + _httpContextAccessor = httpContextAccessor; + } + + public IDisposable BeginScope(TState state) + { + return null; + } + + public bool IsEnabled(LogLevel logLevel) + { + return true; + // TODO compare against settings and add support for per-category log levels, or we can just leave it up to LogFilters to decide later + } + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func formatter) + { + if (exception == null) return; + + var customData = new Dictionary + { + ["AspNetCore.LogLevel"] = logLevel + "", + ["AspNetCore.EventId.Id"] = eventId.Id + "", + ["AspNetCore.EventId.Name"] = eventId.Name, + ["AspNetCore.Message"] = formatter(state, exception), + }; + + if (_httpContextAccessor?.HttpContext is HttpContext context) + { + exception.Log(context, _category, customData: customData); + } + else + { + exception.LogNoContext(_category, customData: customData); + } + } + } +} diff --git a/src/StackExchange.Exceptional.AspNetCore/ExceptionalLoggerProvider.cs b/src/StackExchange.Exceptional.AspNetCore/ExceptionalLoggerProvider.cs new file mode 100644 index 00000000..43d4ff38 --- /dev/null +++ b/src/StackExchange.Exceptional.AspNetCore/ExceptionalLoggerProvider.cs @@ -0,0 +1,27 @@ + +using System; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; + +namespace StackExchange.Exceptional +{ + class ExceptionalLoggerProvider : ILoggerProvider + { + private readonly IOptions _settings; + private readonly IHttpContextAccessor _httpContextAccessor; + + public ExceptionalLoggerProvider(IOptions settings, IHttpContextAccessor httpContextAccessor = null) + { + _settings = settings; + _httpContextAccessor = httpContextAccessor; + } + + ILogger ILoggerProvider.CreateLogger(string categoryName) + => new ExceptionalLogger(categoryName, _settings, _httpContextAccessor); + + void IDisposable.Dispose() + { + } + } +} diff --git a/src/StackExchange.Exceptional.AspNetCore/ExceptionalServiceExtensions.cs b/src/StackExchange.Exceptional.AspNetCore/ExceptionalServiceExtensions.cs index 56fd269d..767afa26 100644 --- a/src/StackExchange.Exceptional.AspNetCore/ExceptionalServiceExtensions.cs +++ b/src/StackExchange.Exceptional.AspNetCore/ExceptionalServiceExtensions.cs @@ -1,6 +1,8 @@ using Microsoft.Extensions.Configuration; using StackExchange.Exceptional; using System; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection.Extensions; namespace Microsoft.Extensions.DependencyInjection { @@ -36,6 +38,10 @@ public static IServiceCollection AddExceptional(this IServiceCollection services // When done configuring, set the background settings object for non-context logging. services.Configure(Exceptional.Configure); + // setup for ILogger & co + services.TryAddSingleton(); + services.AddSingleton(); + return services; } } diff --git a/src/StackExchange.Exceptional.AspNetCore/StackExchange.Exceptional.AspNetCore.csproj b/src/StackExchange.Exceptional.AspNetCore/StackExchange.Exceptional.AspNetCore.csproj index 6647859e..82ff9a23 100644 --- a/src/StackExchange.Exceptional.AspNetCore/StackExchange.Exceptional.AspNetCore.csproj +++ b/src/StackExchange.Exceptional.AspNetCore/StackExchange.Exceptional.AspNetCore.csproj @@ -9,7 +9,7 @@ - + From 73777dcd394dced02154842ecbae7a1f1b2da0c6 Mon Sep 17 00:00:00 2001 From: m0sa Date: Fri, 22 Jun 2018 01:01:09 +0200 Subject: [PATCH 02/11] yo dawg, I've heard you like to log exceptions while you log exceptions fixing double logs, where middleware was writing exceptions to an ILogger, fixes unit tests --- .../ExceptionalLogger.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/StackExchange.Exceptional.AspNetCore/ExceptionalLogger.cs b/src/StackExchange.Exceptional.AspNetCore/ExceptionalLogger.cs index 07d63385..ca5fde56 100644 --- a/src/StackExchange.Exceptional.AspNetCore/ExceptionalLogger.cs +++ b/src/StackExchange.Exceptional.AspNetCore/ExceptionalLogger.cs @@ -1,4 +1,4 @@ -using System; +using System; using System.Collections.Generic; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Logging; @@ -11,12 +11,18 @@ class ExceptionalLogger : ILogger private readonly string _category; private readonly IHttpContextAccessor _httpContextAccessor; private readonly IOptions _settings; + private readonly bool _ignored; + private static readonly HashSet _ignoredCategories = new HashSet + { + typeof(ExceptionalMiddleware).FullName, // exceptional middleware calls some ILogger stuff itself, ignore those calls) + }; public ExceptionalLogger(string category, IOptions settings, IHttpContextAccessor httpContextAccessor = null) { _category = category; _settings = settings; _httpContextAccessor = httpContextAccessor; + _ignored = _ignoredCategories.Contains(category); } public IDisposable BeginScope(TState state) @@ -26,12 +32,13 @@ public IDisposable BeginScope(TState state) public bool IsEnabled(LogLevel logLevel) { - return true; + return !_ignored; // TODO compare against settings and add support for per-category log levels, or we can just leave it up to LogFilters to decide later } public void Log(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func formatter) { + if (_ignored) return; if (exception == null) return; var customData = new Dictionary From 2af7b787bde5a9063c8bc771336e199faf342211 Mon Sep 17 00:00:00 2001 From: Nick Craver Date: Sat, 23 Jun 2018 18:14:34 -0400 Subject: [PATCH 03/11] Remove IHttpContextAccessor auto-registering Leaving it up to the user, if they want to. --- samples/Samples.AspNetCore/Startup.cs | 8 +++++++- .../ExceptionalServiceExtensions.cs | 5 +---- .../StackExchange.Exceptional.AspNetCore.csproj | 2 +- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/samples/Samples.AspNetCore/Startup.cs b/samples/Samples.AspNetCore/Startup.cs index 1c1cbc39..8eeb1002 100644 --- a/samples/Samples.AspNetCore/Startup.cs +++ b/samples/Samples.AspNetCore/Startup.cs @@ -1,7 +1,9 @@ using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Hosting; +using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Configuration; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; namespace Samples.AspNetCore { @@ -20,6 +22,10 @@ public Startup(IConfiguration configuration, IHostingEnvironment env) public void ConfigureServices(IServiceCollection services) { services.AddMvc(); + // (Optional): If you want ILogger calls that log an exception to have request details + // Then it needs access to the HttpContext statically, this registers that ability. + // If you're using Identity or ApplicationInsights, this is already registered. + services.TryAddSingleton(); // Make IOptions available for injection everywhere services.AddExceptional(Configuration.GetSection("Exceptional"), settings => { @@ -52,4 +58,4 @@ public void Configure(IApplicationBuilder app) }); } } -} \ No newline at end of file +} diff --git a/src/StackExchange.Exceptional.AspNetCore/ExceptionalServiceExtensions.cs b/src/StackExchange.Exceptional.AspNetCore/ExceptionalServiceExtensions.cs index 767afa26..6ed973b7 100644 --- a/src/StackExchange.Exceptional.AspNetCore/ExceptionalServiceExtensions.cs +++ b/src/StackExchange.Exceptional.AspNetCore/ExceptionalServiceExtensions.cs @@ -1,8 +1,6 @@ using Microsoft.Extensions.Configuration; using StackExchange.Exceptional; using System; -using Microsoft.AspNetCore.Http; -using Microsoft.Extensions.DependencyInjection.Extensions; namespace Microsoft.Extensions.DependencyInjection { @@ -38,8 +36,7 @@ public static IServiceCollection AddExceptional(this IServiceCollection services // When done configuring, set the background settings object for non-context logging. services.Configure(Exceptional.Configure); - // setup for ILogger & co - services.TryAddSingleton(); + // Setup for ILogger services.AddSingleton(); return services; diff --git a/src/StackExchange.Exceptional.AspNetCore/StackExchange.Exceptional.AspNetCore.csproj b/src/StackExchange.Exceptional.AspNetCore/StackExchange.Exceptional.AspNetCore.csproj index 82ff9a23..6647859e 100644 --- a/src/StackExchange.Exceptional.AspNetCore/StackExchange.Exceptional.AspNetCore.csproj +++ b/src/StackExchange.Exceptional.AspNetCore/StackExchange.Exceptional.AspNetCore.csproj @@ -9,7 +9,7 @@ - + From 67913647212167505757daebd47e08aab2a6d7e8 Mon Sep 17 00:00:00 2001 From: Nick Craver Date: Sat, 23 Jun 2018 18:15:12 -0400 Subject: [PATCH 04/11] Add ILogger example to ASP.NET Core sample --- .../Samples.AspNetCore/Controllers/TestController.cs | 12 ++++++++++++ .../Samples.AspNetCore/Views/Shared/_Layout.cshtml | 1 + 2 files changed, 13 insertions(+) diff --git a/samples/Samples.AspNetCore/Controllers/TestController.cs b/samples/Samples.AspNetCore/Controllers/TestController.cs index 7aa633a6..f3bf1b9a 100644 --- a/samples/Samples.AspNetCore/Controllers/TestController.cs +++ b/samples/Samples.AspNetCore/Controllers/TestController.cs @@ -1,4 +1,5 @@ using Microsoft.AspNetCore.Mvc; +using Microsoft.Extensions.Logging; using StackExchange.Exceptional; using System; using System.Threading.Tasks; @@ -7,6 +8,17 @@ namespace Samples.AspNetCore.Controllers { public class TestController : Controller { + private readonly ILogger _logger; + + public TestController(ILogger logger) => _logger = logger; + + public ActionResult Logger() + { + var ex = new Exception("Test Exception for ILogger goodness."); + _logger.LogError(ex, ex.Message); + return Content("Check the log!"); + } + public async Task Throw() { await ExceptionalUtils.Test.GetRedisException().LogAsync(ControllerContext.HttpContext).ConfigureAwait(false); diff --git a/samples/Samples.AspNetCore/Views/Shared/_Layout.cshtml b/samples/Samples.AspNetCore/Views/Shared/_Layout.cshtml index 9b8772bd..3406f4c0 100644 --- a/samples/Samples.AspNetCore/Views/Shared/_Layout.cshtml +++ b/samples/Samples.AspNetCore/Views/Shared/_Layout.cshtml @@ -33,6 +33,7 @@
  • Home
  • Exceptions
  • Throw
    • +
  • ILogger
    • From 5e3a119efd51b09cc6b2e3d5a5686ff442cb5750 Mon Sep 17 00:00:00 2001 From: Nick Craver Date: Sat, 23 Jun 2018 18:15:42 -0400 Subject: [PATCH 05/11] Cleanup --- .../ExceptionalLoggerProvider.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/StackExchange.Exceptional.AspNetCore/ExceptionalLoggerProvider.cs b/src/StackExchange.Exceptional.AspNetCore/ExceptionalLoggerProvider.cs index 43d4ff38..b742d4ee 100644 --- a/src/StackExchange.Exceptional.AspNetCore/ExceptionalLoggerProvider.cs +++ b/src/StackExchange.Exceptional.AspNetCore/ExceptionalLoggerProvider.cs @@ -1,4 +1,4 @@ - + using System; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Logging; @@ -6,7 +6,7 @@ namespace StackExchange.Exceptional { - class ExceptionalLoggerProvider : ILoggerProvider + internal class ExceptionalLoggerProvider : ILoggerProvider { private readonly IOptions _settings; private readonly IHttpContextAccessor _httpContextAccessor; @@ -20,8 +20,6 @@ public ExceptionalLoggerProvider(IOptions settings, IHttpCo ILogger ILoggerProvider.CreateLogger(string categoryName) => new ExceptionalLogger(categoryName, _settings, _httpContextAccessor); - void IDisposable.Dispose() - { - } + void IDisposable.Dispose() { } } } From 2c4dd4b88bca269e6ba820a98c90ce2294a02e6b Mon Sep 17 00:00:00 2001 From: Nick Craver Date: Sat, 23 Jun 2018 18:16:32 -0400 Subject: [PATCH 06/11] ILogger: add setting for severity and known EventId entries --- .../ExceptionalLogger.cs | 30 +++++++------------ .../ExceptionalLoggingEvents.cs | 21 +++++++++++++ .../ExceptionalMiddleware.cs | 4 +-- .../ExceptionalSettings.cs | 7 +++++ 4 files changed, 40 insertions(+), 22 deletions(-) create mode 100644 src/StackExchange.Exceptional.AspNetCore/ExceptionalLoggingEvents.cs diff --git a/src/StackExchange.Exceptional.AspNetCore/ExceptionalLogger.cs b/src/StackExchange.Exceptional.AspNetCore/ExceptionalLogger.cs index ca5fde56..47e214e5 100644 --- a/src/StackExchange.Exceptional.AspNetCore/ExceptionalLogger.cs +++ b/src/StackExchange.Exceptional.AspNetCore/ExceptionalLogger.cs @@ -6,45 +6,35 @@ namespace StackExchange.Exceptional { - class ExceptionalLogger : ILogger + internal class ExceptionalLogger : ILogger { private readonly string _category; private readonly IHttpContextAccessor _httpContextAccessor; private readonly IOptions _settings; - private readonly bool _ignored; - private static readonly HashSet _ignoredCategories = new HashSet - { - typeof(ExceptionalMiddleware).FullName, // exceptional middleware calls some ILogger stuff itself, ignore those calls) - }; public ExceptionalLogger(string category, IOptions settings, IHttpContextAccessor httpContextAccessor = null) { _category = category; _settings = settings; _httpContextAccessor = httpContextAccessor; - _ignored = _ignoredCategories.Contains(category); } - public IDisposable BeginScope(TState state) - { - return null; - } + public IDisposable BeginScope(TState state) => null; - public bool IsEnabled(LogLevel logLevel) - { - return !_ignored; - // TODO compare against settings and add support for per-category log levels, or we can just leave it up to LogFilters to decide later - } + public bool IsEnabled(LogLevel logLevel) => _settings.Value.ILoggerLevel <= logLevel; public void Log(LogLevel logLevel, EventId eventId, TState state, Exception exception, Func formatter) { - if (_ignored) return; - if (exception == null) return; + // Ignore non-exceptions and Exceptional events themselves + if (exception == null || (ExceptionalLoggingEvents.Min <=eventId.Id && eventId.Id <= ExceptionalLoggingEvents.Max)) + { + return; + } var customData = new Dictionary { - ["AspNetCore.LogLevel"] = logLevel + "", - ["AspNetCore.EventId.Id"] = eventId.Id + "", + ["AspNetCore.LogLevel"] = logLevel.ToString(), + ["AspNetCore.EventId.Id"] = eventId.Id.ToString(), ["AspNetCore.EventId.Name"] = eventId.Name, ["AspNetCore.Message"] = formatter(state, exception), }; diff --git a/src/StackExchange.Exceptional.AspNetCore/ExceptionalLoggingEvents.cs b/src/StackExchange.Exceptional.AspNetCore/ExceptionalLoggingEvents.cs new file mode 100644 index 00000000..701b49f1 --- /dev/null +++ b/src/StackExchange.Exceptional.AspNetCore/ExceptionalLoggingEvents.cs @@ -0,0 +1,21 @@ +namespace StackExchange.Exceptional +{ + /// + /// Events IDs for known exceptional events while logging. + /// + public static class ExceptionalLoggingEvents + { + internal const int Min = RequestException; + internal const int Max = ExceptionalPageException; + + /// + /// A request threw an exception, caught by Exceptional Middleware. + /// + public const int RequestException = 77000; + + /// + /// A request was thrown while trying to render the Exceptional error page. + /// + public const int ExceptionalPageException = 77001; + } +} diff --git a/src/StackExchange.Exceptional.AspNetCore/ExceptionalMiddleware.cs b/src/StackExchange.Exceptional.AspNetCore/ExceptionalMiddleware.cs index 049d6c22..2db1c0ed 100644 --- a/src/StackExchange.Exceptional.AspNetCore/ExceptionalMiddleware.cs +++ b/src/StackExchange.Exceptional.AspNetCore/ExceptionalMiddleware.cs @@ -63,7 +63,7 @@ public async Task Invoke(HttpContext context) } catch (Exception ex) { - _logger.LogError(0, ex, "An unhandled exception has occurred, logging to Exceptional"); + _logger.LogError(ExceptionalLoggingEvents.RequestException, ex, "An unhandled exception has occurred, logging to Exceptional"); var error = await ex.LogAsync(context).ConfigureAwait(false); // If options say to do so, show the exception page to the user @@ -101,7 +101,7 @@ public async Task Invoke(HttpContext context) } catch (Exception pex) { - _logger.LogError(0, pex, "An exception was thrown attempting to display the Exceptional page."); + _logger.LogError(ExceptionalLoggingEvents.ExceptionalPageException, pex, "An exception was thrown attempting to display the Exceptional page."); } } throw; diff --git a/src/StackExchange.Exceptional.AspNetCore/ExceptionalSettings.cs b/src/StackExchange.Exceptional.AspNetCore/ExceptionalSettings.cs index a245bcfc..c0cb4654 100644 --- a/src/StackExchange.Exceptional.AspNetCore/ExceptionalSettings.cs +++ b/src/StackExchange.Exceptional.AspNetCore/ExceptionalSettings.cs @@ -1,4 +1,5 @@ using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.Logging; using StackExchange.Exceptional.Internal; using System; @@ -19,5 +20,11 @@ public class ExceptionalSettings : ExceptionalSettingsBase /// but may need to be replaced in special multi-proxy situations. /// public Func GetIPAddress { get; set; } = context => context.Connection.RemoteIpAddress.ToString(); + + /// + /// The minimum log level for . + /// Defaults to . + /// + public LogLevel ILoggerLevel { get; set; } = LogLevel.Error; } } From 53aed8df5b13bcb5ffeef4f6c1e272c3337030cb Mon Sep 17 00:00:00 2001 From: Nick Craver Date: Sat, 23 Jun 2018 18:23:08 -0400 Subject: [PATCH 07/11] Fix comment --- samples/Samples.AspNetCore/Startup.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/samples/Samples.AspNetCore/Startup.cs b/samples/Samples.AspNetCore/Startup.cs index 8eeb1002..ba8475e2 100644 --- a/samples/Samples.AspNetCore/Startup.cs +++ b/samples/Samples.AspNetCore/Startup.cs @@ -22,8 +22,8 @@ public Startup(IConfiguration configuration, IHostingEnvironment env) public void ConfigureServices(IServiceCollection services) { services.AddMvc(); - // (Optional): If you want ILogger calls that log an exception to have request details - // Then it needs access to the HttpContext statically, this registers that ability. + // (Optional): If you want ILogger calls that log an exception to have request details, + // then it needs access to the HttpContext statically, this registers that ability. // If you're using Identity or ApplicationInsights, this is already registered. services.TryAddSingleton(); // Make IOptions available for injection everywhere From 7c02a5d089efecc00a7de3b501d274ecaf56f461 Mon Sep 17 00:00:00 2001 From: Nick Craver Date: Sun, 16 Dec 2018 12:34:03 -0500 Subject: [PATCH 08/11] Bump Jil for proper multi-platform testing --- .../StackExchange.Exceptional.Tests.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/StackExchange.Exceptional.Tests/StackExchange.Exceptional.Tests.csproj b/tests/StackExchange.Exceptional.Tests/StackExchange.Exceptional.Tests.csproj index f32e836a..e4c689a7 100644 --- a/tests/StackExchange.Exceptional.Tests/StackExchange.Exceptional.Tests.csproj +++ b/tests/StackExchange.Exceptional.Tests/StackExchange.Exceptional.Tests.csproj @@ -9,7 +9,7 @@ - + From 4e46c6a129a555179dc41d58ffbb8fce5b597fa7 Mon Sep 17 00:00:00 2001 From: Nick Craver Date: Sun, 16 Dec 2018 12:37:41 -0500 Subject: [PATCH 09/11] Prevent duplicate logging fo cases like ILogger --- src/StackExchange.Exceptional.Shared/Error.cs | 24 +++++++++++++++++++ .../Internal/Constants.cs | 5 ++++ 2 files changed, 29 insertions(+) diff --git a/src/StackExchange.Exceptional.Shared/Error.cs b/src/StackExchange.Exceptional.Shared/Error.cs index ba42e699..e41212fb 100644 --- a/src/StackExchange.Exceptional.Shared/Error.cs +++ b/src/StackExchange.Exceptional.Shared/Error.cs @@ -119,6 +119,10 @@ private void AddData(Exception exception) foreach (string k in exception.Data.Keys) { + if (k == Constants.LoggedDataKey) + { + continue; + } if (regex?.IsMatch(k) == true) { InitCustomData(); @@ -201,8 +205,18 @@ public bool LogToStore(ErrorStore store = null) if (abort) return false; // if we've been told to abort, then abort dammit! Trace.WriteLine(Exception); // always echo the error to trace for local debugging + + if (Exception.Data?.Contains(Constants.LoggedDataKey) == true) + { + // already logged - match the GUIDs up + GUID = (Guid)Exception.Data[Constants.LoggedDataKey]; + return true; + } + store.Log(this); + if (Exception.Data != null) Exception.Data[Constants.LoggedDataKey] = GUID; // mark as logged + Settings.AfterLog(this, store); return true; @@ -220,8 +234,18 @@ public async Task LogToStoreAsync(ErrorStore store = null) if (abort) return true; // if we've been told to abort, then abort dammit! Trace.WriteLine(Exception); // always echo the error to trace for local debugging + + if (Exception.Data?.Contains(Constants.LoggedDataKey) == true) + { + // already logged - match the GUIDs up + GUID = (Guid)Exception.Data[Constants.LoggedDataKey]; + return true; + } + await store.LogAsync(this).ConfigureAwait(false); + if (Exception.Data != null) Exception.Data[Constants.LoggedDataKey] = GUID; // mark as logged + Settings.AfterLog(this, store); return true; diff --git a/src/StackExchange.Exceptional.Shared/Internal/Constants.cs b/src/StackExchange.Exceptional.Shared/Internal/Constants.cs index e4f7417e..a3e68641 100644 --- a/src/StackExchange.Exceptional.Shared/Internal/Constants.cs +++ b/src/StackExchange.Exceptional.Shared/Internal/Constants.cs @@ -19,5 +19,10 @@ public static class Constants /// Key for prefixing fields in .Data for logging to CustomData /// public const string CustomDataKeyPrefix = "ExceptionalCustom-"; + + /// + /// The key in Exception.Data that indicates Exceptional has already handled this exception and should ignore future attempts to log it. + /// + public const string LoggedDataKey = "Exceptional.Logged"; } } From 58e2c47dacc3b60d6c5d9a4df228962b0e5eed23 Mon Sep 17 00:00:00 2001 From: Nick Craver Date: Sun, 16 Dec 2018 12:40:54 -0500 Subject: [PATCH 10/11] Comment tips for ASP.NET Core --- samples/Samples.AspNetCore/Startup.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/samples/Samples.AspNetCore/Startup.cs b/samples/Samples.AspNetCore/Startup.cs index ce6c4e35..46e37910 100644 --- a/samples/Samples.AspNetCore/Startup.cs +++ b/samples/Samples.AspNetCore/Startup.cs @@ -25,6 +25,7 @@ public void ConfigureServices(IServiceCollection services) // (Optional): If you want ILogger calls that log an exception to have request details, // then it needs access to the HttpContext statically, this registers that ability. // If you're using Identity or ApplicationInsights, this is already registered. + // If using .NET Core 2.1+, you can call the new helper instead: services.AddHttpContextAccessor(); services.TryAddSingleton(); // Make IOptions available for injection everywhere services.AddExceptional(Configuration.GetSection("Exceptional"), settings => From cdb632959dc3d5acd03e1f1129d10a7e804c686c Mon Sep 17 00:00:00 2001 From: Nick Craver Date: Sun, 16 Dec 2018 13:36:32 -0500 Subject: [PATCH 11/11] ILogger: add test --- .../Logging.cs | 63 +++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/tests/StackExchange.Exceptional.Tests.AspNetCore/Logging.cs b/tests/StackExchange.Exceptional.Tests.AspNetCore/Logging.cs index 0a6ca7c1..9c3215f9 100644 --- a/tests/StackExchange.Exceptional.Tests.AspNetCore/Logging.cs +++ b/tests/StackExchange.Exceptional.Tests.AspNetCore/Logging.cs @@ -1,8 +1,16 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Net.Http; using System.Threading.Tasks; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.TestHost; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.DependencyInjection.Extensions; +using Microsoft.Extensions.Logging; +using StackExchange.Exceptional.Stores; using Xunit; using Xunit.Abstractions; @@ -138,5 +146,60 @@ public async Task LogNoContext() Assert.Null(error.ServerVariables); } } + + [Fact] + public async Task ILoggerLogging() + { + Error error = null; + using (var server = new TestServer(new WebHostBuilder() + .ConfigureServices(services => + { + services.TryAddSingleton(); + services.AddExceptional(s => + { + s.DefaultStore = new MemoryErrorStore(); + CurrentSettings = s; + }); + }) + .Configure(app => + { + var logger = app.ApplicationServices.GetRequiredService>(); + app.UseExceptional(); + app.Run(async context => + { + var ex = new Exception("Log!"); + logger.LogError(ex, ex.Message); + var errors = await CurrentSettings.DefaultStore.GetAllAsync(); + error = errors.FirstOrDefault(); + await context.Response.WriteAsync("Hey."); + }); + }))) + { + using (var response = await server.CreateClient().GetAsync("?QueryKey=QueryValue").ConfigureAwait(false)) + { + Assert.Equal("Hey.", await response.Content.ReadAsStringAsync().ConfigureAwait(false)); + } + + Assert.Equal("Log!", error.Message); + Assert.Equal("System.Exception", error.Type); + Assert.Equal(Environment.MachineName, error.MachineName); + Assert.Equal("localhost", error.Host); + Assert.Equal("http://localhost/?QueryKey=QueryValue", error.FullUrl); + Assert.Equal("/", error.UrlPath); + + Assert.NotEmpty(error.RequestHeaders); + Assert.Equal("localhost", error.RequestHeaders["Host"]); + + Assert.Single(error.QueryString); + Assert.Equal("QueryValue", error.QueryString["QueryKey"]); + + Assert.NotEmpty(error.ServerVariables); + Assert.Equal("localhost", error.ServerVariables["Host"]); + Assert.Equal("/", error.ServerVariables["Path"]); + Assert.Equal("GET", error.ServerVariables["Request Method"]); + Assert.Equal("http", error.ServerVariables["Scheme"]); + Assert.Equal("http://localhost/?QueryKey=QueryValue", error.ServerVariables["Url"]); + } + } } }