feat: middleware for compatibility with express/connect for Hono

[EdamAme-x]

resolve: https://github.com/honojs/hono/issues/3293

Still some concerns and bugs.

The author should do the following, if applicable

• [x] Add tests
• [x] Run tests
• [x] yarn changeset at the top of this repo and push the changeset
• [x] Follow the contribution guide

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: b393f280b89cb4f65b41f6f66057e5e60e8821cf

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@hono/connect	Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[EdamAme-x]

hi @yusukebe

There are two problems with this implementation.

The first is regarding the header update process.

This test fails. https://github.com/honojs/middleware/pull/928/files#diff-05c9dabcbf8c76abe4ba387843468024649359e039e0f521ed755f44de39ca44R38-R43

Here is the cause. https://github.com/honojs/middleware/pull/928/files#diff-e82bf8d64b2d70f2d39b9aa64ada59b6d93549da4aa4f72f7c9f15e3d7426fb0R43

The header is not removed successfully.

[EdamAme-x]

Second, even using “next” internally does not count as Hono's overall middleware. It is instantiated once internally and the request is passed on.

However, this problem can be sufficiently avoided by being careful about the order of execution.

(It is obvious that the performance is quite bad)

[EdamAme-x]

ToDo: mark as dev deps "helmet": "^8.0.0",

[EdamAme-x]

hi @yusukebe @sor4chi Do you know any reason why the header cannot be removed successfully here?

https://github.com/honojs/middleware/pull/928/files#diff-e82bf8d64b2d70f2d39b9aa64ada59b6d93549da4aa4f72f7c9f15e3d7426fb0R43

[yusukebe]

Hi @EdamAme-x

I think this approach is okay (actually, there is only this way), though the overheads are heavy as you said. I'll check the problems.

[dajulia3]

Just here to say that this would be incredibly useful. If you need any additional help or eyes on test edge cases let me know.

[kirill-dev-pro]

Is there something I could help with this PR? Would be great to have it merged

[EdamAme-x]

There are still a few bugs that need to be fixed.

[mbrevda]

There are still a few bugs that need to be fixed.

Can you elaborate? Perhaps someone can jump in here and help

[mridang]

Hi @EdamAme-x any help needed with the issue? One thing I've found is that there is a chance of of Prematurely Finalized Response being Ignored.

The potentiald bug is in how the middleware handles responses sent directly from a Connect middleware (e.g., res.send(), res.end()).

IIRC, in Hono, when a middleware sets a response directly on the context (c.res = ...), it must also set c.finalized = true which tells the Hono engine that response is complete and to stop processing any further middleware or handlers and send this response immediately.

thhe missing c.finalized in packages/connect/src/index.ts, the code block is if (res) { c.res = res; }, which is missing c.finalized = true;. This bug might prevent a middleware from successfully ending the request chain.

I also noticed that you are overwriting the headers.

const connectResponse = transformResponseToServerResponse(response);
        const preparedHeaders = (c.newResponse(null, 204, {})).headers;
        const connectHeaders = connectResponse.headers;

        for (const key of [...preparedHeaders.keys()]) {
          c.header(key, undefined);
        }

        for (const [key, value] of [...connectHeaders.entries()]) {
          c.header(key, value);
        }

Wouldn't this cause some middleware headers to be lost?

[Caceresenzo]

Your branch is pretty old and still use a version of Hono that does not propery handle header clearing.

#928 (accidentally?) fixed it. (link to patch)

The faulty lines were:

if (this.#headers) {
	this.#headers.delete(name)
} else if (this.#preparedHeaders) {
	delete this.#preparedHeaders[name.toLocaleLowerCase()]
}

By using a else-if, the headers were not deleted from #preparedHeaders, as they were stored at both variable at the same time. Removing the else (keeping just the if) fix your test.

You can also upgrade to the latest version of Hono, it also make the test pass.

---

There is also other issues, while trying to make a proof of concept using express.static middleware, I found out that the URL was not properly set. As if you are in a middleware, you only get it without the prefix.

Meaning that transformRequestToIncomingMessage, do not send it correctly:

export function transformRequestToIncomingMessage(
	c: Context, // Yes context is needed here
	options?: RequestOptions,
) {
	const request = c.req.raw;
	const parsedURL = new URL(request.url, "http://localhost");

	// ...

	let middlewarePath = routePath(c);

	if (middlewarePath.at(-1) === '*') {
		middlewarePath = middlewarePath.slice(0, -1);
	}
	if (middlewarePath.at(-1) === '/') {
		middlewarePath = middlewarePath.slice(0, -1);
	}

	const message = createRequest({
		method: request.method.toUpperCase() as RequestMethod,
		url: parsedURL.pathname.substring(middlewarePath.length),
		headers: Object.fromEntries(request.headers.entries()),
	// ...

	return message;
}

I was unable to make the middleware works in the end as streaming require to use hono/streaming which could be tricky to intercept.