Skip to content
This repository has been archived by the owner on Dec 22, 2019. It is now read-only.

Make Middleware Async by Default #17

Open
leostera opened this issue Feb 9, 2019 · 7 comments
Open

Make Middleware Async by Default #17

leostera opened this issue Feb 9, 2019 · 7 comments

Comments

@leostera
Copy link
Owner

leostera commented Feb 9, 2019

After talking with @ulrikstrid about having an async router, it makes a lot of sense that the default middleware stack works asynchronously.

The first step is fairly straightforward, we just need to parametrize the Middleware stack with an io('a) type. Then the runner of the stack will get passed a sequencing function (>>=). Voila, all middleware is now async.

The interesting bit to me is how to make it flexible enough to allow for middleware that will define how the rest of the stack unfolds.

For example:

server
|> use(Middleware.log)
|> use(Router.with_handler(handler))
|> Lwt_main.run;

This would log before routing, and then return a response. But what would need to happen to allow for log to actually log the response status and response time?

The above right now behaves like:

  1. Log stuff
  2. Route and reply

Whereas I'd like the async behavior to allow for:

  1. Begin Measuring time
  2. Route and reply
  3. Log time between 1 and now

Without changing the way the middleware stack was built. In other words, the definition of the Middleware.log middleware would have to look a little more like:

let log = (ctx, next) => {
  let time_1 = Time.now();
  next() >>= (result => {
   let time_2 = Time.now();
   Log.debug( m => m("%f.3ms", time_2 -. time_1);
   result
  });
};
@emmenko
Copy link
Contributor

emmenko commented Feb 9, 2019

Thanks for looking into that!

I agree that having a flexible middleware stack would open up supporting more use cases. I'm trying to think of examples other than loggers that would need to hook into the response cycle. 🤔


I guess a bit unrelated to the async stuff, but I was wondering if a middleware can end the response before going through the router.
For example, let's say we have an auth middleware that checks for a valid auth token and rejects the request in case it's invalid. Is that possible with the current implementation?

server
|> use(Common.log)
|> use(App.auth) /* <-- this could end the response, without going through the router */
|> reply(Common.router(App.route_handler))
|> Lwt_main.run;

@ulrikstrid
Copy link
Contributor

On my phone but will do my best to write down my random thoughts.

The auth middleware would also be able to "takeover" routing. That means that any middleware in the stack would have to be able to send a reply.

If we have a option(next) that we pass through the middleware stack we could probably do both. Normal case is that you just pass it along, if you need to know about the other middlewares being done you pass a new callback and when you want to return you pass None to the next function to signify that no work should be done and I guess call next. Or maybe it should be named return it something.

@leostera
Copy link
Owner Author

leostera commented Feb 10, 2019

@emmenko: I guess a bit unrelated to the async stuff, but I was wondering if a middleware can end the response before going through the router.

@ulrikstrid: That means that any middleware in the stack would have to be able to send a reply.

Rather than returning an option(next) I'd like a proof that either next or reply was called. So essentially, the return type of a middleware function would have to be one that can only be constructed with by calling next() or reply(...).

This would ensure that we either are calling the next middleware, or we are short-circuiting with a reply. As long as reply() doesn't actually perform the reply, but just describes it, it should be okay: that is, there would still be a single reply value being returned from the middleware.

For next() the returned value would have to also be a description of what should happen after the middleware chain has completed, so that calling it multiple times doesn't actually affect the semantics of the program.

That is, this should be safe to do:

let log = ctx => {
  let a = ctx.reply(200, "what");
  let b = ctx.reply(200, "what");
  let c = ctx.reply(200, "what");
  let d = ctx.next() >>= (res => ctx.reply(200, res));
  let e = ctx.next();
  a;
};

Which of course is silly, but it means we can guarantee all middleware either continues the chain, or terminates the chain with a reply. That is, it should be very very hard to build a middleware stack that doesn't end up in a reply. (You can always have a let rec log that just infinitely recurses ¯_(ツ)_/¯ )

Worth noting this method would make it possible to write all of these below:

/* Do something and continue the middleware stack */
let log_only_at_the_beginning = ctx => {
  Log.app(m => m("Logging stuff"));
  ctx.next();
};

/* Change stack unfolding order by running something _after_ it's done */
let measure_latency = ctx => {
  let time_1 = Time.now();
  ctx.next() >>= ((status, res) => {
    let time_2 = Time.now();
    Log.debug(m => m("Time: %.2f ms", time_2 -. time_1));
    ctx.reply(status, res);
  });
};
 
/* Short-circuit stack unfolding */
let auth = ctx => {
  switch(ctx.state.user) {
  | Some(user) => ctx.next();
  | None => ctx.reply(401, "No user found");
  };
};

What do you think?

@emmenko
Copy link
Contributor

emmenko commented Feb 11, 2019

Yeah, this looks like a nice API 👌

@arecvlohe
Copy link

Not sure if this was your intention but some of these ideas remind me of hyper from PureScript. Thought I would share since I think it has some nice ideas: https://hyper.wickstrom.tech/docs/0.9.0/index.html

@tlvenn
Copy link

tlvenn commented Dec 7, 2019

It might be worth looking at the Plug spec from Elixir world which I find pretty well designed and flexible. The way it composes pipelines which transforms a connection record is pretty neat.

Worth noting maybe in contrast to what you have in mind, sending a reply does not halt the execution of the pipeline, you have to explicitly call an halt function to short circuit it.

@ulrikstrid
Copy link
Contributor

@tlvenn of you like the plug pattern you might like http://GitHub.com/reason-native-web/morph

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants