Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(http): middleware #3785

Closed
wants to merge 13 commits into from
54 changes: 54 additions & 0 deletions http/middleware.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.

/**
* Middleware handler based on {@linkcode Deno.ServeHandlerInfo} but an added
* `next()` function which calls the next middleware handler in the middleware
* chain.
*/
export type MiddlewareHandler = (
request: Request,
info: Deno.ServeHandlerInfo,
Copy link
Contributor

@syhol syhol Dec 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I'm thinking that too.

next: () => Response | Promise<Response>,
) => Response | Promise<Response>;

/**
* Creates a {@linkcode Deno.ServeHandler} from the given middleware chain,
* which can then be passed to {@linkcode Deno.serve}.
*
* @param middlewares Middleware chain
*
* @example
* ```ts
* import {
* type MiddlewareHandler,
* createHandler,
* } from "https://deno.land/std@$STD_VERSION/http/middleware.ts";
*
* const middleware1: MiddlewareHandler = async (_request, _info, next) => {
* const start = performance.now();
* const response = await next();
* const duration = performance.now() - start;
* response.headers.set("x-request-time", duration.toString());
* return response;
* };
*
* const middleware2: MiddlewareHandler = (request, info) => {
* return Response.json({ request, info });
* };
*
* const handler = createHandler([middleware1, middleware2])
* ```
*/
export function createHandler(
middlewares: MiddlewareHandler[],
): Deno.ServeHandler {
return (request, info) => {
function compose(index: number): Response | Promise<Response> {
iuioiua marked this conversation as resolved.
Show resolved Hide resolved
if (index >= middlewares.length) {
throw new RangeError("Middleware chain exhausted");
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this error make sense?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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]);

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
return middlewares[index](request, info, () => compose(index + 1));
}
return compose(0);
};
}
55 changes: 55 additions & 0 deletions http/middleware_test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
// Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.

import { assertEquals, assertRejects } from "../assert/mod.ts";
import { createHandler, type MiddlewareHandler } from "./middleware.ts";

const info: Deno.ServeHandlerInfo = {
remoteAddr: { transport: "tcp", hostname: "foo", port: 200 },
};

Deno.test("createHandler() chains middlewares in order", async () => {
const order: number[] = [];

const middleware1: MiddlewareHandler = async (_request, _info, next) => {
const response = await next();
response.headers.set("X-Foo-1", "Bar-1");
order.push(1);
return response;
};

const middleware2: MiddlewareHandler = async (_request, _info, next) => {
const response = await next();
response.headers.set("X-Foo-2", "Bar-2");
order.push(2);
return response;
};

const finalMiddleware: MiddlewareHandler = () => {
order.push(3);
return new Response();
};

const handler = createHandler([middleware1, middleware2, finalMiddleware]);
const request = new Request("http://localhost");
const response = await handler(request, info);

assertEquals(response.status, 200);
assertEquals(response.headers.get("X-Foo-1"), "Bar-1");
assertEquals(response.headers.get("X-Foo-2"), "Bar-2");
assertEquals(order, [3, 2, 1]);
});

Deno.test("createHandler() throws when next() is called incorrectly", async () => {
const finalMiddleware: MiddlewareHandler = async (_request, _info, next) => {
await next();
return new Response();
};

const handler = createHandler([finalMiddleware]);
const request = new Request("http://localhost");
await assertRejects(
async () => await handler(request, info),
RangeError,
"Middleware chain exhausted",
);
});
1 change: 1 addition & 0 deletions http/mod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export * from "./etag.ts";
export * from "./http_errors.ts";
export * from "./http_status.ts";
export * from "./method.ts";
export * from "./middleware.ts";
export * from "./negotiation.ts";
export * from "./server.ts";
export * from "./server_sent_event.ts";
Expand Down