Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to provide correct exception message to client #16

Closed
kadyrkulovr opened this issue Jan 19, 2021 · 3 comments
Closed

How to provide correct exception message to client #16

kadyrkulovr opened this issue Jan 19, 2021 · 3 comments

Comments

@kadyrkulovr
Copy link

kadyrkulovr commented Jan 19, 2021

Hi,

What is the best way to provide exception message to the client?

Example:

Request:

<soapenv:Envelope xmlns:soapenv="http://schemas.xmlsoap.org/soap/envelope/" xmlns:xro="http://x-road.eu/xsd/xroad.xsd" xmlns:iden="http://x-road.eu/xsd/identifiers" xmlns:cal="http://calculator.x-road.eu/">
   <soapenv:Header>
      ...
   </soapenv:Header>
   <soapenv:Body>
      <cal:Calculate>
         <request>
            <X>2</X>
            <Y>0</Y>
            <Operation>Divide</Operation>
         </request>
      </cal:Calculate>
   </soapenv:Body>
</soapenv:Envelope>

Response:

<SOAP-ENV:Envelope xmlns:SOAP-ENV="http://schemas.xmlsoap.org/soap/envelope/">
   <SOAP-ENV:Body>
      <SOAP-ENV:Fault>
         <faultcode>Server.InternalError</faultcode>
         <faultstring>Exception has been thrown by the target of an invocation.</faultstring>
      </SOAP-ENV:Fault>
   </SOAP-ENV:Body>
</SOAP-ENV:Envelope>

Question:
You can see that the server return message Exception has been thrown by the target of an invocation.
How can I provide client with message: Attempted to divide by zero?

image

Thank you.

Update:

Adding stack trace.

XRoadLib.Extensions.AspNetCore.XRoadLibMiddleware: Error: Unexpected error while processing X-Road request

System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation.
 ---> System.DivideByZeroException: Attempted to divide by zero.
   at Calculator.WebService.CalculateWebService.<>c.<GetOperation>b__1_3(Int32 x, Int32 y) in E:\xRoad\XRoadLib-master\samples\Calculator\WebService\CalculateWebService.cs:line 18
   at Calculator.WebService.CalculateWebService.Calculate(CalculationRequest request) in E:\xRoad\XRoadLib-master\samples\Calculator\WebService\CalculateWebService.cs:line 10
   --- End of inner exception stack trace ---
   at System.RuntimeMethodHandle.InvokeMethod(Object target, Object[] arguments, Signature sig, Boolean constructor, Boolean wrapExceptions)
   at System.Reflection.RuntimeMethodInfo.Invoke(Object obj, BindingFlags invokeAttr, Binder binder, Object[] parameters, CultureInfo culture)
   at System.Reflection.MethodBase.Invoke(Object obj, Object[] parameters)
   at XRoadLib.Extensions.AspNetCore.WebServiceRequestHandler.InvokeRuntimeMethodAsync(WebServiceContext context, Object serviceObject, Object[] parameters) in E:\xRoad\XRoadLib-master\src\XRoadLib.Extensions.AspNetCore\WebServiceRequestHandler.cs:line 162
   at XRoadLib.Extensions.AspNetCore.WebServiceRequestHandler.InvokeWebServiceAsync(WebServiceContext context) in E:\xRoad\XRoadLib-master\src\XRoadLib.Extensions.AspNetCore\WebServiceRequestHandler.cs:line 149
   at XRoadLib.Extensions.AspNetCore.WebServiceRequestHandler.InvokeWebServiceAsync(WebServiceContext context) in E:\xRoad\XRoadLib-master\src\XRoadLib.Extensions.AspNetCore\WebServiceRequestHandler.cs:line 157
   at XRoadLib.Extensions.AspNetCore.WebServiceRequestHandler.HandleRequestAsync(WebServiceContext context) in E:\xRoad\XRoadLib-master\src\XRoadLib.Extensions.AspNetCore\WebServiceRequestHandler.cs:line 48
   at XRoadLib.Extensions.AspNetCore.XRoadLibMiddleware.Invoke(HttpContext httpContext, IWebServiceHandler handler) in E:\xRoad\XRoadLib-master\src\XRoadLib.Extensions.AspNetCore\XRoadLibMiddleware.cs:line 35

On screen below in method SoapMessageFormatter.CreateFault(Exception exception) you can see that DivideByZeroException inside InnerExceptionProperty.

image

@kadyrkulovr kadyrkulovr changed the title How to throw exception messages How to provide correct exception message to client Jan 19, 2021
@kadyrkulovr
Copy link
Author

Could you add support for this type exceptions?

something like:

//in method XRoadLibMiddleware.Invoke

//from
//var fault = context.MessageFormatter.CreateFault(exception);

//to
var fault = exception is System.Reflection.TargetInvocationException
                    ? context.MessageFormatter.CreateFault(exception.InnerException)
                    : context.MessageFormatter.CreateFault(exception);

or maybe there is better approach?

@janno-p
Copy link
Owner

janno-p commented Feb 22, 2021

Hi,

First I have to apologize for such a long wait. Somehow I had muted notifications in this project and only by chance I'm responding at the moment. Hope, I got everything enabled again, so I'm more prepared next time.

About exceptions and error handling:

First thing to know, is that you should consider two types of exceptions: technical and non-technical errors.

Technical errors

Technical errors are usually mistakes in code or unhandled edge cases, which should be review-d and, if possible, fixed in application so that they couldn't happen again. In this library they appear as unhandled exceptions which bubble up to service request handler. The handler catches the exception and serializes it in the response as SOAP fault (as in your response sample). Usually these errors require development effort to fix.

Non-technical errors

These are errors, which are returned on purpose. Usually in return to invalid use input, some detected erroneous state in application or some other reason, when application has detected that it cannot continue with given input. Usually they are related to user input in given request and can be overcome by changing the request as required by the application.

Customizing exceptional behavior

By default, every exception is handled as technical error, but it's possible to handle the exception before serialization to get more appropriate behavior for certain system. For that you have to provide your own implementation of request handler by overriding the default one:

public class CustomWebRequestHandler : WebServiceRequestHandler
{
    public CustomWebRequestHandler(IServiceProvider serviceProvider, IServiceManager serviceManager)
        : base(serviceProvider, serviceManager)
    { }

    protected override Task OnInvocationErrorAsync(WebServiceContext context)
    {
        if (context.Exception is TargetInvocationException ex && ex.InnerException != null)
            ExceptionDispatchInfo.Capture(ex.InnerException).Throw();

        return Task.CompletedTask;
    }
}

To use this handler instead of default one, change the endpoints configuration in startup class:

// endpoints.MapGet("/", c => c.ExecuteWsdlRequest<CalculatorServiceManager>());
endpoints.MapGet("/", c => c.ExecuteWsdlRequest<IServiceManager>());
// endpoints.MapPost("/", c => c.ExecuteWebServiceRequest<CalculatorServiceManager>());

endpoints.MapPost("/", c =>
{
    using var handler = c.RequestServices.GetRequiredService<CustomWebRequestHandler>();
    return XRoadLibMiddleware.Invoke(c, handler);
});

Also some dependencies need to be set up in startup class aswell:

// services.AddSingleton<CalculatorServiceManager>();
services.AddSingleton<IServiceManager, CalculatorServiceManager>();

services.AddScoped<CustomWebRequestHandler>();

This should get your initial proposal working (by revealing innerexception to client)

Recommendations

In real application it might not be the best idea to display unexpected exceptions to clients (because their messages might reveal delicate info about application internals). For technical errors I would recommend instead to generate unique id for the exception, log it somewhere you can access it later, and display some generic message with error id to client.

protected override Task OnInvocationErrorAsync(WebServiceContext context)
{
    if (IsBusinessError(context.Exception))
    {
        context.Result = MapToClientError(context.Exception);
        return Task.CompletedTask;
    }

    Guid internalErrorId = Guid.NewGuid();

    // log message with internalErrorId
    // ...

    throw new Exception($"Internal server error (ID: {internalErrorId})")
}

Also, you can provide generic implementation for non-technical errors in the same OnInvocationErrorAsync override method. In this method you need to provide some logic to detect and map non-technical errors to corresponding DTO type and return it as a result of the message. You can provide the serialization details by implementing them yourself or by implementing IXRoadFault interface for the result type.

class ClientError : IXRoadFault
{
    public string FaultCode { get; set; }
    public string FaultString { get; set; }
}

and custom behaviour (using the same example, but as another type of error result):

protected override Task OnInvocationErrorAsync(WebServiceContext context)
{
    // TODO : Use domain-specific exception types or some other filter to avoid exposing internal details in exceptions.

    if (context.Exception is TargetInvocationException ex && ex.InnerException != null)
        context.Result = new ClientError
        {
            FaultCode = "CODE-1234",
            FaultString = ex.InnerException.Message
        };

    return Task.CompletedTask;
}

@kadyrkulovr
Copy link
Author

@janno-p Hi,

I implemented CustomWebRequestHandler and now my input validations are working.
Thank you so much for your detailed explanation.

@janno-p janno-p closed this as completed Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants