From 5e00937d591db8cc36cdfcf28f7696c2b7a3c7a9 Mon Sep 17 00:00:00 2001 From: rowanmiller Date: Tue, 16 Dec 2014 11:15:16 -0800 Subject: [PATCH] Handle wrapped exceptions in Database Error Page If a database exception is wrapped later in the request (after EF has logged it) then we were not displaying the database error page. This was occurring in ASP.NET Identity where the exception was getting wrapped in an AggregateException. We now walk the tree of inner exceptions looking for a match. Also adding some extra logging as this was hard to debug without resorting to source code. --- .../DatabaseErrorPageMiddleware.cs | 35 +++++++- .../Properties/Strings.Designer.cs | 80 +++++++++++++++++++ .../Strings.resx | 15 ++++ .../DatabaseErrorPageMiddlewareTest.cs | 35 ++++++++ 4 files changed, 162 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.AspNet.Diagnostics.Entity/DatabaseErrorPageMiddleware.cs b/src/Microsoft.AspNet.Diagnostics.Entity/DatabaseErrorPageMiddleware.cs index bc32702c55..b4fb72d6d2 100644 --- a/src/Microsoft.AspNet.Diagnostics.Entity/DatabaseErrorPageMiddleware.cs +++ b/src/Microsoft.AspNet.Diagnostics.Entity/DatabaseErrorPageMiddleware.cs @@ -66,8 +66,7 @@ namespace Microsoft.AspNet.Diagnostics.Entity { try { - if (_loggerProvider.Logger.LastError.IsErrorLogged - && _loggerProvider.Logger.LastError.Exception == ex) + if (ShouldDisplayErrorPage(_loggerProvider.Logger.LastError, ex, _logger)) { using (RequestServicesContainer.EnsureRequestServices(context, _serviceProvider)) { @@ -79,7 +78,11 @@ namespace Microsoft.AspNet.Diagnostics.Entity } else { - if (dbContext.Database is RelationalDatabase) + if (!(dbContext.Database is RelationalDatabase)) + { + _logger.WriteVerbose(Strings.DatabaseErrorPage_NotRelationalDatabase); + } + else { var databaseExists = dbContext.Database.AsRelational().Exists(); @@ -115,5 +118,31 @@ namespace Microsoft.AspNet.Diagnostics.Entity throw; } } + + private static bool ShouldDisplayErrorPage(DataStoreErrorLogger.DataStoreErrorLog lastError, Exception exception, ILogger logger) + { + logger.WriteVerbose(Strings.FormatDatabaseErrorPage_AttemptingToMatchException(exception.GetType())); + + if (!lastError.IsErrorLogged) + { + logger.WriteVerbose(Strings.DatabaseErrorPage_NoRecordedException); + return false; + } + + bool match = false; + for (var e = exception; e != null && !match; e = e.InnerException) + { + match = lastError.Exception == e; + } + + if (!match) + { + logger.WriteVerbose(Strings.DatabaseErrorPage_NoMatch); + return false; + } + + logger.WriteVerbose(Strings.DatabaseErrorPage_Matched); + return true; + } } } diff --git a/src/Microsoft.AspNet.Diagnostics.Entity/Properties/Strings.Designer.cs b/src/Microsoft.AspNet.Diagnostics.Entity/Properties/Strings.Designer.cs index 01ed143a61..713fe5e89f 100644 --- a/src/Microsoft.AspNet.Diagnostics.Entity/Properties/Strings.Designer.cs +++ b/src/Microsoft.AspNet.Diagnostics.Entity/Properties/Strings.Designer.cs @@ -458,6 +458,86 @@ namespace Microsoft.AspNet.Diagnostics.Entity return GetString("DatabaseErrorPage_EnableMigrationsCommandsInfo"); } + /// + /// {0} occurred, checking if Entity Framework recorded this exception as resulting from a failed database operation. + /// + internal static string DatabaseErrorPage_AttemptingToMatchException + { + get { return GetString("DatabaseErrorPage_AttemptingToMatchException"); } + } + + /// + /// {0} occurred, checking if Entity Framework recorded this exception as resulting from a failed database operation. + /// + internal static string FormatDatabaseErrorPage_AttemptingToMatchException(object p0) + { + return string.Format(CultureInfo.CurrentCulture, GetString("DatabaseErrorPage_AttemptingToMatchException"), p0); + } + + /// + /// Entity Framework recorded that the current exception was due to a failed database operation. Attempting to show database error page. + /// + internal static string DatabaseErrorPage_Matched + { + get { return GetString("DatabaseErrorPage_Matched"); } + } + + /// + /// Entity Framework recorded that the current exception was due to a failed database operation. Attempting to show database error page. + /// + internal static string FormatDatabaseErrorPage_Matched() + { + return GetString("DatabaseErrorPage_Matched"); + } + + /// + /// Entity Framework did not record any exceptions due to failed database operations. This means the current exception is not a failed Entity Framework database operation, or the current exception occurred from a DbContext that was not obtained from request services. + /// + internal static string DatabaseErrorPage_NoRecordedException + { + get { return GetString("DatabaseErrorPage_NoRecordedException"); } + } + + /// + /// Entity Framework did not record any exceptions due to failed database operations. This means the current exception is not a failed Entity Framework database operation, or the current exception occurred from a DbContext that was not obtained from request services. + /// + internal static string FormatDatabaseErrorPage_NoRecordedException() + { + return GetString("DatabaseErrorPage_NoRecordedException"); + } + + /// + /// The target data store is not a relational database. Skipping the database error page. + /// + internal static string DatabaseErrorPage_NotRelationalDatabase + { + get { return GetString("DatabaseErrorPage_NotRelationalDatabase"); } + } + + /// + /// The target data store is not a relational database. Skipping the database error page. + /// + internal static string FormatDatabaseErrorPage_NotRelationalDatabase() + { + return GetString("DatabaseErrorPage_NotRelationalDatabase"); + } + + /// + /// The current exception (and its inner exceptions) do not match the last exception Entity Framework recorded due to a failed database operation. This means the database operation exception was handled and another exception occurred later in the request. + /// + internal static string DatabaseErrorPage_NoMatch + { + get { return GetString("DatabaseErrorPage_NoMatch"); } + } + + /// + /// The current exception (and its inner exceptions) do not match the last exception Entity Framework recorded due to a failed database operation. This means the database operation exception was handled and another exception occurred later in the request. + /// + internal static string FormatDatabaseErrorPage_NoMatch() + { + return GetString("DatabaseErrorPage_NoMatch"); + } + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Microsoft.AspNet.Diagnostics.Entity/Strings.resx b/src/Microsoft.AspNet.Diagnostics.Entity/Strings.resx index 12b4868cf5..7e57f9d7b1 100644 --- a/src/Microsoft.AspNet.Diagnostics.Entity/Strings.resx +++ b/src/Microsoft.AspNet.Diagnostics.Entity/Strings.resx @@ -201,4 +201,19 @@ To use migrations from a command prompt you will need to <a href='http://go.microsoft.com/fwlink/?LinkId=518242'>install K Version Manager (KVM)</a>. Once installed, you can run migration commands from a standard command prompt in the project directory. + + {0} occurred, checking if Entity Framework recorded this exception as resulting from a failed database operation. + + + Entity Framework recorded that the current exception was due to a failed database operation. Attempting to show database error page. + + + Entity Framework did not record any exceptions due to failed database operations. This means the current exception is not a failed Entity Framework database operation, or the current exception occurred from a DbContext that was not obtained from request services. + + + The target data store is not a relational database. Skipping the database error page. + + + The current exception (and its inner exceptions) do not match the last exception Entity Framework recorded due to a failed database operation. This means the database operation exception was handled and another exception occurred later in the request. + \ No newline at end of file diff --git a/test/Microsoft.AspNet.Diagnostics.Entity.FunctionalTests/DatabaseErrorPageMiddlewareTest.cs b/test/Microsoft.AspNet.Diagnostics.Entity.FunctionalTests/DatabaseErrorPageMiddlewareTest.cs index 61516a7a97..e62828c312 100644 --- a/test/Microsoft.AspNet.Diagnostics.Entity.FunctionalTests/DatabaseErrorPageMiddlewareTest.cs +++ b/test/Microsoft.AspNet.Diagnostics.Entity.FunctionalTests/DatabaseErrorPageMiddlewareTest.cs @@ -338,6 +338,41 @@ namespace Microsoft.AspNet.Diagnostics.Entity.Tests } } + [Fact] + public async Task Error_page_displayed_when_exception_wrapped() + { + TestServer server = SetupTestServer(); + HttpResponseMessage response = await server.CreateClient().GetAsync("http://localhost/"); + + Assert.Equal(HttpStatusCode.InternalServerError, response.StatusCode); + var content = await response.Content.ReadAsStringAsync(); + Assert.Contains("I wrapped your exception", content); + Assert.Contains(StringsHelpers.GetResourceString("FormatDatabaseErrorPage_NoDbOrMigrationsTitle", typeof(BloggingContext).Name), content); + } + + class WrappedExceptionMiddleware + { + public WrappedExceptionMiddleware(RequestDelegate next) + { } + + public virtual Task Invoke(HttpContext context) + { + using (var db = context.ApplicationServices.GetService()) + { + db.Blogs.Add(new Blog()); + try + { + db.SaveChanges(); + throw new Exception("SaveChanges should have thrown"); + } + catch (Exception ex) + { + throw new Exception("I wrapped your exception", ex); + } + } + } + } + private static TestServer SetupTestServer(ILoggerProvider logProvider = null) where TContext : DbContext {