Updated error responses of the ResourceModificationController to be RFC 9457 compliant - #1444
Conversation
…urceModificationController and added localization
|
|
| if (resourceModel is null) | ||
| return NotFound(new MoryxExceptionResponse { Title = string.Format(Strings.ResourceNotFoundException_ById_Message, id) }); | ||
|
|
||
| return NotFound(new ProblemDetails |
There was a problem hiding this comment.
We should be able to shorten this in an elegenat fashion. Something like
return NotFound().With(Title, Details, context)Maybe @seveneleven has an opinion on this, designing fluent APIs is something of a habit of his 😁
If we go with the example above, you'd need to create extensions methods on, in this case, the NotFoundResult type, which returns a NotFoundObjectResult with the problemdetails object inside that you also used here. The result would be an easier to read return statement with less duplicated property assignements.
And then you would repeat that for BadRequest and any other standard error response code we are currenlty using 😊
Edit: For the resource not found response I'm also very sure, that you can further decrease code duplication if you define an extension method within this project which already has the other properties set as well.
Edit 2: I just saw there is a Problem(...) virtual method on the controller base class which already gets the context set. So my new suggestion for the fluent API is
// Placed in Moryx.AspNetCore project
return Problem().NotFound(Title, Details)And within this project
return Problem().NotResourceFound(id)| Type = "https://www.rfc-editor.org/rfc/rfc9110.html#name-404-not-found", | ||
| Title = Strings.ResourceModificationController_ResourceNotFound_Title, | ||
| Status = StatusCodes.Status404NotFound, | ||
| Detail = string.Format(CultureInfo.CurrentCulture, Strings.ResourceModificationController_ResourceNotFoundException_ById_Message, id), |
There was a problem hiding this comment.
| Detail = string.Format(CultureInfo.CurrentCulture, Strings.ResourceModificationController_ResourceNotFoundException_ById_Message, id), | |
| Detail = string.Format(CultureInfo.CurrentCulture, Strings.ResourceModificationController_ResourceNotFound_ById_Message, id), |
| { | ||
| public string Title { get; set; } | ||
|
|
||
| public string Exception { get; set; } |
There was a problem hiding this comment.
As the RFC specifically states Problem details are not a debugging tool for the underlying implementation we should remove this type completely and should not return exception messages or stack traces from any endpoint.
Instead, we should log the exceptions in the endpoints together with a trace Id, which can be added to the problem details in the extensions. I think this is the place were we should start with providing an extension method to be used in the Program.cs as a central solution. Following is a minimal AI snippet for that. However, it does not include logging the exceptions in the controller or appending trace IDs only where there is actually a problem details response. So it only swerves as a starting point.
using System.Diagnostics;
builder.Services.AddProblemDetails(options =>
{
options.CustomizeProblemDetails = context =>
{
context.ProblemDetails.Extensions["traceId"] =
Activity.Current?.TraceId.ToString()
?? context.HttpContext.TraceIdentifier;
};
});
Changes
Standardized Error Responses
All error responses returned by
ResourceModificationControllernow use the RFC 9457 format with the following fields:typetitlestatusdetailinstanceField descriptions:
typereferences the HTTP status code definition in RFC 9110.titleprovides a localized summary of the error.statuscontains the HTTP status code returned by the API.detailprovides a localized and more detailed description of the error.instancecontains the path of the request that triggered the error (HttpContext.Request.Path).Controller Error Handling
Replaced
MoryxExceptionResponsewithProblemDetails.The controller only returns expected and explicity handled errors. For these scenarios, the standard
ProblemDetailsclass is fully sufficient because no additional exception information is required. Therefore,MoryxExceptionResponsedid not provide any additional value in this context.MoryxExceptionResponseis still used by the globalMoryxExceptionFilter, which handles unexpected exceptions and includes the exception field in the response body to support debugging.Refactoring of MoryxExceptionResponse
MoryxExceptionResponsenow inherits fromProblemDetails.As a result:
ProblemDetailsalready provides the standardizedtitlefield.Localization
Added localization for the
titleanddetailfields in English, German, Italian, Polish and Chinese.