Not all result statuses are covered in ToActionResult

[Happi-cat]

Currently we're checking for NotFound/Invalid explicitly in ToActionResult. And finally we're returning Ok w/o checking if its really ok. So any other status will be returned as 200 (and thats not correct I think)

Just see link and code below

            if (result.Status == ResultStatus.NotFound) return controller.NotFound();
            if (result.Status == ResultStatus.Invalid)
            {
                foreach (var error in result.ValidationErrors)
                {
                    // TODO: Fix after updating to 3.0.0
                    controller.ModelState.AddModelError(error.Identifier, error.ErrorMessage);
                }
                return controller.BadRequest(controller.ModelState);
            }

            return controller.Ok(result.Value);

PS: status has Error/Forbidden which aren't covered by if/then above. PPS: So Forbidden will be treated as 200.

---

Assume should be like:

// first point (ok)
if (result.Status == ResultStatus.Ok) return controller.Ok(result.Value);
// custom responses
if (result.Status == ResultStatus.NotFound) return controller.NotFound();
if (result.Status == ResultStatus.Invalid) 
{
     /*  some codee here */ 
    return controller.BadRequest(controller.ModelState);
}

if (result.Status == ResultStatus.Forbidden) { /* some code here*/ }
if (result.Status == ResultStatus.Error } { /* some code here */ }

// default for not ok responses: can be NotSupported or smth
throw new  NotSupportedException();

[Happi-cat]

@ardalis btw, what do you think about ResultStatus.Error ? what should we return, status code 500 or 400 (bad request)? from one side 400 can be used just because thats a kind of non-fatal error (i.e. handled gracefully) while 500 can be used when Exception was thrown (critical issue)

from another side even that Error should have status code 500..

[aligoren]

@ardalis btw, what do you think about ResultStatus.Error ? what should we return, status code 500 or 400 (bad request)? from one side 400 can be used just because thats a kind of non-fatal error (i.e. handled gracefully) while 500 can be used when Exception was thrown (critical issue)

from another side even that Error should have status code 500..

There are differences between them. Let me explain to you.

When you get a 400 error, there is an error because of user interaction. So, the error is caused by a user.

If you get a 500 error, it means that the user did everything right, but the server did not work as expected.

If you want to send an error to explain a client-based error, you need to use 4xx. If you want to send a server-based error, you need to use 5xx.

You can check RFC-7231.

Actually, you need to decide which error code you should use. 400 or 422. Because if you want to use 400, your result will say 'there are malformed contents in the request'. If you want to use 422, your result will say that 'everything is well-formed but there is something not semantically corrected'.

These are my opinions.

[Happi-cat]

66

[Happi-cat]

@ardalis assume can be closed as #66 was merged