Skip to content

Distinguish between NotFound resource and NotFound route #341

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

Closed
NullVoxPopuli opened this issue Jul 13, 2018 · 10 comments
Closed

Distinguish between NotFound resource and NotFound route #341

NullVoxPopuli opened this issue Jul 13, 2018 · 10 comments

Comments

@NullVoxPopuli
Copy link
Contributor

does it make sense to return JSONAPI errors on uncaught server errors?

for example, I'm getting a 404, cause I'm bad at databasing, and the response body is empty.
imo, it'd be great if there was a response body with the error message text / and or exception class.

maybe this'd be a development mode only thing? idk.

@jaredcnance
Copy link
Contributor

jaredcnance commented Jul 13, 2018

All errors should be returned in a json:api error document. Otherwise how would a json:api client know how to handle them?

Can you provide more details around your specific situation? You’re getting a 404 because you’re returning null from your controller? Or an exception isn’t being handled properly?

Or are you referring to the stack trace? That can be disabled outside of the development environment.

@NullVoxPopuli
Copy link
Contributor Author

NullVoxPopuli commented Jul 13, 2018

I'm really just wanting anything returned. lol.

Here is what I have so far:

image

api_1      | [16:04:58 INF] Request starting HTTP/1.1 GET http://api:7081/api/users/current-user  
api_1      | [16:04:58 INF] Route matched with {action = "GetAsync", controller = "Users"}. Executing action Optimajet.DWKit.StarterApplication.Controllers.UsersController.GetAsync (Optimajet.DWKit.StarterApplication)
api_1      | [16:04:58 INF] Entity Framework Core 2.1.0-rtm-30799 initialized 'AppDbContext' using provider 'Npgsql.EntityFrameworkCore.PostgreSQL' with options: None
api_1      | [16:04:58 INF] Executing action method Optimajet.DWKit.StarterApplication.Controllers.UsersController.GetAsync (Optimajet.DWKit.StarterApplication) with arguments (["0"]) - Validation state: Invalid
api_1      | [16:04:58 INF] Executed DbCommand (1ms) [Parameters=[@__id_0='?' (DbType = Int32)], CommandType='Text', CommandTimeout='30']
api_1      | SELECT e."Id", e."Email", e."ExternalId", e."Name"
api_1      | FROM "User" AS e
api_1      | WHERE e."Id" = @__id_0
api_1      | LIMIT 2
api_1      | [16:04:58 INF] Executed action method Optimajet.DWKit.StarterApplication.Controllers.UsersController.GetAsync (Optimajet.DWKit.StarterApplication), returned result Microsoft.AspNetCore.Mvc.NotFoundResult in 12.681ms.
api_1      | [16:04:58 INF] Executing HttpStatusCodeResult, setting HTTP status code 404
api_1      | [16:04:58 INF] Executed action Optimajet.DWKit.StarterApplication.Controllers.UsersController.GetAsync (Optimajet.DWKit.StarterApplication) in 18.5098ms
api_1      | [16:04:58 INF] Request finished in 27.8235ms 404 

The whole startup.cs, if you're curious: https://github.com/sillsdev/appbuilder-portal/blob/master/source/OptimaJet.DWKit.StarterApplication/Startup.cs#L96

@jaredcnance
Copy link
Contributor

Does a user with the id “current-user” exist in the database? I don’t see a custom repository that would alter the default behavior. And by default it will just look for a user with the specified id in the database.

@NullVoxPopuli
Copy link
Contributor Author

there is no "current-user" user atm,
but, I'm more just wondering if there was any way to get more information in the response payload?

@jaredcnance
Copy link
Contributor

Per the specification:

A server MUST respond with 404 Not Found when processing a request to fetch a single resource that does not exist

However, you can always alter this behavior by overriding the controller method or implementing the necessary service or repository interfaces:

https://json-api-dotnet.github.io/#/custom-errors

https://json-api-dotnet.github.io/#/layer-overview

@NullVoxPopuli
Copy link
Contributor Author

so for clarification, a middleware to provide more information on all errors is out of scope for this project?

@jaredcnance
Copy link
Contributor

jaredcnance commented Jul 13, 2018

You can always write your own middleware. But, in general, the most detailed information about an error is available at the site where the error occurs. It would be difficult to provide useful information using generalized middleware. However, for this example, you could create a general controller that overrides the GetAsync methods and returns an error body instead of just a 404. For this specific behavior, I’d also be open to discussing an official implementation because it reduces ambiguity between a route not found and a resource not found. Something like:

{
  "errors": [
    {
      "title": "Resource 'users' with id 'current-user' was not found.",
      "status": "404"
    }
  ]
}

Is that what you're thinking?

To get this kind of behavior, you could override the controller like so:

namespace Optimajet.DWKit.StarterApplication.Controllers
{
    private readonly IResourceService<User> _userService;

    public class UsersController : JsonApiController<User>
    {
        public UsersController(
            IJsonApiContext jsonApiContext,
            IResourceService<User> userService,
            ILoggerFactory loggerFactory)
        : base(jsonApiContext, resourceService, loggerFactory)
        { 
            _userService = userService;
        }

        [HttpGet("{id}")]
        public override async Task<IActionResult> GetAsync(TId id)
        {
            var entity = await _userService.GetAsync(id);

            if (entity == null)
                return Error(new Error(404, $"Resource 'users' with id '{id}' was not found."));

            return Ok(entity);
        }
    }
}

To achieve this using a general middleware, you'd need to:

  • Verify the route matched a controller method and was handled by the controller
  • The controller returned 404 because the resource could not be found

You should be able to do this using an IActionFilter, but then you have to operate on the IActionResult and do something like:

if(ctx.Result is NotFoundResult) {
 // handle the 404 case
}

But because this is outside the context of the actual result evaluation, we're really inferring what 404 means and can't provide any information that is more useful than a 404 status code itself.

@jaredcnance
Copy link
Contributor

@NullVoxPopuli I'm closing this because I'm not sure I have any work to takeaway from this. Feel free to reopen if you're interested in continuing the discussion.

@jaredcnance jaredcnance changed the title Errors middleware? Distinguish between NotFound resource and NotFound route Jul 23, 2018
@jaredcnance
Copy link
Contributor

re-opening because I want to track a solution for distinguishing between 404 route not found and 404 resource (with id) not found.

@bart-degreed
Copy link
Contributor

Fixed by #714.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

3 participants