Error handling fixed.

backend/src/Squidex.Web/ApiExceptionConverter.cs

@@ -12,6 +12,7 @@ using System.Linq;
 using System.Security;
 using System.Text;
 using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Mvc;
 using Squidex.Infrastructure;
 using Squidex.Infrastructure.Validation;
 
@@ -19,7 +20,6 @@ namespace Squidex.Web
 {
     public static class ApiExceptionConverter
     {
-        private static readonly List<Func<Exception, ErrorDto?>> Handlers = new List<Func<Exception, ErrorDto?>>();
         private static readonly Dictionary<int, string> Links = new Dictionary<int, string>
         {
             [400] = "https://tools.ietf.org/html/rfc7231#section-6.5.1",
@@ -34,84 +34,104 @@ namespace Squidex.Web
             [500] = "https://tools.ietf.org/html/rfc7231#section-6.6.1"
         };
 
-        private static void AddHandler<T>(Func<T, ErrorDto> handler) where T : Exception
+        public static (ErrorDto Error, bool WellKnown) ToErrorDto(this ProblemDetails problem, HttpContext? httpContext)
         {
-            Handlers.Add(ex => ex is T typed ? handler(typed) : null);
-        }
+            Guard.NotNull(problem);
 
-        static ApiExceptionConverter()
-        {
-            AddHandler<ValidationException>(OnValidationException);
-            AddHandler<DecoderFallbackException>(OnDecoderException);
-            AddHandler<DomainObjectNotFoundException>(OnDomainObjectNotFoundException);
-            AddHandler<DomainObjectVersionException>(OnDomainObjectVersionException);
-            AddHandler<DomainForbiddenException>(OnDomainForbiddenException);
-            AddHandler<DomainException>(OnDomainException);
-            AddHandler<SecurityException>(OnSecurityException);
+            var error = new ErrorDto { Message = problem.Title, StatusCode = problem.Status };
+
+            Enrich(httpContext, error);
+
+            return (error, true);
         }
 
-        public static ErrorDto ToErrorDto(this Exception exception, HttpContext? httpContext)
+        public static (ErrorDto Error, bool WellKnown) ToErrorDto(this Exception exception, HttpContext? httpContext)
         {
             Guard.NotNull(exception);
 
-            foreach (var handler in Handlers)
-            {
-                var result = handler(exception);
-
-                if (result != null)
-                {
-                    result.TraceId = Activity.Current?.Id ?? httpContext?.TraceIdentifier;
-
-                    if (result.StatusCode.HasValue)
-                    {
-                        result.Type = Links.GetOrDefault(result.StatusCode.Value);
-                    }
+            var result = CreateError(exception);
 
-                    return result;
-                }
-            }
+            Enrich(httpContext, result.Error);
 
-            return new ErrorDto { StatusCode = 500 };
+            return result;
         }
 
-        private static ErrorDto OnDecoderException(DecoderFallbackException ex)
+        private static void Enrich(HttpContext? httpContext, ErrorDto error)
         {
-            return new ErrorDto { StatusCode = 400, Message = ex.Message };
-        }
+            error.TraceId = Activity.Current?.Id ?? httpContext?.TraceIdentifier;
 
-        private static ErrorDto OnDomainObjectNotFoundException(DomainObjectNotFoundException ex)
-        {
-            return new ErrorDto { StatusCode = 404 };
+            if (error.StatusCode.HasValue)
+            {
+                error.Type = Links.GetOrDefault(error.StatusCode.Value);
+            }
         }
 
-        private static ErrorDto OnDomainObjectVersionException(DomainObjectVersionException ex)
+        private static (ErrorDto Error, bool WellKnown) CreateError(Exception exception)
         {
-            return new ErrorDto { StatusCode = 412, Message = ex.Message };
-        }
+            switch (exception)
+            {
+                case ValidationException ex:
+                    return (new ErrorDto
+                    {
+                        StatusCode = 400,
+                        Message = ex.Summary,
+                        Details = ToDetails(ex)
+                    }, true);
 
-        private static ErrorDto OnDomainException(DomainException ex)
-        {
-            return new ErrorDto { StatusCode = 400, Message = ex.Message };
-        }
+                case DomainObjectNotFoundException _:
+                    return (new ErrorDto
+                    {
+                        StatusCode = 404,
+                        Message = null!
+                    }, true);
 
-        private static ErrorDto OnDomainForbiddenException(DomainForbiddenException ex)
-        {
-            return new ErrorDto { StatusCode = 403, Message = ex.Message };
-        }
+                case DomainObjectVersionException _:
+                    return (new ErrorDto
+                    {
+                        StatusCode = 412,
+                        Message = exception.Message
+                    }, true);
 
-        private static ErrorDto OnSecurityException(SecurityException ex)
-        {
-            return new ErrorDto { StatusCode = 403, Message = ex.Message };
-        }
+                case DomainForbiddenException _:
+                    return (new ErrorDto
+                    {
+                        StatusCode = 403,
+                        Message = exception.Message
+                    }, true);
 
-        private static ErrorDto OnValidationException(ValidationException ex)
-        {
-            return new ErrorDto { StatusCode = 400, Message = ex.Summary, Details = ToDetails(ex) };
+                case DomainException _:
+                    return (new ErrorDto
+                    {
+                        StatusCode = 400,
+                        Message = exception.Message
+                    }, true);
+
+                case SecurityException _:
+                    return (new ErrorDto
+                    {
+                        StatusCode = 403,
+                        Message = "Forbidden"
+                    }, false);
+
+                case DecoderFallbackException _:
+                    return (new ErrorDto
+                    {
+                        StatusCode = 400,
+                        Message = exception.Message
+                    }, true);
+
+                default:
+                    return (new ErrorDto
+                    {
+                        StatusCode = 500,
+                        Message = "Server Error"
+                    }, false);
+            }
         }
 
         private static string[] ToDetails(ValidationException ex)
         {
-            return ex.Errors?.Select(e =>
+            return ex.Errors.Select(e =>
             {
                 if (e.PropertyNames?.Any() == true)
                 {
@@ -121,7 +141,7 @@ namespace Squidex.Web
                 {
                     return e.Message;
                 }
-            }).ToArray() ?? new string[0];
+            }).ToArray();
         }
     }
 }

backend/src/Squidex.Web/ApiExceptionFilterAttribute.cs

@@ -8,6 +8,8 @@
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.AspNetCore.Mvc.Filters;
+using Microsoft.Extensions.DependencyInjection;
+using Squidex.Infrastructure.Log;
 
 namespace Squidex.Web
 {
@@ -19,32 +21,40 @@ namespace Squidex.Web
 
             if (resultContext.Result is ObjectResult objectResult && objectResult.Value is ProblemDetails problem)
             {
-                var error = new ErrorDto { Message = problem.Title, Type = problem.Type, StatusCode = problem.Status };
+                var (error, _) = problem.ToErrorDto(context.HttpContext);
 
-                if (problem.Extensions.TryGetValue("traceId", out var temp) && temp is string traceId)
-                {
-                    error.TraceId = traceId;
-                }
-
-                objectResult.Value = error;
+                context.Result = GetResult(error);
             }
         }
 
         public void OnException(ExceptionContext context)
         {
-            var error = context.Exception.ToErrorDto(context.HttpContext);
+            var (error, wellKnown) = context.Exception.ToErrorDto(context.HttpContext);
 
-            if (error.StatusCode == 404)
+            if (!wellKnown)
             {
-                context.Result = new NotFoundResult();
+                var log = context.HttpContext.RequestServices.GetService<ISemanticLog>();
+
+                if (log != null)
+                {
+                    log.LogError(context.Exception, w => w.WriteProperty("status", "UnhandledException"));
+                }
             }
-            else
+
+            context.Result = GetResult(error);
+        }
+
+        private static IActionResult GetResult(ErrorDto error)
+        {
+            if (error.StatusCode == 404)
             {
-                context.Result = new ObjectResult(error)
-                {
-                    StatusCode = error.StatusCode
-                };
+                return new NotFoundResult();
             }
+
+            return new ObjectResult(error)
+            {
+                StatusCode = error.StatusCode
+            };
         }
     }
 }

backend/src/Squidex.Web/CommandMiddlewares/EnrichWithActorCommandMiddleware.cs

@@ -41,7 +41,7 @@ namespace Squidex.Web.CommandMiddlewares
                 {
                     var actorToken = user.Token();
 
-                    squidexCommand.Actor = actorToken ?? throw new SecurityException("No actor with subject or client id available.");
+                    squidexCommand.Actor = actorToken ?? throw new DomainForbiddenException("No actor with subject or client id available.");
                 }
 
                 if (squidexCommand.User == null)

backend/src/Squidex.Web/Pipeline/RequestLogPerformanceMiddleware.cs

@@ -5,6 +5,7 @@
 //  All rights reserved. Licensed under the MIT license.
 // ==========================================================================
 
+using System;
 using System.Threading.Tasks;
 using Microsoft.AspNetCore.Http;
 using Microsoft.Extensions.Options;
@@ -39,6 +40,12 @@ namespace Squidex.Web.Pipeline
                 {
                     await next(context);
                 }
+                catch (Exception ex)
+                {
+                    log.LogError(ex, w => w.WriteProperty("status", "UnhandledException"));
+
+                    context.Response.StatusCode = 500;
+                }
                 finally
                 {
                     var elapsedMs = watch.Stop();

backend/src/Squidex/Areas/Api/Controllers/Contents/Models/ImportResultDto.cs

@@ -26,7 +26,9 @@ namespace Squidex.Areas.Api.Controllers.Contents.Models
 
         public static ImportResultDto FromImportResult(ImportResultItem result, HttpContext httpContext)
         {
-            return new ImportResultDto { ContentId = result.ContentId, Error = result.Exception?.ToErrorDto(httpContext) };
+            var error = result.Exception?.ToErrorDto(httpContext).Error;
+
+            return new ImportResultDto { ContentId = result.ContentId, Error = error };
         }
     }
 }

backend/src/Squidex/Areas/IdentityServer/Controllers/Account/AccountController.cs

@@ -71,7 +71,7 @@ namespace Squidex.Areas.IdentityServer.Controllers.Account
         [Route("account/forbidden/")]
         public IActionResult Forbidden()
         {
-            throw new SecurityException("User is not allowed to login.");
+            throw new DomainForbiddenException("User is not allowed to login.");
         }
 
         [HttpGet]

backend/tests/Squidex.Web.Tests/ApiExceptionFilterAttributeTests.cs

@@ -8,13 +8,16 @@
 using System;
 using System.Collections.Generic;
 using System.Security;
+using System.Text;
 using System.Threading.Tasks;
+using FakeItEasy;
 using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc;
 using Microsoft.AspNetCore.Mvc.Abstractions;
 using Microsoft.AspNetCore.Mvc.Filters;
 using Microsoft.AspNetCore.Routing;
 using Squidex.Infrastructure;
+using Squidex.Infrastructure.Log;
 using Squidex.Infrastructure.Validation;
 using Xunit;
 
@@ -22,18 +25,9 @@ namespace Squidex.Web
 {
     public class ApiExceptionFilterAttributeTests
     {
+        private readonly ISemanticLog log = A.Fake<ISemanticLog>();
         private readonly ApiExceptionFilterAttribute sut = new ApiExceptionFilterAttribute();
 
-        [Fact]
-        public void Should_generate_404_for_DomainObjectNotFoundException()
-        {
-            var context = E(new DomainObjectNotFoundException("1", typeof(object)));
-
-            sut.OnException(context);
-
-            Assert.IsType<NotFoundResult>(context.Result);
-        }
-
         [Fact]
         public void Should_generate_400_for_ValidationException()
         {
@@ -42,7 +36,7 @@ namespace Squidex.Web
                 new ValidationError("Error2", "P"),
                 new ValidationError("Error3", "P1", "P2"));
 
-            var context = E(ex);
+            var context = Error(ex);
 
             sut.OnException(context);
 
@@ -54,73 +48,134 @@ namespace Squidex.Web
             Assert.Equal(ex.Summary, ((ErrorDto)result.Value).Message);
 
             Assert.Equal(new[] { "Error1", "P: Error2", "P1, P2: Error3" }, ((ErrorDto)result.Value).Details);
+
+            A.CallTo(() => log.Log(SemanticLogLevel.Error, None.Value, A<Action<None, IObjectWriter>>._))
+                .MustNotHaveHappened();
+        }
+
+        [Fact]
+        public void Should_generate_404_for_DomainObjectNotFoundException()
+        {
+            var context = Error(new DomainObjectNotFoundException("1", typeof(object)));
+
+            sut.OnException(context);
+
+            Assert.IsType<NotFoundResult>(context.Result);
+
+            A.CallTo(() => log.Log(SemanticLogLevel.Error, None.Value, A<Action<None, IObjectWriter>>._))
+                .MustNotHaveHappened();
+        }
+
+        [Fact]
+        public void Should_generate_500_and_log_for_unknown_error()
+        {
+            var context = Error(new InvalidOperationException());
+
+            sut.OnException(context);
+
+            Validate(500, context.Result, null);
+
+            A.CallTo(() => log.Log(SemanticLogLevel.Error, None.Value, A<Action<None, IObjectWriter>>._))
+                .MustHaveHappened();
         }
 
         [Fact]
         public void Should_generate_400_for_DomainException()
         {
-            var context = E(new DomainException("NotAllowed"));
+            var context = Error(new DomainException("NotAllowed"));
 
             sut.OnException(context);
 
             Validate(400, context.Result, context.Exception);
+
+            A.CallTo(() => log.Log(SemanticLogLevel.Error, None.Value, A<Action<None, IObjectWriter>>._))
+                .MustNotHaveHappened();
+        }
+
+        [Fact]
+        public void Should_generate_400_for_DecoderFallbackException()
+        {
+            var context = Error(new DecoderFallbackException("Decoder"));
+
+            sut.OnException(context);
+
+            Validate(400, context.Result, context.Exception);
+
+            A.CallTo(() => log.Log(SemanticLogLevel.Error, None.Value, A<Action<None, IObjectWriter>>._))
+                .MustNotHaveHappened();
         }
 
         [Fact]
         public void Should_generate_412_for_DomainObjectVersionException()
         {
-            var context = E(new DomainObjectVersionException("1", typeof(object), 1, 2));
+            var context = Error(new DomainObjectVersionException("1", typeof(object), 1, 2));
 
             sut.OnException(context);
 
             Validate(412, context.Result, context.Exception);
+
+            A.CallTo(() => log.Log(SemanticLogLevel.Error, None.Value, A<Action<None, IObjectWriter>>._))
+                .MustNotHaveHappened();
         }
 
         [Fact]
         public void Should_generate_403_for_DomainForbiddenException()
         {
-            var context = E(new DomainForbiddenException("Forbidden"));
+            var context = Error(new DomainForbiddenException("Forbidden"));
 
             sut.OnException(context);
 
             Validate(403, context.Result, context.Exception);
+
+            A.CallTo(() => log.Log(SemanticLogLevel.Error, None.Value, A<Action<None, IObjectWriter>>._))
+                .MustNotHaveHappened();
         }
 
         [Fact]
-        public void Should_generate_403_for_SecurityException()
+        public void Should_generate_403_and_log_for_SecurityException()
         {
-            var context = E(new SecurityException("Forbidden"));
+            var context = Error(new SecurityException());
 
             sut.OnException(context);
 
-            Validate(403, context.Result, context.Exception);
+            Validate(403, context.Result, null);
+
+            A.CallTo(() => log.Log(SemanticLogLevel.Error, None.Value, A<Action<None, IObjectWriter>>._))
+                .MustHaveHappened();
         }
 
         [Fact]
         public async Task Should_unify_errror()
         {
-            var context = R(new ProblemDetails { Status = 403, Type = "type" });
+            var context = Problem(new ProblemDetails { Status = 403, Type = "type" });
 
             await sut.OnResultExecutionAsync(context, () => Task.FromResult(Result(context)));
 
             Validate(403, context.Result, null);
+
+            A.CallTo(() => log.Log(SemanticLogLevel.Error, None.Value, A<Action<None, IObjectWriter>>._))
+                .MustNotHaveHappened();
         }
 
-        private static ResultExecutedContext Result(ResultExecutingContext context)
+        private ResultExecutedContext Result(ResultExecutingContext context)
         {
             var actionContext = ActionContext();
 
-            return new ResultExecutedContext(actionContext, new List<IFilterMetadata>(), context.Result, context.Controller);
+            var result = context.Result;
+
+            return new ResultExecutedContext(actionContext, new List<IFilterMetadata>(), result, context.Controller);
         }
 
-        private static ResultExecutingContext R(ProblemDetails problem)
+        private ResultExecutingContext Problem(ProblemDetails problem)
         {
             var actionContext = ActionContext();
 
-            return new ResultExecutingContext(actionContext, new List<IFilterMetadata>(), new ObjectResult(problem) { StatusCode = problem.Status }, null);
+            var result = new ObjectResult(problem) { StatusCode = problem.Status };
+
+            return new ResultExecutingContext(actionContext, new List<IFilterMetadata>(), result, null);
         }
 
-        private static ExceptionContext E(Exception exception)
+        private ExceptionContext Error(Exception exception)
         {
             var actionContext = ActionContext();
 
@@ -130,9 +185,17 @@ namespace Squidex.Web
             };
         }
 
-        private static ActionContext ActionContext()
+        private ActionContext ActionContext()
         {
-            var httpContext = new DefaultHttpContext();
+            var services = A.Fake<IServiceProvider>();
+
+            A.CallTo(() => services.GetService(typeof(ISemanticLog)))
+                .Returns(log);
+
+            var httpContext = new DefaultHttpContext
+            {
+                RequestServices = services
+            };
 
             var actionContext = new ActionContext(httpContext, new RouteData(), new ActionDescriptor
             {

backend/tests/Squidex.Web.Tests/CommandMiddlewares/EnrichWithActorCommandMiddlewareTests.cs

@@ -39,7 +39,7 @@ namespace Squidex.Web.CommandMiddlewares
             var command = new CreateContent();
             var context = Ctx(command);
 
-            await Assert.ThrowsAsync<SecurityException>(() => sut.HandleAsync(context));
+            await Assert.ThrowsAsync<DomainForbiddenException>(() => sut.HandleAsync(context));
         }
 
         [Fact]