Controlling HTTP status from failing form field guard

[ijackson]

Hi. I'm upgrading one of my applications from Rocket 0.4 to 0.5. I am having a little trouble converting my custom FromFormValue to FromFormField.

My old code is roughly like this:

impl<'r,> FromFormValue<'r> for MyType<'r> {
  type Error = OnlineErrorResponse;
  fn from_form_value(param: &'r RawStr) -> Result<Self,OnlineErrorResponse> {
...
enum OnlineErrorResponse {...}
impl Respoonder for OnlineErrorResponse { ... set status according to the error ... etc. }

With Rocket 0.5 I'm to impl FromFormField which means I have to return a rocket::form::Error. The rocket::form::Error does not seem to have a way to record the desired HTTP status response, and hence control the return value from form::Error::status.

(Please forgive me if I have missed a way to do this; I looked in the api docs for form::Error and FromFormField and also the relevant-seeming sections of the guide, searching both for "form" and "error".)

I want to control the http status because I am using a form guard in circumstances where I sometimes want to say 404 if the guard fails, and sometimes even 403.

I don't think this can be fixed easily without a breaking change to at least one of rocket::form::Error or ErrorKind so it would be good to fix this before Rocket 0.5.0.

I think the best fix is probably to add a new variant to ErrorKind, or to expand Custom, something like this:

+ Custom(Option<Status>, Box<dyn Error + Send + 'static>)
- Custom(Box<dyn Error + Send + 'static>)

or this:

+ CustomStatus(Status, Box<dyn Error + Send + 'static>)

I wonder if ErroKind ought to be marked #[non_exhaustive] for future-proofing.

[SergioBenitez]

I wonder if ErroKind ought to be marked #[non_exhaustive] for future-proofing.

This sounds like a good idea. I'm all for it.

(Please forgive me if I have missed a way to do this; I looked in the api docs for form::Error and FromFormField and also the relevant-seeming sections of the guide, searching both for "form" and "error".)

Thanks for looking at the docs. You're right that it's not possible to set an arbitrary status code today in a FromForm for custom errors. Built-in errors have a status code associated with them, however. You can also easily do this in a handler:

#[derive(FromForm)]
struct Foo<'r> {
    field: form::Result<'r, T>
}

#[post("/", data = "<form>")]
fn post(form: Form<Foo<'_>>) -> (Status, T) {
    if form.field.is_err() {
        return (Status::NotFound, T);
    }

    ...
}

If you have a responder than implements From<form::Errors<'r>>, you can, of course, do:

#[post("/", data = "<form>")]
fn post(form: Form<Foo<'_>>) -> Result<T, ErrorResponder> {
    let form = form.into_inner();
    let field = form.field?;
    ...
}

I think the best fix is probably to add a new variant to ErrorKind, or to expand Custom, something like this:

Let's say that your MyType fails and wants to set a status code of 404 (this is likely semantically wrong, but let's ignore that for now). Additionally, another field fails and wants to set a status code of 413. Which status code should be set in the response?

The current algorithm is to set the max(all_status_codes). This means that the 404 would be lost. If you want to guarantee that the handler returns a specific status code, I see no other solution than for the handler to do something like the two above solutions.

[ijackson]

I wonder if ErroKind ought to be marked #[non_exhaustive] for future-proofing.

This sounds like a good idea. I'm all for it.

Cool. I will send a merge request.

[ijackson]

#[derive(FromForm)]
struct Foo<'r> {
    field: form::Result<'r, T>

Thanks for this helpful suggestion. I hadn't thought of this approach, I think because I had forgotten about the impls on Result.

I still think it would be helpful to extend ErrorKind. In particular, the field: Result approach pushes more of the error handling logic into the handler, and binds it to the field rather than the type. That's tolerable but suboptimal, to my taste at least.

I think the best fix is probably to add a new variant to ErrorKind, or to expand Custom, something like this:

Let's say that your MyType fails and wants to set a status code of 404 (this is likely semantically wrong, but let's ignore that for now). Additionally, another field fails and wants to set a status code of 413. Which status code should be set in the response?

Good question.

In my application, for all of the handlers that would be affected, the answer is either (i) there is only one field or (ii) if there are several errors, we don't care which is returned.

The current algorithm is to set the max(all_status_codes). This means that the 404 would be lost. If you want to guarantee that the handler returns a specific status code, I see no other solution than for the handler to do something like the two above solutions.

Right.

I think this issue ought to be addressed in documentation, probably the documentation for the ErrorKinds that contain a Status.

Given what you say, ISTM that you probably don't want Custom to always carry a Status.

Having slept on it, my application would like to be able to save a box in this case. So I think I would now like to suggest:

 enum ErrorKind {
+    CustomStatus(Status, Option<Box<dyn Error + Send + 'static>>),

with appropriate docs including discussion about indeterminacy of the http status in the case of multiple field parsing errors. The docs could suggest the use of Result, too, if finer control is needed.

[raymclee]

I'm using FromForm for get request. and validating the range of the query params. when the validate function return error it just go other next route and return 404. The validate error message is logged to console. Is there a way to catch the error message to show to the user?

[SergioBenitez]

Is there a way to catch the error message to show to the user?

Yes. Use form::Result<Form<T>> instead of just Form<T>. Or use a Result for each field. Or Option if you don't care about what happened and just want to know something went wrong.