Unable to do early returns from actions

[jwoertink]

Say you have some complex action that checks a bunch of variables, then returns your response then finally has a fallback. Currently you have to handle every else case because you can't just do return json(...)

get "/something" do
  if current_user
    if other_thing
       if last_check
         return json(...)
       end
     end
  end

  json(....)
end

I think it's because the macro expands in this if/else section here

https://github.com/luckyframework/lucky/blob/b639085f11a775358737515aa0cbbcc38cfcc864/src/lucky/routable.cr#L99

which would mean that none of the after_pipe code would run since it would proxy your return statement up to the call method and early return there.

My first thought was maybe you can just throw that body in to a proc, but that could mess with macro scoping like calling params or something :thinking:

[jwoertink]

Just another thought on this, that {{ body }} line could just be moved to a different method like call_body or something. That actually might be the better solution.

private def call_body : Lucky::Response
  {{ body }}
end

def call
#...
%response = if %pipe_result.is_a?(Lucky::Response)
        %pipe_result
      else
        call_body
      end
#...
end

[jwoertink]

After a bunch of poking around... I can't get this to happen again. We tried using this in our app, and kept running in to issues. We ended up having to account for every single condition branch. I just tried to recreate this in Lucky, and I'm unable to.

I did check out my previous idea though, and I think this change may still be worth it...

Moving the {{ body }} to a separate method means that Crystal will catch your missing response with this:

This is in comparison to what Lucky does now which is:

Granted, the second one looks a lot nicer, and gives you some direction as to how to fix the error, but it doesn't tell you where the error is. For a new person, this error really isn't helpful because you may have just added 20 actions without checking and then ran this and have no clue where to fix the error.

For fun, I added #{@type} to my error to see if a simple fix to the pretty error could be done...

Notice it says Multiple::Index is the action where it failed.... but the action I actually messed up was Tests::MultiConditionWithEarlyReturn. I don't know why it thinks it's an action in a spec that I'm not even working near :man_shrugging:

So I guess what I'm saying is, making this change is probably better anyway.

[jwoertink]

Alright, there you go

I feel like that's a bit of the best of both worlds scenario