Skip to content
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

Response::with_* functions should either panic or state the error #22

Open
CuriouslyCurious opened this issue Sep 4, 2023 · 0 comments

Comments

@CuriouslyCurious
Copy link
Contributor

CuriouslyCurious commented Sep 4, 2023

Currently the with_*() functions for Response all have different error-behavior:

  • with_json() panics on serialization failure.
  • with_asset() will instead not panic, continue to execute and return a 404 upon attempting to retrieve the asset. This is the same behavior as if the route isn't even defined. Very confusing.
  • with_file() is the same as with_asset() except that if there is an error opening the file a 500 code with the actual error embedded is returned instead of a 404.

I think it should either be all panic, all returning a 500 with the included error text, a result be propagated somehow or the error message be printed in the "log". 404s are very confusing when the file you are trying to include doesn't exist as it looks like the route doesn't exist instead of the file not existing.

Minimum confusing example:

use vial::{routes, run};

routes! {
    GET "/file" => |_| Response::from_file("I DO NOT EXIST");
}

fn main() {
    _ = run!();
}

Output after two curls:

~ vial running at http://0.0.0.0:7667
GET 404 /file
GET 404 /route_not_existo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant