fix(types): allow Promise<void> return type for proxyRes callback#1190
fix(types): allow Promise<void> return type for proxyRes callback#1190chimurai merged 1 commit intochimurai:masterfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/types.ts`:
- Line 51: The proxyRes type allows async handlers but the event registration
currently casts them to (...args: unknown[]) => void so Promise rejections are
dropped; fix the registration in the proxy-events module by removing the void
cast and invoking handlers via Promise.resolve(handler(...args)).catch(err => /*
log/forward the error */), or by making the registration function async and
awaiting handler(...args) inside a try/catch; ensure any call sites that
register proxyRes handlers preserve the original signature so Promise<void>
rejections are logged/handled rather than ignored.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
| head: any, | ||
| ) => void; | ||
| proxyRes?: (proxyRes: TReq, req: TReq, res: TRes) => void; | ||
| proxyRes?: (proxyRes: TReq, req: TReq, res: TRes) => void | Promise<void>; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that async `options.on` handlers are currently registered without await/catch handling.
set -euo pipefail
# Inspect current event registration implementation
sed -n '1,140p' src/plugins/default/proxy-events.ts
# Show exact registration line and nearby context
rg -n -C3 "proxyServer\\.on<keyof typeof options\\.on>\\(eventName, handler as \\(\\.\\.\\.args: unknown\\[\\]\\) => void\\)" src/plugins/default/proxy-events.ts
# Check whether there is any Promise normalization/catch around handler invocation
rg -n -C2 "Promise\\.resolve\\(|\\.catch\\(|await .*handler" src/plugins/default/proxy-events.tsRepository: chimurai/http-proxy-middleware
Length of output: 1734
🏁 Script executed:
# Extract the OnProxyEvent type definition around line 51
sed -n '40,65p' src/types.tsRepository: chimurai/http-proxy-middleware
Length of output: 965
🏁 Script executed:
# Check if response-interceptor.ts relates to proxyRes events
rg -n "proxyRes|response-interceptor" src/handlers/response-interceptor.ts | head -20
# Check if any other event handlers wrap Promises in their registration
rg -n -B3 -A3 "Promise.*handler|catch.*handler|\.then(" src/plugins/default/proxy-events.ts
# Search broader for Promise handling patterns in proxy server event registration
rg -n "\.on\(" src/plugins/default/proxy-events.tsRepository: chimurai/http-proxy-middleware
Length of output: 935
🏁 Script executed:
# Search for where responseInterceptor is used/imported
rg -n "responseInterceptor|response-interceptor" --type ts | grep -v node_modules
# Search for how it relates to on.proxyRes
rg -n "on\.proxyRes|options\.on\.proxyRes" src/Repository: chimurai/http-proxy-middleware
Length of output: 1916
🏁 Script executed:
# Get the full responseInterceptor implementation to see if it handles Promise rejections
sed -n '23,100p' src/handlers/response-interceptor.tsRepository: chimurai/http-proxy-middleware
Length of output: 2557
proxyRes permits async handlers, but Promise rejections are silently dropped at runtime.
Line 51 correctly widens the type to void | Promise<void>, but handler registration at src/plugins/default/proxy-events.ts:42 still casts handlers to (...args: unknown[]) => void, causing any Promise rejections to be unhandled. This affects both direct async handlers and even utility wrappers like responseInterceptor that return Promise<void>, since both are registered without Promise handling.
💡 Suggested runtime guard in event registration
- proxyServer.on<keyof typeof options.on>(eventName, handler as (...args: unknown[]) => void);
+ proxyServer.on<keyof typeof options.on>(eventName, (...args: unknown[]) => {
+ try {
+ const result = (handler as (...a: unknown[]) => unknown)(...args);
+ if (result && typeof (result as Promise<unknown>).then === 'function') {
+ void (result as Promise<unknown>).catch((error) => {
+ debug(`event handler "${String(eventName)}" rejected: %o`, error);
+ });
+ }
+ } catch (error) {
+ debug(`event handler "${String(eventName)}" threw: %o`, error);
+ }
+ });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/types.ts` at line 51, The proxyRes type allows async handlers but the
event registration currently casts them to (...args: unknown[]) => void so
Promise rejections are dropped; fix the registration in the proxy-events module
by removing the void cast and invoking handlers via
Promise.resolve(handler(...args)).catch(err => /* log/forward the error */), or
by making the registration function async and awaiting handler(...args) inside a
try/catch; ensure any call sites that register proxyRes handlers preserve the
original signature so Promise<void> rejections are logged/handled rather than
ignored.
The proxyRes callback type only allowed void return type, but when using responseInterceptor, the handler is async and returns Promise<void>. This fix updates the type to allow both void and Promise<void> return types, matching the actual behavior of responseInterceptor. Fixes chimurai#1151
commit: |
|
Thanks! |
3 participants
Fixes #1151
Problem
The proxyRes callback type only allowed void return type, but when using responseInterceptor, the handler is async and returns Promise.
Solution
Updated the proxyRes type to allow both void and Promise return types.
Changes
Summary by CodeRabbit
Release Notes