feat(http): middleware

[iuioiua]

createHandler() creates a handler from a given middleware chain. This implementation aims to be plain, simple, unopinionated and align with the Deno runtime and Web APIs as much as possible.

Hopefully, this can be used as a fundamental building block for web frameworks and spur the creation of middleware with a similar design philosophy.

Closes #1295

[iuioiua]

Does this error make sense?

[lino-levan]

I'm not sure if a RangeError is the right Error class for this. Should this be a custom error?

[iuioiua]

I'm not certain either. I'd be keen to hear other's thoughts.

[marvinhagemeister]

The way the createHandler function is designed here assumes that the last middleware never calls next(). So in that sense it's fine, although it prevents composing createHandler itself. Not sure if that is a goal here though.

const a = createHandler([m1, m2, m3]);
const b = createHandler([m1, m2, m3]);

// Not possible
const c = createHandler([a, b]);

[iuioiua]

Yeah, it wasn't one of my goals to do that... I don't know the advantage of doing it and whether there'd be sufficient use cases for that.

[lino-levan]

I'm slightly -1 on this from a philosophical standpoint, but the actual code looks fine to me.

[kt3k]

Maybe let's explore how this MiddlewareFunction can be used in fresh and oak?

Also this middleware doesn't seem having ability to pass information to the later middleware such as auth info, etc (or is that possible?). I wonder if this 'MiddlewareFunction' interface is general enough to be 'standard' middleware

[iuioiua]

@kitsonk and @marvinhagemeister, WDYT? Can you see this being useful for your respective frameworks? 🙂

[timreichen]

I would suggest in general to rename the module:

• middleware -> handlers to be more inclusive for future features.

And also

• MiddlewareFunction -> something like MiddlewareHandler or RequestHandler to be more specific
• createHandler -> composeHandlers or createMiddlewareHandler for more clarity

Also this middleware doesn't seem having ability to pass information to the later middleware such as auth info, etc (or is that possible?). I wonder if this 'MiddlewareFunction' interface is general enough to be 'standard' middleware

I think this feature would be a must for useful middleware handlers in std.

Passing data to the next handler could be achieved by extending Deno.ServeHandlerInfo and optionally passing data when calling next(). Maybe something like this:

export type MiddlewareHandler<I> = (
  request: Request,
  info: I,
  next: (info?: I) => Response | Promise<Response>,
) => Response | Promise<Response>;

export function composeHandlers<I extends Deno.ServeHandlerInfo>(
  middlewares: MiddlewareHandler<I>[],
): (request: Request, info: I) => Promise<Response> | Response {
  return (request, info) => {
    function compose(index: number, info: I): Response | Promise<Response> {
      const handler = middlewares[index];
      if (!handler) throw new RangeError("Middleware chain exhausted");
      return handler(
        request,
        info,
        (newInfo = info) => compose(index + 1, newInfo),
      );
    }
    return compose(0, info);
  };
}

So it can be used like this:

import { composeHandlers } from "https://deno.land/std/http/handlers.ts";

interface StateHandlerInfo extends Deno.ServeHandlerInfo {
  state: Record<PropertyKey, unknown>;
}

const composedHandler = composeHandlers<StateHandlerInfo>([
  (_request, info, next) => next({ ...info, state: { foo: "bar" } }),
  (_request, info) => Response.json(info.state),
]);

Deno.serve((request, info) => composedHandler(request, { ...info, state: {} }));

[iuioiua]

I would suggest in general to rename the module:

• middleware -> handlers to be more inclusive for future features.

This file is deliberately only for middleware-related APIs.

And also

• MiddlewareFunction -> something like MiddlewareHandler or RequestHandler to be more specific

I agree. Done.

• createHandler -> composeHandlers or createMiddlewareHandler for more clarity

I don't want to go down a bikeshedding rabbit hole, but createHandler() that accepts multiple middlewares is clear to me. Adding the word "compose" might raise the question: "What exactly does it mean to compose?"

I think this feature would be a must for useful middleware handlers in std.

Passing data to the next handler could be achieved by extending Deno.ServeHandlerInfo and optionally passing data when calling next().

I think the important question to answer is: "Is a context object a requirement here or can context be managed without adding a parameter to MiddlewareHandler?" Likely. But I'm not sure. Either way, I'm on the fence about extending Deno.ServeHandlerInfo. Another idea is:

interface MiddlewareHandlerOptions<T> {
  info?: Deno.ServeHandlerInfo;
  context?: T;
}

export type MiddlewareHandler<T> = (
  request: Request,
  next: () => Response | Promise<Response>,
  options?: MiddlewareHandlerOptions<T>,
) => Response | Promise<Response>;

[kt3k]

"Is a context object a requirement here or can context be managed without adding a parameter to MiddlewareHandler?"

I think the context info can be stored somewhere else by using WeakMap keyed by request object.

const contextMap = new WeakMap();

export function getContext(request: Request) {
  return contextMap.get(request);
}

const middleware1 = async (req, info, next) => {
  const ctx = getContext(req);
  ...
};

[iuioiua]

Oh, I like that approach.

[marvinhagemeister]

I like the idea of exposing a composable middleware primitive in the std. One thing I'm wondering if the current signature proposed in this PR is useful enough. It allows you to call a bunch of functions in sequence, but the way it's designed here it's not possible to share state among them.

[marvinhagemeister]

The way the createHandler function is designed here assumes that the last middleware never calls next(). So in that sense it's fine, although it prevents composing createHandler itself. Not sure if that is a goal here though.

const a = createHandler([m1, m2, m3]);
const b = createHandler([m1, m2, m3]);

// Not possible
const c = createHandler([a, b]);

[iuioiua]

I like the idea of exposing a composable middleware primitive in the std. One thing I'm wondering if the current signature proposed in this PR is useful enough. It allows you to call a bunch of functions in sequence, but the way it's designed here it's not possible to share state among them.

If it had a state parameter, could you imagine it being something Fresh would use?

[iuioiua]

I've added a state parameter. PTAL, @marvinhagemeister.

I am still not 100% sure about having this in the Standard Library. This PR aims to explore the idea either way. The implementation does aim to be as plain and unopinionated as possible.

@kitsonk, WDYT?

[iuioiua]

PTAL @keithamus and @LionC

[keithamus]

Once nice thing in https://github.com/denoland/deno_std/pull/1555/files#diff-d7730497c05f4870ef5336fe16c6eddb662c84d364b8ec55a1810a36a4901591R51 is that the Middleware was typed to allow for dependencies in the state which allowed for a very nice typing of middleware.

[kitsonk]

My view (#1295 (comment)) has not changed, and considering other things that are being deemed too frameworky around HTTP servers are being removed it seems that is the direction of travel...

[syhol]

I know this is from the Deno.serve API, but might it be cleaner to move it into either the options object or the options state object? Similar to what you suggest here #3785 (comment).

1. It follows the deno_std guidelines "max 2 args, put the rest into an options object"
2. Deno.ServeHandlerInfo isn't a very interesting object, no offence 😅, but it adds noise to the API when I can imagine most people will never use it.

[iuioiua]

Yeah, I'm thinking that too.

[Leokuma]

Is it mandatory that every middleware in a chain has the same State type? Doesn't it make it difficult to reuse middlewares accross different chains/endpoints?

[LionC]

Is it mandatory that every middleware in a chain has the same State type? Doesn't it make it difficult to reuse middlewares accross different chains/endpoints?

In my draft (linked in the original post), they do not have the same type - they state what they require to already be in the context to work (so basically context inputs) and what they add to the context (context outputs) and the chaining function then composes them, combining the types correctly and in order. This makes them completely composable, as a combined chain is just a new middleware with those inputs and outputs again, and a listener is just a middleware with an empty context input.

I think if we go Typescript first, we should honor order to catch errors, as it is just a function chain in the end.

[Leokuma]

Thank you, Lion.

Another question: the way this PR implements it, isn't there a chance of "state collision" between middlewares? Like, a middleware declares a state.cryptoKey and another middleware also uses the same prop, but it's just a naming coincidence as they serve different purposes. When you chain them together, you have to clean up state to avoid data from the previous middleware leaking into the next?

[LionC]

Thank you, Lion.

Another question: the way this PR implements it, isn't there a chance of "state collision" between middlewares? Like, a middleware declares a state.cryptoKey and another middleware also uses the same prop, but it's just a naming coincidence as they serve different purposes. When you chain them together, you have to clean up state to avoid data from the previous middleware leaking into the next?

Sorry, I replied in regards to how my original proposal and PoC PR implemented it (linked in the first post here as well), not related to this MR.

[iuioiua]

Is it mandatory that every middleware in a chain has the same State type? Doesn't it make it difficult to reuse middlewares accross different chains/endpoints?

In my draft (linked in the original post), they do not have the same type - they state what they require to already be in the context to work (so basically context inputs) and what they add to the context (context outputs) and the chaining function then composes them, combining the types correctly and in order. This makes them completely composable, as a combined chain is just a new middleware with those inputs and outputs again, and a listener is just a middleware with an empty context input.

I think if we go Typescript first, we should honor order to catch errors, as it is just a function chain in the end.

Do you see a way to add this functionality to this PR's implementation? I'm open to suggestions.

[iuioiua]

I will close this for now because this has gone stale and is not a priority right now. I'd be happy to look at a future PR that addresses some of the concerns mentioned here.