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

before_all handler isn't called for error routes #663

Open
syeopite opened this issue Sep 23, 2023 · 4 comments
Open

before_all handler isn't called for error routes #663

syeopite opened this issue Sep 23, 2023 · 4 comments
Assignees
Labels

Comments

@syeopite
Copy link

Description

It seems like the before_all handler isn't called for the error routes.

Steps to Reproduce

  1. Take a simple project with a before_all handler:

    require "kemal"
    
    before_all &.set "message", "hello"
    get "/", &.get("message")
    error 404, &.get("message")
    
    Kemal.run
  2. Query a route that doesn't exist such as 0.0.0.0:3000/route

  3. Notice that the before_all handler isn't called

Expected behavior:

curl "0.0.0.0:3000" # hello
curl "0.0.0.0:3000/path" # hello

Actual behavior: [What actually happens]

curl "0.0.0.0:3000" # hello
curl "0.0.0.0:3000/path" # => internal server error

Reproduces how often: [What percentage of the time does it reproduce?]

100% of the time

Versions

Crystal 1.9.2 (2023-08-24)

LLVM: 16.0.6
Default target: x86_64-pc-linux-gnu
@sdogruyol
Copy link
Member

Looks like a bug to. me, thanks a lot for reporting @syeopite I'll put this as priority in my pipeline 👍

@sdogruyol
Copy link
Member

My initial findings show that Kemal::Config::INSTANCE.handlers is not working as expected. Instead of respecting Kemal::Config::FILTER_HANDLERS it just errors out flat. Changing the order of the filters didn't work either. Need to investigate if it's a special behavior to 404

@grkek
Copy link
Contributor

grkek commented Sep 30, 2023

My initial findings show that Kemal::Config::INSTANCE.handlers is not working as expected. Instead of respecting Kemal::Config::FILTER_HANDLERS it just errors out flat. Changing the order of the filters didn't work either. Need to investigate if it's a special behavior to 404

Might be the exception handler short-circuiting the response and ignoring filter handlers

@jpmartinspt
Copy link

jpmartinspt commented Oct 18, 2023

It seems, like the filter handler does not get the blocks if a route is not found?

return call_next(context) unless context.route_found?

Commenting that line seems to make the original example work, but from a quick search I could not find the reasoning for it to be there in the first place. Happy to try and put in a PR to fix if you give some pointers on why it's there.

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

No branches or pull requests

5 participants