IntentArchitect / Support

A repository dedicated to handling issues and support queries
3 stars 0 forks source link

Result Pattern #94

Open JonathanLydall opened 3 months ago

JonathanLydall commented 3 months ago

Ask a question

(Posted on behalf of another a user)

Good day,

I hope this email finds you well. I wanted to know if there's a way to implement Resultant Pattern. At the moment if the backend has an internal error, it returns a error 500. Is it possible to for it to return the below

public class ApiResult<T>
{
  // props that indicate status of request, like bool and T data type

  public static async new Task<ApiResult<T>> FromHttpResponseMessage(HttpResponseMessage response)
  {
    // Deserialise response, and map to type
  }
}
JonathanLydall commented 3 months ago

Our .NET Clean Architecture Application Template already supports essentially this pattern in a standardised way by returning any non-success result as a Problem Details which we have applied using ASP.NET Core's built in problem details infrastructure.

Here is an example showing the returned result for a 500 error:

{
  "type": "https://httpstatuses.io/500",
  "title": "Internal Server Error",
  "status": 500,
  "traceId": "00-206c68dab5e51be2304088695e107cea-8a6ed6e52825901e-00"
}

The above is when ASPNETCORE_ENVIRONMENT is set to Release, when set to Development it will show the following:

{
  "type": "https://httpstatuses.io/500",
  "title": "Internal Server Error",
  "status": 500,
  "detail": "System.NotImplementedException: Your implementation here...\r\n   at WrappedErrorResults.Application.SomeEndpoint.SomeEndpointCommandHandler.Handle(SomeEndpointCommand request, CancellationToken cancellationToken) in D:\\Dev\\Spikes\\Intent\\WrappedErrorResults\\WrappedErrorResults\\WrappedErrorResults.Application\\SomeEndpoint\\SomeEndpointCommandHandler.cs:line 24\r\n   at MediatR.Wrappers.RequestHandlerWrapperImpl`1.<>c__DisplayClass1_0.<<Handle>g__Handler|0>d.MoveNext()\r\n--- End of stack trace from previous location ---\r\n   at WrappedErrorResults.Application.Common.Behaviours.UnitOfWorkBehaviour`2.Handle(TRequest request, RequestHandlerDelegate`1 next, CancellationToken cancellationToken) in D:\\Dev\\Spikes\\Intent\\WrappedErrorResults\\WrappedErrorResults\\WrappedErrorResults.Application\\Common\\Behaviours\\UnitOfWorkBehaviour.cs:line 45\r\n   at WrappedErrorResults.Application.Common.Behaviours.ValidationBehaviour`2.Handle(TRequest request, RequestHandlerDelegate`1 next, CancellationToken cancellationToken) in D:\\Dev\\Spikes\\Intent\\WrappedErrorResults\\WrappedErrorResults\\WrappedErrorResults.Application\\Common\\Behaviours\\ValidationBehaviour.cs:line 36\r\n   at WrappedErrorResults.Application.Common.Behaviours.AuthorizationBehaviour`2.Handle(TRequest request, RequestHandlerDelegate`1 next, CancellationToken cancellationToken) in D:\\Dev\\Spikes\\Intent\\WrappedErrorResults\\WrappedErrorResults\\WrappedErrorResults.Application\\Common\\Behaviours\\AuthorizationBehaviour.cs:line 83\r\n   at WrappedErrorResults.Application.Common.Behaviours.PerformanceBehaviour`2.Handle(TRequest request, RequestHandlerDelegate`1 next, CancellationToken cancellationToken) in D:\\Dev\\Spikes\\Intent\\WrappedErrorResults\\WrappedErrorResults\\WrappedErrorResults.Application\\Common\\Behaviours\\PerformanceBehaviour.cs:line 35\r\n   at WrappedErrorResults.Application.Common.Behaviours.UnhandledExceptionBehaviour`2.Handle(TRequest request, RequestHandlerDelegate`1 next, CancellationToken cancellationToken) in D:\\Dev\\Spikes\\Intent\\WrappedErrorResults\\WrappedErrorResults\\WrappedErrorResults.Application\\Common\\Behaviours\\UnhandledExceptionBehaviour.cs:line 27\r\n   at WrappedErrorResults.Api.Controllers.DefaultController.SomeEndpoint(SomeEndpointCommand command, CancellationToken cancellationToken) in D:\\Dev\\Spikes\\Intent\\WrappedErrorResults\\WrappedErrorResults\\WrappedErrorResults.Api\\Controllers\\DefaultController.cs:line 41\r\n   at lambda_method6(Closure, Object)\r\n   at Microsoft.AspNetCore.Mvc.Infrastructure.ActionMethodExecutor.TaskOfActionResultExecutor.Execute(ActionContext actionContext, IActionResultTypeMapper mapper, ObjectMethodExecutor executor, Object controller, Object[] arguments)\r\n   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeActionMethodAsync>g__Awaited|12_0(ControllerActionInvoker invoker, ValueTask`1 actionResultValueTask)\r\n   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.<InvokeNextActionFilterAsync>g__Awaited|10_0(ControllerActionInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)\r\n   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Rethrow(ActionExecutedContextSealed context)\r\n   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)\r\n   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.InvokeInnerFilterAsync()\r\n--- End of stack trace from previous location ---\r\n   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeNextExceptionFilterAsync>g__Awaited|26_0(ResourceInvoker invoker, Task lastTask, State next, Scope scope, Object state, Boolean isCompleted)\r\n   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Rethrow(ExceptionContextSealed context)\r\n   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)\r\n   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.InvokeFilterPipelineAsync()\r\n--- End of stack trace from previous location ---\r\n   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Awaited|17_0(ResourceInvoker invoker, Task task, IDisposable scope)\r\n   at Microsoft.AspNetCore.Mvc.Infrastructure.ResourceInvoker.<InvokeAsync>g__Awaited|17_0(ResourceInvoker invoker, Task task, IDisposable scope)\r\n   at Microsoft.AspNetCore.Authorization.AuthorizationMiddleware.Invoke(HttpContext context)\r\n   at Microsoft.AspNetCore.Authentication.AuthenticationMiddleware.Invoke(HttpContext context)\r\n   at Microsoft.AspNetCore.Diagnostics.ExceptionHandlerMiddlewareImpl.<Invoke>g__Awaited|10_0(ExceptionHandlerMiddlewareImpl middleware, HttpContext context, Task task)",
  "traceId": "00-e433b4e926cd4951781eea1141384644-33d7ae9dbde9c667-00"
}

The standard problem details result type is also used for "expected" errors such as, validation errors (which include a structured list of validation errors):

{
  "type": "https://httpstatuses.io/400",
  "title": "One or more validation errors occurred.",
  "status": 400,
  "errors": {
    "Field": [
      "The Field field is required."
    ]
  },
  "traceId": "00-272b04d2e2d57f2e280f9de9a8c5c7aa-bcf608131f2642f7-00"
}

Or for something not being found (which can be triggered by throwing a NotFoundException generated by the Clean Architecture patterns):

{
  "type": "https://httpstatuses.io/404",
  "status": 404,
  "detail": "Could not record with id 1",
  "traceId": "00-b8f77e32d73480ae508e20b8f5bf8441-36b76a256108aecd-00"
}

The suggested pattern for consumers of APIs which can return standard problem details response is to check for a non-success response code and then try parse the result accordingly. Our HttpClient module does exactly this already and all details are available in the exception:

An example of a method using an HttpClient:

var relativeUri = $"api/some-endpoint";
var httpRequest = new HttpRequestMessage(HttpMethod.Post, relativeUri);
httpRequest.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json"));

var content = JsonSerializer.Serialize(command, _serializerOptions);
httpRequest.Content = new StringContent(content, Encoding.UTF8, "application/json");

using (var response = await _httpClient.SendAsync(httpRequest, HttpCompletionOption.ResponseHeadersRead, cancellationToken).ConfigureAwait(false))
{
    if (!response.IsSuccessStatusCode)
    {
        throw await HttpClientRequestException.Create(_httpClient.BaseAddress!, httpRequest, response, cancellationToken).ConfigureAwait(false);
    }
}
Details of the exception implementation:

```csharp public HttpClientRequestException(Uri requestUri, HttpStatusCode statusCode, IReadOnlyDictionary> responseHeaders, string? reasonPhrase, string responseContent) : base(GetMessage(requestUri, statusCode, reasonPhrase, responseContent)) { RequestUri = requestUri; StatusCode = statusCode; ResponseHeaders = responseHeaders; ReasonPhrase = reasonPhrase; ResponseContent = responseContent; if (!string.IsNullOrWhiteSpace(responseContent) && responseHeaders.TryGetValue("Content-Type", out var contentTypeValues) && contentTypeValues.FirstOrDefault() == "application/problem+json; charset=utf-8") { ProblemDetails = JsonSerializer.Deserialize(responseContent); } } public ProblemDetailsWithErrors? ProblemDetails { get; private set; } public Uri RequestUri { get; private set; } public HttpStatusCode StatusCode { get; private set; } public IReadOnlyDictionary> ResponseHeaders { get; private set; } public string? ReasonPhrase { get; private set; } public string ResponseContent { get; private set; } public static async Task Create( Uri baseAddress, HttpRequestMessage request, HttpResponseMessage response, CancellationToken cancellationToken) { var fullRequestUri = new Uri(baseAddress, request.RequestUri!); var content = await response.Content.ReadAsStringAsync(cancellationToken).ConfigureAwait(false); var headers = response.Headers.ToDictionary(k => k.Key, v => v.Value); var contentHeaders = response.Content.Headers.ToDictionary(k => k.Key, v => v.Value); var allHeaders = headers .Concat(contentHeaders) .GroupBy(kvp => kvp.Key) .ToDictionary(group => group.Key, group => group.Last().Value); return new HttpClientRequestException(fullRequestUri, response.StatusCode, allHeaders, response.ReasonPhrase, content); } private static string GetMessage( Uri requestUri, HttpStatusCode statusCode, string? reasonPhrase, string responseContent) { var message = $"Request to {requestUri} failed with status code {(int)statusCode} {reasonPhrase}."; if (!string.IsNullOrWhiteSpace(responseContent)) { message += " See content for more detail."; } return message; } ``` ```csharp public class ProblemDetailsWithErrors { public ProblemDetailsWithErrors() { Type = null!; Title = null!; TraceId = null!; Errors = null!; ExtensionData = null!; } [JsonPropertyName("type")] public string Type { get; set; } [JsonPropertyName("title")] public string Title { get; set; } [JsonPropertyName("status")] public int Status { get; set; } [JsonPropertyName("traceId")] public string TraceId { get; set; } [JsonPropertyName("errors")] public Dictionary Errors { get; set; } [JsonExtensionData] public Dictionary ExtensionData { get; set; } } ```

jasonpienaar1 commented 3 months ago

Having the problem details in the result works fine. Is it possible to also have a success result with potential data in it? Let's say we created data with a post, and we expect the saved object id back, can the backend implement the result pattern from the MediatR handlers, and return either Ok or Bad Request from the MediatR result and put that in the HttpResponse for us to deserialize and map to data in the UI? @JonathanLydall

JonathanLydall commented 3 months ago

Hi @jasonpienaar1,

I may need to better understand your exact situation to give you a better answer, but perhaps this will provide you the information you're looking for.

We don't have any existing modules at this time to wrap all results from the server regardless of success or failure, however, checking over HTTP whether a MediatR handler had an "Ok" or "Bad" result is implicitly possible by checking if the HTTP response code indicates success (i.e. is a 2xx response).

Depending on what technology your UI is built in this is generally easily available, for example using HttpClients in .NET this is available as a bool by checking HttpResponseMessage.IsSuccessStatusCode or in JavaScript you can check the Response: ok property.

If you want a wrapping object for all results on your client side which is more easily consumable by your UI, it should be a one liner for each service proxy method you have which is doing the HTTP call. You would pass the HTTP response to a function which would check the result code and return the wrapper object you described in your initial question with a field set to "good" or "bad" and another field for the result.

If you are only looking at the JSON deserialized result and not accessing the response directly (i.e. you can't see the headers easily), you can check the "status" field by doing something like (result.status < 200 || result.status > 299).

Would the above work well for you? Otherwise, let us know what UI technology you're using and more info about what seems to be getting in the way of easily consuming a "success" or "failure" result code from the server.

JohannesMogashoa commented 3 months ago

Hi @JonathanLydall

I have a similar situation with our project where if one of your services goes down we are unable to capture a meaningful message that we can use in our backend-for-frontend and it returns an internal server error.

In our client projects we are already using a similar result pattern as above to try and show a better message to the frontend when a service is down or when there is an issue but we would like it to persist throughout.

For example, for a resource not found, we would like to return a 204 with a message rather than 404 which also assists with Application Insights analysis.

We would like to follow a structure almost like this: https://github.com/ardalis/Result

JonathanLydall commented 3 months ago

Hi @JohannesMogashoa,

I understand what you mean about having more control about the errors to present on the front-end, controlling the errors for any scenario should be very easy to do already with the Clean Architecture by throwing exceptions (possibly even new ones of your own kind) and then transforming them in the Filters/ExceptionFilter.cs file under the Api project.

For example, for changing how a NotFoundException is handled, by default the file has the following:

public void OnException(ExceptionContext context)
{
    switch (context.Exception)
    {
        case ValidationException exception:
            foreach (var error in exception.Errors)
            {
                context.ModelState.AddModelError(error.PropertyName, error.ErrorMessage);
            }
            context.Result = new BadRequestObjectResult(new ValidationProblemDetails(context.ModelState))
            .AddContextInformation(context);
            context.ExceptionHandled = true;
            break;
        case ForbiddenAccessException:
            context.Result = new ForbidResult();
            context.ExceptionHandled = true;
            break;
        case UnauthorizedAccessException:
            context.Result = new UnauthorizedResult();
            context.ExceptionHandled = true;
            break;
        case NotFoundException exception:
            context.Result = new NotFoundObjectResult(new ProblemDetails
            {
                Detail = exception.Message
            })
            .AddContextInformation(context);
            context.ExceptionHandled = true;
            break;
    }
}

The NotFoundException case can be changed to instead return a 204 response.

You can also create your own exception type, either inside the Application or Domain project and then add a case in the above to transform it to any kind of response you want.

For example you could create a "ClientVisibleException" in the Common\Exceptions folder in the Application project, and then in the file above, add a case to the switch statement to transform it into the industry standard problem details response which you can easily read from your front-end in the same way it's handling validation errors already.

This also ties in nicely with the Clean Architecture principles where the domain and application layers should ideally be technology agnostic.

While creating a "result pattern" module is something we ultimately plan on doing, the pattern has drawbacks compared the current pattern employed. In particular, if a domain operation or event needs a particular error to be propagated back to a user interface, the Result return type would need to be permeated throughout the entire architecture to achieve this (it's essentially the same problem that happens when using async/await), on the other hand, with the existing pattern of handling exceptions, it works as is with only the most minimal of changes in one place.

Although not applicable to almost all .NET developers, it's worth acknowledging that a drawback of the current exception based pattern is that exceptions in .NET have a performance impact, but in practice this impact is in almost all cases negligible compared to other aspects in the system which may be affecting its performance. The performance impact of exceptions becomes an issue in cases like a monolith running on a single server where most or all other performance issues have already been addressed and more performance is still needed.

Please don't hesitate to respond if you have any further comments or questions.