Implement both synchronous (1-arity) and async (3-arity) Ring handler fns #54

[enspritz]

No description provided.

[enspritz]

Something like this?

[weavejester]

Thanks for the PR, but it's a little trickier than just changing one function. As part of the authentication process, get-access-token sends a HTTP request to the OAuth server. In an asynchronous workflow, this request also needs to be asynchronous. This means that the get-access-token function needs an async version, and so too does its calling function, make-redirect-handler.

The custom error handlers should also use the async version when the middleware-wrapped handler is called with the 3-arity form.

Also, one trick you might not know about is that you don't need to put a function in a let-clause to self-reference it. You can give the function an interal name as part of the fn form:

(fn handler
  ([request] ...)
  ([request respond _raise]
    (respond (handler request))))

[enspritz]

Thanks for taking the time to look at this @weavejester

Async handler fn signatures enables use with the Aleph and Reitit setup we have. On this one point alone, it's a design win for me, and increases total addressable market for the lib. As for actual asynchronous behavior, it's not my immediate concern at this time -- may I suggest we leave that to someone more enlightened than me. At some point in the future a poor, afflicted soul might be motivated to submit a PR to remedy that.

Custom error handlers: check.

Internal name as part of fn form: Thanks for pointing that out. My intention was to improve legibility by delineating the Ring handler fn multi-arity definition from the essence of the function. Of course, in the end it's your call.

[weavejester]

As for actual asynchronous behavior, it's not my immediate concern at this time -- may I suggest we leave that to someone more enlightened than me. At some point in the future a poor, afflicted soul might be motivated to submit a PR to remedy that.

I'm afraid I can't merge this PR without it. This design blocks the current thread while waiting for the OAuth server, and its not uncommon for asynchronous servers to be configured with (or default to) a relatively low number of active threads. If we're going to release an async version, we need to do it correctly and use non-blocking I/O, otherwise we're just going to be causing headaches for downstream developers.

Internal name as part of fn form: Thanks for pointing that out. My intention was to improve legibility by delineating the Ring handler fn multi-arity definition from the essence of the function.

I mentioned it because I believe that using the fn name would be more legible than using a let binding.

[weavejester]

I've implemented a fully async version in commit f780551.