abpframework / abp

Open-source web application framework for ASP.NET Core! Offers an opinionated architecture to build enterprise software solutions with best practices on top of the .NET. Provides the fundamental infrastructure, cross-cutting-concern implementations, startup templates, application modules, UI themes, tooling and documentation.
https://abp.io
GNU Lesser General Public License v3.0
12.95k stars 3.45k forks source link

Abp vNext exception handler defects & Improved solution #6761

Closed zwbdzb closed 3 years ago

zwbdzb commented 3 years ago

I am a senior abp user, when I use the abp vNext [exception handler] feature ,
I found some defects, and I try to give a Improved solution for the defect.

exception handler defects

The default abp scaffolding. use the Built-in exception handler , and default do not send detail info to clients。

We can enabled the SendExceptionsDetailsToClients = true to send the detail info to client,

{
"error": {
  "code": null,
  "message": "ERROR [42000] [Cloudera][ImpalaODBC] (360) Syntax error occurred during query execution: [HY000] : AnalysisException: Could not resolve column/field reference: 'ug_fed89221846a42dc8427932b2965a020'\n",
   "details": "OdbcException: ERROR [42000] [Cloudera][ImpalaODBC] (360) Syntax error occurred during query execution: [HY000] : AnalysisException: Could not resolve column/field reference: 'ug_fed89221846a42dc8427932b2965a020'\n\nSTACK TRACE: at Gridsum.EAP.Olap.ExecuteQueryLayer.HandleQueryAsync(QueryContext queryContext, CancellationToken cancellationToken) in /home/gitlab-runner/builds/ttRjAPVA/0/eap/website/app/src/Gridsum.EAP.DataQuery/Olap/ExecuteQueryLayer.cs:line 81\n at Gridsum.EAP.Olap.DistributedCacheLayer.HandleQueryAsync(QueryContext queryContext, CancellationToken cancellationToken) in /home/gitlab-runner/builds/ttRjAPVA/0/eap/website/app/src/Gridsum.EAP.DataQuery/Olap/DistributedCacheLayer.cs:line 50\n at Gridsum.EAP.DataQuery.AbstractQueryExecutor`1.ExecuteQueryAsync(TQuery query, DistributedCacheEntryOptions options, CancellationToken cancellationToken) in /home/gitlab-runner/builds/ttRjAPVA/0/eap/website/app/src/Gridsum.EAP.DataQuery/AbstractQueryExecutor.cs:line 60\n at Gridsum.EAP.DataQuery.AbstractQueryExecutor`1.ExecuteQueryAsync(TQuery query, CancellationToken token) in /home/gitlab-runner/builds/ttRjAPVA/0/eap/website/app/src/Gridsum.EAP.DataQuery/AbstractQueryExecutor.cs:line 47\n at Gridsum.EAP.Application.UserGroupService.ClearUserGroupUserAsync(UserGroupUpdateUserDto updateDto, String idshort, CancellationToken cancelToken) in /home/gitlab-runner/builds/ttRjAPVA/0/eap/website/app/src/Gridsum.EAP.Application/UserGroup/UserGroupService.cs:line 332\n at Gridsum.EAP.Application.UserGroupService.UpdateUserGroupUserAsync(UserGroupUpdateUserDto updateDto, CancellationToken cancelToken) in /home/gitlab-runner/builds/ttRjAPVA/0/eap/website/app/src/Gridsum.EAP.Application/UserGroup/UserGroupService.cs:line 370\n at Castle.DynamicProxy.AsyncInterceptorBase.ProceedAsynchronous[TResult](IInvocation invocation, IInvocationProceedInfo proceedInfo "42000] [Cloudera][ImpalaODBC] (360 "42000] [Cloudera][ImpalaODBC] (360) Syntax error occurred during query execution: [HY000] : AnalysisException: Could not resolve column/field reference: 'ug_fed89221846a42dc8427932b2965a020'\n\nSTACK TRACE: at Gridsum.EAP.Olap.ExecuteQueryLayer.HandleQueryAsync(QueryContext queryContext, CancellationToken cancellationToken) in /home/gitlab-runner/builds/ttRjAPVA/0/eap/website/app/src/Gridsum.EAP.DataQuery/Olap/ExecuteQueryLayer.cs:line 81\n at Gridsum.EAP.Olap.DistributedCacheLayer.HandleQueryAsync(QueryContext queryContext, CancellationToken cancellationToken) in /home/gitlab-runner/builds/ttRjAPVA/0/eap/website/app/src/Gridsum.EAP.DataQuery/Olap/DistributedCacheLayer.cs:line 50\n at Gridsum.EAP.DataQuery.AbstractQueryExecutor`1.ExecuteQueryAsync(TQuery query, DistributedCacheEntryOptions options, CancellationToken cancellationToken) in /home/gitlab-runner/builds/ttRjAPVA/0/eap/website/app/src/Gridsum.EAP.DataQuery/AbstractQueryExecutor.cs:line 60\n at Gridsum.EAP.DataQuery.AbstractQueryExecutor`1.ExecuteQueryAsync(TQuery query, CancellationToken token) in /home/gitlab-runner/builds/ttRjAPVA/0/eap/website/app/src/Gridsum.EAP.DataQuery/AbstractQueryExecutor.cs:line 47\n at Gridsum.EAP.Application.UserGroupService.ClearUserGroupUserAsync(UserGroupUpdateUserDto updateDto, String idshort, CancellationToken cancelToken) in /home/gitlab-runner/builds/ttRjAPVA/0/eap/website/app/src/Gridsum.EAP.Application/UserGroup/UserGroupService.cs:line 332\n at Gridsum.EAP.Application.UserGroupService.UpdateUserGroupUserAsync(UserGroupUpdateUserDto updateDto, CancellationToken cancelToken) in /home/gitlab-runner/builds/ttRjAPVA/0/eap/website/app/src/Gridsum.EAP.Application/UserGroup/UserGroupService.cs:line 370\n at Castle.DynamicProxy.AsyncInterceptorBase.ProceedAsynchronous[TResult") Syntax error occurred during query execution: [HY000] : AnalysisException: Could not resolve column/field reference: 'ug_fed89221846a42dc8427932b2965a020'\n\nSTACK TRACE: at Gridsum.EAP.Olap.ExecuteQueryLayer.HandleQueryAsync(QueryContext queryContext, CancellationToken cancellationToken) in /home/gitlab-runner/builds/ttRjAPVA/0/eap/website/app/src/Gridsum.EAP.DataQuery/Olap/ExecuteQueryLayer.cs:line 81\n at Gridsum.EAP.Olap.DistributedCacheLayer.HandleQueryAsync(QueryContext queryContext, CancellationToken cancellationToken) in /home/gitlab-runner/builds/ttRjAPVA/0/eap/website/app/src/Gridsum.EAP.DataQuery/Olap/DistributedCacheLayer.cs:line 50\n at Gridsum.EAP.DataQuery.AbstractQueryExecutor`1.ExecuteQueryAsync(TQuery query, DistributedCacheEntryOptions options, CancellationToken cancellationToken) in /home/gitlab-runner/builds/ttRjAPVA/0/eap/website/app/src/Gridsum.EAP.DataQuery/AbstractQueryExecutor.cs:line 60\n at Gridsum.EAP.DataQuery.AbstractQueryExecutor`1.ExecuteQueryAsync(TQuery query, CancellationToken token) in /home/gitlab-runner/builds/ttRjAPVA/0/eap/website/app/src/Gridsum.EAP.DataQuery/AbstractQueryExecutor.cs:line 47\n at Gridsum.EAP.Application.UserGroupService.ClearUserGroupUserAsync(UserGroupUpdateUserDto updateDto, String idshort, CancellationToken cancelToken) in /home/gitlab-runner/builds/ttRjAPVA/0/eap/website/app/src/Gridsum.EAP.Application/UserGroup/UserGroupService.cs:line 332\n at Gridsum.EAP.Application.UserGroupService.UpdateUserGroupUserAsync(UserGroupUpdateUserDto updateDto, CancellationToken cancelToken) in /home/gitlab-runner/builds/ttRjAPVA/0/eap/website/app/src/Gridsum.EAP.Application/UserGroup/UserGroupService.cs:line 370\n at Castle.DynamicProxy.AsyncInterceptorBase.ProceedAsynchronous[TResult")\n at Volo.Abp.Castle.DynamicProxy.CastleAbpMethodInvocationAdapterWithReturnValue`1.ProceedAsync()\n at Volo.Abp.Validation.ValidationInterceptor.InterceptAsync(IAbpMethodInvocation invocation)\n at Volo.Abp.Castle.DynamicProxy.CastleAsyncAbpInterceptorAdapter`1.InterceptAsync[TResult](IInvocation invocation, IInvocationProceedInfo proceedInfo, Func`3 proceed "TResult")\n at Castle.DynamicProxy.AsyncInterceptorBase.ProceedAsynchronous[TResult](IInvocation invocation, IInvocationProceedInfo proceedInfo "TResult")\n at Volo.Abp.Castle.DynamicProxy.CastleAbpMethodInvocationAdapterWithReturnValue`1.ProceedAsync()\n at Volo.Abp.Auditing.AuditingInterceptor.InterceptAsync(IAbpMethodInvocation invocation)\n at Volo.Abp.Castle.DynamicProxy.CastleAsyncAbpInterceptorAdapter`1.InterceptAsync[TResult](IInvocation invocation, IInvocationProceedInfo proceedInfo, Func`3 proceed "TResult")\n at Castle.DynamicProxy.AsyncInterceptorBase.ProceedAsynchronous[TResult](IInvocation invocation, IInvocationProceedInfo proceedInfo "TResult")\n at Volo.Abp.Castle.DynamicProxy.CastleAbpMethodInvocationAdapterWithReturnValue`1.ProceedAsync()\n at Volo.Abp.Uow.UnitOfWorkInterceptor.InterceptAsync(IAbpMethodInvocation invocation)\n at Volo.Abp.Castle.DynamicProxy.CastleAsyncAbpInterceptorAdapter`1.InterceptAsync[TResult](IInvocation invocation, IInvocationProceedInfo proceedInfo, Func`3 proceed "TResult")\n at Gridsum.EAP.Controllers.UserGroupController.UpdateUserGroupUserAsync(String id, CancellationToken cancelToken) in /home/gitlab-runner/builds/ttRjAPVA/0/eap/website/app/src/Gridsum.EAP.HttpApi/Controllers/UserGroupController.cs:line 320\n at lambda_method3440(Closure , Object )\n at Microsoft.AspNetCore.Mvc.Infrastructure.ActionMethodExecutor.AwaitableObjectResultExecutor.Execute(IActionResultTypeMapper mapper, ObjectMethodExecutor executor, Object controller, Object[] arguments)\n at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeActionMethodAsync>g__Awaited|12_0(ControllerActionInvoker invoker, ValueTask`1 actionResultValueTask)\n at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeNextActionFilterAsync>g__Awaited|10_0(ControllerActionInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)\n at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Rethrow(ActionExecutedContextSealed context)\n at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)\n at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeInnerFilterAsync>g__Awaited|13_0(ControllerActionInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)\n at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeNextExceptionFilterAsync>g__Awaited|25_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)\n",
  "data": null,
   "validationErrors": null
  }
}

but the abp [ exception handler] have the following defects

  1. Do not Automatically handles all exceptions , need to meet the any of the following conditions QQ截图20201222225408

    when the controller action output is not object result , the abp can not capture the action exception! I don't know why there are such restrictions , I think this is a bug!

  2. The output info have no request TraceIdentitier, any of the following conditions

  3. The output info message details field is tediously long , Not suitable for frontend tool tip

I think the above requirement is the common exception handler & display mode.

AbpExceptionFilter Source code

I have see the [AbpExceptionFilter] source code , The source code confirm the defects . source code

Improved solution

We can replace the default [AbpExceptionFilter], and resolve the bug. step1:

public class EapExceptionFilter : IFilterMetadata, IAsyncExceptionFilter, ITransientDependency
    {
        public ILogger<EapExceptionFilter> Logger { get; set; }

        private readonly IExceptionToErrorInfoConverter _errorInfoConverter;
        private readonly IHttpExceptionStatusCodeFinder _statusCodeFinder;
        private readonly IJsonSerializer _jsonSerializer;
        private readonly AbpExceptionHandlingOptions _exceptionHandlingOptions;

        public EapExceptionFilter(
            IExceptionToErrorInfoConverter errorInfoConverter,
            IHttpExceptionStatusCodeFinder statusCodeFinder,
            IJsonSerializer jsonSerializer,
            IOptions<AbpExceptionHandlingOptions> exceptionHandlingOptions)
        {
            _errorInfoConverter = errorInfoConverter;
            _statusCodeFinder = statusCodeFinder;
            _jsonSerializer = jsonSerializer;
            _exceptionHandlingOptions = exceptionHandlingOptions.Value;

            Logger = NullLogger<EapExceptionFilter>.Instance;
        }

        public async Task OnExceptionAsync(ExceptionContext context)
        {
            if (!ShouldHandleException(context))
            {
                return;
            }

            await HandleAndWrapException(context);
        }

        protected virtual bool ShouldHandleException(ExceptionContext context)
        {
            if (context.ActionDescriptor.IsControllerAction() 
                // &&
                // context.ActionDescriptor.HasObjectResult()
                    )
            {
                return true;
            }

            if (context.HttpContext.Request.CanAccept(MimeTypes.Application.Json))
            {
                return true;
            }

            if (context.HttpContext.Request.IsAjax())
            {
                return true;
            }

            return false;
        }

        protected virtual async Task HandleAndWrapException(ExceptionContext context)
        {
            //TODO: Trigger an AbpExceptionHandled event or something like that.

            context.HttpContext.Response.Headers.Add(AbpHttpConsts.AbpErrorFormat, "true");
            context.HttpContext.Response.StatusCode = (int)_statusCodeFinder.GetStatusCode(context.HttpContext, context.Exception);

            var remoteServiceErrorInfo = _errorInfoConverter.Convert(context.Exception, _exceptionHandlingOptions.SendExceptionsDetailsToClients);
            remoteServiceErrorInfo.Code = context.HttpContext.TraceIdentifier;
            remoteServiceErrorInfo.Message = SimplifyMessage(context.Exception);
            // HttpResponse
            context.Result = new ObjectResult(new RemoteServiceErrorResponse(remoteServiceErrorInfo));

            // 写日志
            var logLevel = context.Exception.GetLogLevel();
            var remoteServiceErrorInfoBuilder = new StringBuilder();
            remoteServiceErrorInfoBuilder.AppendLine($"---------- {nameof(RemoteServiceErrorInfo)} ----------");
            remoteServiceErrorInfoBuilder.AppendLine(_jsonSerializer.Serialize(remoteServiceErrorInfo, indented: true));
            Logger.LogWithLevel(logLevel, remoteServiceErrorInfoBuilder.ToString());
            Logger.LogException(context.Exception, logLevel);

            await context.HttpContext
                .RequestServices
                .GetRequiredService<IExceptionNotifier>()
                .NotifyAsync(
                    new ExceptionNotificationContext(context.Exception)
                );

            context.Exception = null; //Handled!
        }

        protected string SimplifyMessage(Exception error)
        {
            string message = string.Empty;
            switch (error)
            {
                case AbpAuthorizationException e:
                    return message = "Authenticate failure!";
                case AbpValidationException e:
                    return message = "Request param validate failure!";
                case EntityNotFoundException e:
                    return message = "not found the entity!";
                case BusinessException e:
                    return  message = $"business exception!{e.Message}";
                case NotImplementedException e:
                    return  message = "not implement!";
                default:
                    return  message = "server internal error!";
            }
        }
    }

strp2

replace the default [AbpExceptionFilter] to the new ExceptionFilter:

 context.Services.AddMvc(options =>
{
        options.Filters.ReplaceOne(x=> (x as ServiceFilterAttribute)?.ServiceType?.Name==nameof(AbpExceptionFilter), new ServiceFilterAttribute(typeof(EapExceptionFilter)));  
})

I don't know why abp use the exception strategy, I think the above solution is more useful in practice!

maliming commented 3 years ago

This is by design, you can customize it according to your needs.

zwbdzb commented 3 years ago

The design is unreasonable, The Offical doc says the abp capture the all exception,
but can not capture the most common exception。

The Abp request must meet the any of the three requirements。

maliming commented 3 years ago

image

Like I said you can customize it according to your needs.