feat: add 404.sql handler

[wucke13]

Fixes #529 .

@lovasoa Is this an acceptable business logic to you? I'm sure the code can be cleaned up, and there might be an argument for using PathBuf directly instead of doing String manipulation, but besides that?

[lovasoa]

Hi @wucke13 ! The logic seems to match exactly what we discussed; everything good for me!

If you can clean up the code, and maybe put it in a separate function, I'll merge and this will be in the next version.

Thank you for your contribution!

[lovasoa]

I'm not sure what the best way to teach this new feature to users is. I think we should have 404.sql files in at least some of the examples on this repository.

We could also mention its existence in the documentation of the json component and the path function. We could also mention it in configuration.md.

[wucke13]

I definitely plan on adding an example. I'm quite indifferent on one thing: Should we do the path traversal stuff using a String or a PathBuf. What I currently like about using a String, is that we can discern a/b/c/ and /a/b/c. In the former case we want to check /a/b/c/404.sql, but in the latter case we don't. I think this nuance gets lost when converting the String to a PathBuf. @lovasoa Would you agree on using a String?

[lovasoa]

Yes, we should use a string and not a PathBuf. If there are other places where I used a PathBuf to represent a URL path, it is a mistake. PathBufs represent a platform-specific local file path; we want URL path handling to work exactly the same on all platforms.

[wucke13]

Ok, I now have a cleaner solution. Please let me know if you are happy with it as well. If yes, I'll also push an example, squash the first two commits and hopefully then we are ready for merge 😄 .

Edit: nevermind, there is still a bug in it.
Edit 2: now it is ready!

[lovasoa]

Thank you, the overall logic looks good to me !

But we should be careful to invoke the 404 handler only for actual 404s, not for other errors. Maybe we can add tests for that ?

[wucke13]

@lovasoa I adjusted again. Most notably, now the serve_fallback call is only made when the error is 404:

// On 404/NOT_FOUND error, fall back to `404.sql` handler if it exists
let response = match maybe_response {
    // File could not be served due to a 404 error. Try to find a user provide 404 handler in
    // the form of a `404.sql` in the current directory. If there is none, look in the parent
    // directeory, and its parent directory, ...
    Err(e) if e.as_response_error().status_code() == StatusCode::NOT_FOUND => {
        serve_fallback(&mut service_request, e).await?
    }

    // Either a valid response, or an unrelated error that shall be bubbled up.
    e => e?,
};

I'm not sure how to test this elegantly, though at least at a superficial glance this seems to be a pretty robust implementation of "only ever try to do something when there is an 404 Not Found error".

I also added an example, that both illustrates the use and explains the fallback mechanism in prose. While I did mention custom error handling and implementing REST APIs as potential use-cases, I intentionally did not pitch the use of this feature as gateway to dynamic routing in general, I feel that goes against the overall spirit of SQLpage. Creative users anyway will do what they want 😄 , but we don't have to encourage it in the examples.

[lovasoa]

Great! Thank you @wucke13 !

You can add a test in tests/index.rs that makes a request to tests/xyz.sql that doesn't exist, and add a tests/404.sql.

Maybe we can still mention using it for APIs in the documentation of the json component.

[wucke13]

@lovasoa Done. Also squashed a little for a clean history.

[lovasoa]

We are almost there!

[wucke13]

I'd be glad if we can get this done rather sooner or later, my motivation is somewhat depleted.

[lovasoa]

Thank you @wucke13 ! This PR is going to be really useful! I'll try to reference the new feature more in the docs to make it more discoverable