before_all can not intercept http processing use env.redirect "new_path"

[zw963]

before_all can not intercept http processing use env.redirect "new_path"

[sdogruyol]

@zw963 I'll release 1.2.0 with Crystal 1.5.0 👍

[sdogruyol]

I think it's because of the next in the following, it breaks from if block and runs into the else condition

if current_user.nil?
    puts "1"*100
    env.redirect path.sign_in                            # <============== what we expected is redirect to "/auth/login-in" instead
	puts "2"*100
    next
  else

can you remove the nextand try again?

[zw963]

can you remove the nextand try again?

Sorry, remove next not work. the log is same when i visit "/"

[development] Kemal is ready to lead at http://0.0.0.0:3000
1111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111
2222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222222
3333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333333
Exception: Missing hash key: "current_user_open_id" (KeyError)

[zw963]

Hi, @sdogruyol , i quite sure, env.redirect not work in before_all do block.

anyway, i need one place to all request to ensure current_user is exists, if this feature
not work, it quite hard to write DRY code.

[zw963]

Hi, i found why this issue happen.

Following is my before_all code

before_all do |env|
  next if env.request.path == path.sign_in

  current_user = User.get_user_by_auth_token(env.session.string?("auth_token"))

  if current_user.nil?
      env.redirect path.sign_in
    next
  else
      env.set "current_user", current_user
  end
end

Following is my router block

    get path.license_index do |env|
      current_user = env.get("current_user").as(User)

      licenses = LicenseQuery.new
    
      render_with_current_user("license/index.ecr")
    end

When i request path.license_index, at first, will retrieve current_user in before_all block.

We assume current case, current_user is nil, so, code env.redirect path.sign_in will run.

The really wired things is, before env.redirect really happen, the request handle process still wil go into
router block, env.get("current_user").as(User) will raise Missing hash key: "current_user" (KeyError) in
this case, because i never set it in before_all.

so, why this issue is happen is discovered, how to fix that? maybe i do things wrong way?

Anyway, i don't think we should do env.get?("current_user") check in my request block is useful.
because if i need check it, i really don't need before_all!

What i expected just is, redirect early in before_all call.

But, for now, before_all just is a place do some common things, but still continue.

I guess it caused by lazy? as describe by following

The Filter middleware is lazily added as soon as a call to after_X or before_X is made. 
It will not even be instantiated unless a call to after_X or before_X is mad

[cyangle]

@zw963 I think you would want to close the response immediately after redirection.

if current_user.nil?
    puts "1"*100
    env.redirect path.sign_in                            # <============== what we expected is redirect to "/auth/login-in" instead
    env.response.close
    puts "2"*100
    next
  else
    env.session.string("current_user_open_id", current_user.open_id)
  end

@sdogruyol I think it makes sense to close the response inside of the HTTP::Server::Context#redirect method, better to make it an optional parameter:

    def redirect(url : String, status_code : Int32 = 302, *, body : String? = nil, close : Bool = true)
      @response.headers.add "Location", url
      @response.status_code = status_code
      @response.print(body) if body
      @response.close if close
    end

Even with this, it would still go through all before filters, it only stops at the beginning of Kemal::RouteHandler#process_request because it short circuits if the response is already closed.

Maybe we can add something similar to HTTP::StaticFileHandler's fallthrough option to filters.

[zw963]

@zw963 I think you would want to close the response immediately after redirection.

Yes, you are right.

[zw963]

Cool, my issue is solved! quite perfect.

before this, there is no easy way to use setted env in request handler.

e.g.

get path.home_index do |env|
  current_user = env.get("current_user").as(User)
  # ...
end

When i try to visit path.home_index directly without login in, will raise exception
because no "current_user" key get set before.

even, after i change to current_user = env.get?("current_user").as(User), it still not work.
because raise exception: " Exception: cast from Nil to User failed".

I really don't want add a if/else again for make current_user always work, even, i have done it in before_all.

Now, it works like a charm!!

Thanks for @sdogruyol , @cyangle , and all.

[zw963]

Do we consider release 1.1.3?

[zw963]

I'll release 1.2.0 with Crystal 1.5.0

Okay, that soulds reasonable, anyway, some default behavior in before_filter was changed.

[zw963]

@zw963 I'll release 1.2.0 with Crystal 1.5.0 +1

Forget to release 1.2.0 ? 😄

[sdogruyol]

Haha no @zw963 thanks for the ping 👍 I'll cut a release soon 🎉

[sdogruyol]

@zw963 there you go https://github.com/kemalcr/kemal/releases/tag/v1.2.0