feat(hono): support for hono with createHonoProxyMiddleware - #1193
Merged
Merged
Conversation
📝 WalkthroughWalkthroughAdds Hono framework support by introducing Changes
Sequence DiagramsequenceDiagram
participant Client
participant HonoApp as Hono App
participant HonoMW as createHonoProxyMiddleware
participant Proxy as createProxyMiddleware
participant Target as Target Server
Client->>HonoApp: HTTP request
HonoApp->>HonoMW: invoke middleware (c, next)
HonoMW->>Proxy: call proxy with c.env.incoming / c.env.outgoing
Proxy->>Target: forward request
Target-->>Proxy: response
alt success
Proxy-->>HonoMW: callback completes
HonoMW->>HonoApp: call next()
HonoApp-->>Client: proxied response
else error
Proxy-->>HonoMW: callback with error
HonoMW->>HonoMW: logger.error("Proxy error:", err)
HonoMW-->>Client: respond 500 "Proxy Error"
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
commit: |
chimurai
force-pushed
the
hono-proxy
branch
3 times, most recently
from
April 5, 2026 15:21
d6a5277 to
496e025
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
examples/hono/index.js (1)
22-27: Consider consolidating redundant startup messages.There are three separate console.log statements announcing the server startup. For a cleaner example, consider consolidating these.
Suggested simplification
-console.log('Server is running on http://localhost:3000'); - const server = serve(app); -console.log('[DEMO] Server: listening on port 3000'); +console.log('[DEMO] Server listening on http://localhost:3000'); console.log('[DEMO] Opening: http://localhost:3000/users');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@examples/hono/index.js` around lines 22 - 27, The three separate startup console.log calls are redundant; replace them with a single consolidated message after calling serve(app) that includes the server URL and the sample route (e.g., include "Server is running on http://localhost:3000 — Open: /users"). Update around the serve(app) invocation (refer to the server variable and serve/app usage) to remove the extra console.log lines and emit one clear startup log.src/factory-hono.ts (1)
10-12: Document theHttpBindingsrequirement for consumers.The function requires Hono to be configured with
@hono/node-serverbindings (HttpBindings). Users running Hono on other runtimes (Deno, Bun, Cloudflare Workers) won't havec.env.incomingandc.env.outgoingavailable. Consider adding this requirement to the JSDoc.Suggested JSDoc improvement
/** - * - * `@experimental` This API is experimental and may change without a major version bump. Use with caution. + * Creates a Hono middleware that proxies requests using http-proxy-middleware. + * + * `@remarks` + * This middleware requires Hono to be running on Node.js via `@hono/node-server`. + * It uses `c.env.incoming` and `c.env.outgoing` which are only available with `HttpBindings`. + * + * `@experimental` This API is experimental and may change without a major version bump. + * + * `@example` + * ```ts + * import { serve } from '@hono/node-server'; + * import { Hono } from 'hono'; + * import { createHonoProxyMiddleware } from 'http-proxy-middleware'; + * + * const app = new Hono(); + * app.use('/api', createHonoProxyMiddleware({ target: 'http://example.com' })); + * serve(app); + * ``` */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/factory-hono.ts` around lines 10 - 12, Document that createHonoProxyMiddleware requires Hono to be configured with `@hono/node-server-style` bindings (the HttpBindings type) because the middleware accesses c.env.incoming and c.env.outgoing; update the JSDoc above the createHonoProxyMiddleware declaration (and mention MiddlewareHandler<{ Bindings: HttpBindings }>) to show a minimal example of importing and serving with `@hono/node-server` (or state that other runtimes like Deno/Bun/Cloudflare Workers will not expose those env properties), so consumers know to use Hono + `@hono/node-server` or adapt bindings accordingly.recipes/servers.md (1)
141-159: Remove unusedopenimport from the recipe example.The
openpackage import is included in the example but never used in the code snippet. This may confuse readers who are trying to understand the minimal setup for Hono proxy integration.Suggested fix
```javascript import { serve } from '@hono/node-server'; import { Hono } from 'hono'; import { createHonoProxyMiddleware } from 'http-proxy-middleware'; -import open from 'open'; const app = new Hono(); app.use( '/users', createHonoProxyMiddleware({ target: 'http://jsonplaceholder.typicode.com', changeOrigin: true, // for vhosted sites, changes host header to match to target's host logger: console, }), ); const server = serve(app);</details> <details> <summary>🤖 Prompt for AI Agents</summary>Verify each finding against the current code and only fix it if needed.
In
@recipes/servers.mdaround lines 141 - 159, The snippet imports open but
never uses it; remove the unused import statement (the "open" import) from the
top of the file so the module imports only serve, Hono, and
createHonoProxyMiddleware; verify there are no remaining references to open
elsewhere (in case of later edits) and keep the rest of the code using app,
createHonoProxyMiddleware, and serve unchanged.</details> </blockquote></details> <details> <summary>test/e2e/hono.spec.ts (1)</summary><blockquote> `40-48`: **Consider awaiting server close and listening for server readiness to improve test reliability.** Two potential reliability concerns: 1. `serve()` returns immediately, but the server may not be fully bound to the port yet when the test runs. 2. `server.close()` is callback-based and doesn't return a Promise—calling it without awaiting completion could cause port conflicts or resource leaks between tests. <details> <summary>♻️ Proposed fix to improve test reliability</summary> ```diff beforeEach(async () => { app = new Hono<{ Bindings: HttpBindings }>(); serverPort = await getPort(); app.use( '/api', createHonoProxyMiddleware({ target: `http://localhost:${mockTargetServer.port}`, pathFilter: '/api', }), ); - server = serve({ - fetch: app.fetch, - port: serverPort, - }); + server = await new Promise<ServerType>((resolve) => { + const s = serve({ + fetch: app.fetch, + port: serverPort, + }); + s.once('listening', () => resolve(s)); + }); }); - afterEach(() => { - server.close(); + afterEach(async () => { + await new Promise<void>((resolve, reject) => { + server.close((err) => (err ? reject(err) : resolve())); + }); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/e2e/hono.spec.ts` around lines 40 - 48, The tests start the HTTP server with serve(...) assigned to server but don't wait for it to be bound and they call server.close() without waiting for the callback; update the setup/teardown to await readiness and close completion: in the beforeEach where you call serve({ fetch: app.fetch, port: serverPort }) wait for the server to emit a "listening" (or equivalent ready) event or use a Promise that resolves once the underlying listener is bound before proceeding; in afterEach wrap server.close(...) in a Promise that resolves on the close callback or "close" event and await that Promise to ensure the port is released before the next test. Ensure you locate and change the usage of serve, server, app.fetch, serverPort, and the afterEach teardown to implement these awaits.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Line 70: The package is missing optional peerDependencies for the Hono types
used by createHonoProxyMiddleware (its return type MiddlewareHandler<{ Bindings:
HttpBindings }> references hono and `@hono/node-server`), so add "hono" and
"@hono/node-server" to package.json under "peerDependencies" or
"optionalPeerDependencies" with appropriate semver ranges (e.g., caret ranges
matching the versions you support) so TypeScript consumers can resolve those
types; ensure the entries use the exact package names "hono" and
"@hono/node-server" so imports for MiddlewareHandler and HttpBindings compile
for downstream projects.
In `@src/factory-hono.ts`:
- Around line 15-24: Add an E2E test to hono.spec.ts that verifies the
middleware returned by the factory passes control to the next handler when
pathFilter does not match: create a Hono app that mounts the middleware produced
by the factory (the function that calls proxy(c.env.incoming, c.env.outgoing,
...)) with pathFilter set to a specific prefix (e.g., '/api'), then add a
downstream route/handler that returns a distinct response; send a request to a
non-matching path (e.g., '/other') and assert the response comes from the
downstream handler (confirming next() was invoked). Use the same pathFilter
identifier and the middleware factory name from the diff to locate the code;
mirror the non-matching test pattern used in http-proxy-middleware.spec.ts for
consistency.
- Around line 26-29: The catch block that currently does console.error('Proxy
error:', err) and calls c.status(500) must send a complete HTTP response and use
the provided logger from Options; update the proxy error handler to log via the
injected logger (the Options.logger) instead of console.error, and call
c.status(500).body(...) or c.json(...) to send an error body (e.g., a minimal
error message or JSON with error info) so the response is not left hanging;
locate the promise catch attached to the proxy call where c.status(500) is used
and replace the console.error and bare status call accordingly.
---
Nitpick comments:
In `@examples/hono/index.js`:
- Around line 22-27: The three separate startup console.log calls are redundant;
replace them with a single consolidated message after calling serve(app) that
includes the server URL and the sample route (e.g., include "Server is running
on http://localhost:3000 — Open: /users"). Update around the serve(app)
invocation (refer to the server variable and serve/app usage) to remove the
extra console.log lines and emit one clear startup log.
In `@recipes/servers.md`:
- Around line 141-159: The snippet imports open but never uses it; remove the
unused import statement (the "open" import) from the top of the file so the
module imports only serve, Hono, and createHonoProxyMiddleware; verify there are
no remaining references to open elsewhere (in case of later edits) and keep the
rest of the code using app, createHonoProxyMiddleware, and serve unchanged.
In `@src/factory-hono.ts`:
- Around line 10-12: Document that createHonoProxyMiddleware requires Hono to be
configured with `@hono/node-server-style` bindings (the HttpBindings type) because
the middleware accesses c.env.incoming and c.env.outgoing; update the JSDoc
above the createHonoProxyMiddleware declaration (and mention MiddlewareHandler<{
Bindings: HttpBindings }>) to show a minimal example of importing and serving
with `@hono/node-server` (or state that other runtimes like Deno/Bun/Cloudflare
Workers will not expose those env properties), so consumers know to use Hono +
`@hono/node-server` or adapt bindings accordingly.
In `@test/e2e/hono.spec.ts`:
- Around line 40-48: The tests start the HTTP server with serve(...) assigned to
server but don't wait for it to be bound and they call server.close() without
waiting for the callback; update the setup/teardown to await readiness and close
completion: in the beforeEach where you call serve({ fetch: app.fetch, port:
serverPort }) wait for the server to emit a "listening" (or equivalent ready)
event or use a Promise that resolves once the underlying listener is bound
before proceeding; in afterEach wrap server.close(...) in a Promise that
resolves on the close callback or "close" event and await that Promise to ensure
the port is released before the next test. Ensure you locate and change the
usage of serve, server, app.fetch, serverPort, and the afterEach teardown to
implement these awaits.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dca10b18-5307-401b-9950-0a56437e9e87
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (9)
CHANGELOG.mdREADME.mdexamples/hono/index.jspackage.jsonrecipes/servers.mdsrc/factory-hono.tssrc/index.tstest/e2e/hono.spec.tstest/e2e/test-kit.ts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/factory-hono.ts (1)
26-29:⚠️ Potential issue | 🟠 MajorError handling leaves response incomplete and ignores configured logger.
This issue was flagged in a previous review and appears unaddressed. Two problems:
c.status(500)sets the status but doesn't send a response body - the client connection may hang.console.erroris hardcoded instead of usingoptions.loggerwhich the user can configure.Suggested fix
.then(() => next()) .catch((err) => { - console.error('Proxy error:', err); - c.status(500); + const logger = options.logger || console; + if (typeof logger.error === 'function') { + logger.error('Proxy error:', err); + } + return c.text('Proxy Error', 500); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/factory-hono.ts` around lines 26 - 29, The catch handler currently logs with console.error and only calls c.status(500) leaving the response body empty; update the .catch((err) => { ... }) block to log via the configured options.logger (falling back to console.error) and send a complete response body so the request doesn't hang (e.g., call options.logger.error(err) or (options.logger?.error ?? console.error)(err) and then return c.status(500).text('Internal Server Error') or c.body('Internal Server Error') to finalize the response using the existing context variable c).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@recipes/servers.md`:
- Around line 141-161: Remove the unused import "open" (delete the `import open
from 'open';` line) and update the example curl comment to match the mounted
route by changing the comment from "/api/users" to "/users" so the route used in
`app.use('/users', createHonoProxyMiddleware(...))` aligns with the example;
keep the remaining imports (`serve`, `Hono`, `createHonoProxyMiddleware`) and
the `serve(app)` usage as-is.
---
Duplicate comments:
In `@src/factory-hono.ts`:
- Around line 26-29: The catch handler currently logs with console.error and
only calls c.status(500) leaving the response body empty; update the
.catch((err) => { ... }) block to log via the configured options.logger (falling
back to console.error) and send a complete response body so the request doesn't
hang (e.g., call options.logger.error(err) or (options.logger?.error ??
console.error)(err) and then return c.status(500).text('Internal Server Error')
or c.body('Internal Server Error') to finalize the response using the existing
context variable c).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1bcc520a-fcf8-42fe-b001-75aa12c264eb
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (9)
CHANGELOG.mdREADME.mdexamples/hono/index.jspackage.jsonrecipes/servers.mdsrc/factory-hono.tssrc/index.tstest/e2e/hono.spec.tstest/e2e/test-kit.ts
✅ Files skipped from review due to trivial changes (6)
- package.json
- README.md
- CHANGELOG.md
- examples/hono/index.js
- test/e2e/test-kit.ts
- test/e2e/hono.spec.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/index.ts
chimurai
force-pushed
the
hono-proxy
branch
4 times, most recently
from
April 5, 2026 15:39
83b0e4d to
9b82c9a
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
recipes/servers.md (1)
145-145:⚠️ Potential issue | 🟡 MinorRemove unused import.
The
openimport is not used anywhere in the code snippet and should be removed.🧹 Proposed fix
import { serve } from '@hono/node-server'; import { Hono } from 'hono'; import { createHonoProxyMiddleware } from 'http-proxy-middleware'; -import open from 'open'; const app = new Hono();Note: The curl path comment on line 160 has been correctly updated to
/usersto match the mounted route.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@recipes/servers.md` at line 145, Remove the unused import "open" from the top of the file: delete the line importing open (the import statement import open from 'open';) since it's not referenced anywhere in recipes/servers.md; ensure no other references to the open identifier remain and run a quick lint or build to confirm no unused-import warnings persist.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@recipes/servers.md`:
- Line 145: Remove the unused import "open" from the top of the file: delete the
line importing open (the import statement import open from 'open';) since it's
not referenced anywhere in recipes/servers.md; ensure no other references to the
open identifier remain and run a quick lint or build to confirm no unused-import
warnings persist.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 85a15eba-9dd2-462a-82ca-7c68f1d5c8da
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (9)
CHANGELOG.mdREADME.mdexamples/hono/index.jspackage.jsonrecipes/servers.mdsrc/factory-hono.tssrc/index.tstest/e2e/hono.spec.tstest/e2e/test-kit.ts
✅ Files skipped from review due to trivial changes (4)
- CHANGELOG.md
- examples/hono/index.js
- README.md
- test/e2e/hono.spec.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- package.json
- test/e2e/test-kit.ts
- src/factory-hono.ts
chimurai
force-pushed
the
hono-proxy
branch
2 times, most recently
from
April 5, 2026 15:46
ca718b6 to
1b0bb0d
Compare
Closed
There was a problem hiding this comment.
🧹 Nitpick comments (1)
recipes/servers.md (1)
157-157: Optional: drop unusedservervariable in the snippet.Line 157 assigns
serve(app)toserver, butserveris never used in this example. Consider simplifying toserve(app);to keep the docs minimal.Suggested doc tweak
-const server = serve(app); +serve(app);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@recipes/servers.md` at line 157, The snippet assigns the result of serve(app) to an unused variable named server; remove the unused binding and call serve(app) directly (replace "const server = serve(app);" with simply "serve(app);") so the example is minimal and avoids an unused-variable declaration, referencing the symbol server and the call serve(app) to locate the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@recipes/servers.md`:
- Line 157: The snippet assigns the result of serve(app) to an unused variable
named server; remove the unused binding and call serve(app) directly (replace
"const server = serve(app);" with simply "serve(app);") so the example is
minimal and avoids an unused-variable declaration, referencing the symbol server
and the call serve(app) to locate the change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1e1bc2ac-7ea7-48b8-b361-d2ffd009978e
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (9)
CHANGELOG.mdREADME.mdexamples/hono/index.jspackage.jsonrecipes/servers.mdsrc/factory-hono.tssrc/index.tstest/e2e/hono.spec.tstest/e2e/test-kit.ts
✅ Files skipped from review due to trivial changes (6)
- package.json
- src/index.ts
- examples/hono/index.js
- CHANGELOG.md
- README.md
- test/e2e/hono.spec.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- test/e2e/test-kit.ts
- src/factory-hono.ts
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fixes: #992
Add support for Hono:
Summary by CodeRabbit
New Features
Documentation
Examples
Tests
Changelog