[Proposal] Some thoughts on the Lapis router

[karai17]

Accept strings as well as functions

Similar to how the return table of an action takes the require path of a view instead of the view itself, the router also taking the require path of an action instead of the action function itself feels like it would fit here.

Merge respond_to into match

Instead of needing to wrap actions with the helper function respond_to to extract the correct function for a request, it would be handy if this functionality was simply built in to match since it seems designed to be paired specifically with match anyway.

With both proposals in place, our routes could go from

local respond_to = require("lapis.application").respond_to
app:match("index", "/",      respond_to(require "actions.index"))
app:match("users", "/users", respond_to(require "actions.users"))
app:get(  "faq",   "/faq",   require "actions.faq")

to

app:match("index", "/",      "actions.index")
app:match("users", "/users", "actions.users")
app:get(  "faq",   "/faq",   "actions.faq")

Pseudocode

if type(handler) == "string" then
   handler = require(handler)
end

if type(handler) == "table" then
   handler = _respond_to(handler)
end

return handler and handler(req) or error()

[leafo]

I've been meaning to add a more formal way to reorganize the bodies of actions into separate modules, I pushed a patch that introduces a system that works just like views for actions:

• actions_prefix is new app level field that controls what module prefix to look for actions
• an action with the value true will use the route's name to require the action actions.#{route_name}
• a string value for the action will require the action module by that name, actions.#{value}

I decided to go with lazy loading, what the means is when the app boots not all the actions will resolve to their modules. The modules are loaded on request and cached via package.loaded. I figured this is the better default since this type of organization would probably be used as things get larger, and this would speed up booting/loading larger apps. Calling require directly will still work for up-front loading.

[alexandrim0]

@leafo how should I provide capture_errors or capture_errors_json handler if action have to return html or json depending on request header 'Accept'?
It is not clear from documentation. And it would not be good doing this in every action.
Thank you!

[alexandrim0]

Is it possible to use 'capture_errors' handler with lazy loading of actions with the value true?

[leafo]

@alexandrim0

Is it possible to use 'capture_errors' handler with lazy loading of actions with the value true?

No, you would instead have the capture_errors call directly in your action file (which returns a function).

how should I provide capture_errors or capture_errors_json handler if action have to return html or json depending on request header 'Accept'?

So capture_errors_json is very simple: https://github.com/leafo/lapis/blob/master/lapis/application.moon#L356

It runs capture_errors with very minimum configuration, so to handle both cases, I might do something like this:

capture_errors fn, =>
  if accept_json @
    return json: { errors: @errors }
  
  render: true

This combines the logic from both the capture_errors default handler and capture_errors_json

accept_json could be implemented like this:

accept_json = =>
  if accept = @req.headers.accept
    switch type accept
      when "string"
        return true if accept\lower!\match "application/json"
      when "table"
        for v in *accept
          return true if v\lower!\match "application/json"

  false

[alexandrim0]

@leafo thank you for your work and for answer!

Yes, I went this way and wrote my own capture error handler.
But there was a second reason. I set some headers (like Access-Control-Allow-Origin for my SPA) inside of before_filter handler, but capture_errors ignore them and returning of JSON also. So I had set them every time.
Also I had to redefine handle_error function to handle error 405 properly and to set default headers.

Would you make that common things as a part of Lapis?
It could be more convenient for building JSON API.

[leafo]

@alexandrim0

handle_error is designed for handling unexpected errors, and capture_errors is for handing expected errors. https://leafo.net/lapis/reference/exception_handling.html

So, generally speaking the handle_error callback is for when a 500 error happens: An unexpected error or something bad that you might want to log. All expected kinds of errors (bad method, user input validation, etc.) are better processed by capture_errors.

When handle_error is called it actually wipes out the old request object since it considers it invalid, and provides a new one to avoid any side effects messing up how the error page is written. That's why options you may have set in before filters or actions are no longer there (note it's possible that some things can not be undone, like if you set things with the ngx api directly). You can learn about the original_request object here: https://leafo.net/lapis/reference/actions.html#error-handler

[alexandrim0]

It is clear. Thank you!

One more thing. It would be grate if it is possible to choose default handle_error function while using actions as views (with arg true instead of function). It will makes code more clear and decrease boilerplate in action functions.

For now it looks like this:
`
local Routes = {
['health'] = 'get',
['token.refresh'] = 'post',
['token.introspect'] = 'match',
}

for name, action in pairs(Routes) do
App[action](App, name, '/'..name:gsub('%.', '/'), capture_errors_json(require('actions.'..name)))
end
`
What you think?

[leafo]

Closing this since the original request has been addressed for some time now.