kemalcr / kemal

Fast, Effective, Simple Web Framework
https://kemalcr.com
MIT License
3.6k stars 187 forks source link

before_all handler isn't called for error routes #663

Open syeopite opened 11 months ago

syeopite commented 11 months ago

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 commented 11 months ago

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

sdogruyol commented 11 months ago

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 commented 11 months ago

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 commented 11 months ago

It seems, like the filter handler does not get the blocks if a route is not found? https://github.com/kemalcr/kemal/blob/cb9adcd188162f1e3c83ff98263d3587b9a2f9ab/src/kemal/filter_handler.cr#L16 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.