refactor!: next major version (6.x)#9
Conversation
…dling for legacy usage
…app' and add uninstall functionality for plugins
审阅者指南重构 Fexios 核心请求/响应流水线和上下文模型,移除内置的 WebSocket/SSE 支持,改为通过独立插件(ws/sse/cookie‑jar)提供;新增插件卸载能力和新的生命周期事件,强化 URL/查询处理逻辑,并为即将到来的 6.x 主要版本更新文档和测试。 更新后的 HTTP 请求流水线和上下文处理顺序图sequenceDiagram
actor User
participant Fexios
participant Hooks as LifecycleHooks
participant Fetch as FetchImpl
participant Parser as createFexiosResponse
User->>Fexios: request(urlOrOptions, options)
Note over Fexios: Build initial FexiosContext with<br/>ctx.app, ctx.request, ctx.runtime
Fexios->>Hooks: emit(beforeInit, ctx)
Hooks-->>Fexios: ctx'
alt ctx' is final
Fexios-->>User: FexiosFinalContext
else continue
Note over Fexios: Apply runtime defaults (customEnv)
Fexios->>Fexios: applyDefaults(ctx.request)
Note over Fexios: Normalize body, headers, URL, query
Fexios->>Fexios: build Request
Fexios->>Fexios: ctx.request.rawRequest = Request
Fexios->>Hooks: emit(beforeActualFetch, ctx)
Hooks-->>Fexios: ctx''
alt ctx'' is final
Fexios-->>User: FexiosFinalContext
else perform fetch
Fexios->>Fetch: fetch(ctx.request.rawRequest)
Fetch-->>Fexios: rawResponse
Fexios->>Fexios: ctx.rawResponse = rawResponse
Fexios->>Hooks: emit(afterRawResponse, ctx)
Fexios->>Parser: createFexiosResponse(rawResponse, ctx.request.responseType, onProgress, shouldThrow, timeout)
Parser-->>Fexios: response
Fexios->>Fexios: ctx.response = response
Fexios->>Fexios: ctx.rawResponse = response.rawResponse
Fexios->>Fexios: finalizeContext(ctx, finalURL)
Fexios->>Hooks: emit(afterResponse, ctx)
Hooks-->>Fexios: finalCtx
Fexios-->>User: FexiosFinalContext
end
end
通过 pluginWebSocket 建立 WebSocket 连接的顺序图sequenceDiagram
actor User
participant Fexios
participant WSPlugin as pluginWebSocket
participant Hooks as LifecycleHooks
participant WS as WebSocket
User->>Fexios: ws(url, options)
Note over Fexios: pluginWebSocket installed adds ws method
Fexios->>WSPlugin: normalizeWsURL(url, baseURL, query)
WSPlugin-->>Fexios: wsUrl
Fexios->>Fexios: baseCtx = { url, protocols, timeout }
Fexios->>Hooks: emit(websocket:beforeConnect, baseCtx)
Hooks-->>Fexios: ctx
Fexios->>WS: new WebSocket(ctx.url, ctx.protocols)
Fexios->>Fexios: ctx.socket = WebSocket
WS->>Fexios: open
Fexios->>Hooks: emit(websocket:open, ctx)
Note over Fexios,WS: ws message events
WS-->>Fexios: message(event)
Fexios->>Hooks: emit(websocket:message, ctx + event)
Note over Fexios: waitForWsOpen(ws, ctx.timeout)
Fexios-->>User: WebSocket (opened)
WS-->>Fexios: error(event)
Fexios->>Hooks: emit(websocket:error, ctx + event)
WS-->>Fexios: close(event)
Fexios->>Hooks: emit(websocket:close, ctx + event)
通过 pluginSSE 建立 SSE 连接的顺序图sequenceDiagram
actor User
participant Fexios
participant SSEPlugin as pluginSSE
participant Hooks as LifecycleHooks
participant ES as EventSource
User->>Fexios: sse(url, options)
Note over Fexios: pluginSSE installed adds sse method
Fexios->>SSEPlugin: normalizeSseURL(url, baseURL, query)
SSEPlugin-->>Fexios: sseUrl
Fexios->>Fexios: baseCtx = { url, timeout }
Fexios->>Hooks: emit(sse:beforeConnect, baseCtx)
Hooks-->>Fexios: ctx
Fexios->>ES: new EventSource(ctx.url)
Fexios->>Fexios: ctx.eventSource = EventSource
ES->>Fexios: open(event)
Fexios->>Hooks: emit(sse:open, ctx + event)
Note over Fexios,ES: sse message events
ES-->>Fexios: message(event)
Fexios->>Hooks: emit(sse:message, ctx + event)
ES-->>Fexios: error(event)
Fexios->>Hooks: emit(sse:error, ctx + event)
Note over Fexios: waitForSseOpen(es, ctx.timeout)
Fexios-->>User: EventSource (opened)
更新后的 Fexios 核心上下文及插件系统类图classDiagram
class Fexios {
+baseConfigs : FexiosConfigs
-_plugins : Map~string, FexiosPlugin~
+request(url, options) FexiosFinalContext
+plugin(plugin) Fexios
+uninstall(pluginOrName) Fexios
+on(event, handler) void
+off(event, handler) void
+interceptors : FexiosInterceptors
+cookieJar : CookieJar
+ws(url, options) WebSocket
+sse(url, options) EventSource
-attachLegacyAliases(ctx) void
-finalizeContext(ctx, fallbackURL) void
-applyDefaults(request) any
}
class FexiosConfigs {
+baseURL : string
+headers : Record~string, unknown~
+timeout : number
+responseType : FexiosResponseType
+shouldThrow : (response) => boolean
+fetch : FetchLike
+customEnv : any
}
class FexiosRequestOptions {
+url : string | URL
+method : FexiosMethods
+headers : Record~string, unknown~
+query : Record~string, any~ | URLSearchParams
+body : any
+timeout : number
+credentials : RequestCredentials
+cache : RequestCache
+mode : RequestMode
+responseType : FexiosResponseType
+fetch : FetchLike
+customEnv : any
+abortController : AbortController
+onProgress(progress, buffer) void
}
class FexiosRequestContext {
+url : string
+method : FexiosMethods
+headers : Record~string, unknown~
+query : Record~string, any~ | URLSearchParams
+body : any
+timeout : number
+credentials : RequestCredentials
+cache : RequestCache
+mode : RequestMode
+responseType : FexiosResponseType
+fetch : FetchLike
+rawRequest : Request
}
class FexiosRuntimeContext {
+abortController : AbortController
+onProgress(progress, buffer) void
+customEnv : any
}
class FexiosResponseContext {
<<alias>>
}
class FexiosContext {
+app : Fexios
+request : FexiosRequestContext
+runtime : FexiosRuntimeContext
+response : FexiosResponse
+rawResponse : Response
%% legacy aliases
+url : string
+method : FexiosMethods
+headers : Record~string, unknown~
+query : Record~string, any~ | URLSearchParams
+body : any
+abortController : AbortController
+onProgress(progress, buffer) void
+customEnv : any
+rawRequest : Request
+data : any
}
class FexiosFinalContext {
+app : Fexios
+request : FexiosRequestContext
+runtime : FexiosRuntimeContext
+response : FexiosResponse
+rawRequest : Request
+rawResponse : Response
+headers : Headers
+url : string
+responseType : FexiosResponseType
+data : any
}
class FexiosResponse {
+ok : boolean
+status : number
+statusText : string
+headers : Headers
+data : any
+rawResponse : Response
+responseType : FexiosResponseType
}
class FexiosInterceptors {
+request : FexiosInterceptorBeforeRequest
+response : FexiosInterceptorAfterResponse
}
class FexiosInterceptorBeforeRequest {
+handlers() FexiosHookHandlerBeforeRequest[]
+use(hook, prepend) any
+clear() void
}
class FexiosInterceptorAfterResponse {
+handlers() FexiosHookHandlerAfterResponse[]
+use(hook, prepend) any
+clear() void
}
class FexiosPlugin {
+name : string
+install(fx) Fexios
+uninstall(fx) void
}
class CookieJar {
+setCookie(cookie, domain, path) void
+getCookie(name, domain, path) any
+getCookies(domain, path) any[]
+getCookieHeader(domain, path) string
+clear() void
+cleanExpiredCookies() void
}
class pluginCookieJar {
+name : string
+install(fx) void
+uninstall(fx) void
-COOKIE_JAR_PLUGIN_UNINSTALLER : symbol
}
class pluginWebSocket {
+name : string
+install(fx) Fexios
+uninstall(fx) void
}
class pluginSSE {
+name : string
+install(fx) Fexios
+uninstall(fx) void
}
class FexiosWebSocketContext {
+url : string
+protocols : string | string[]
+timeout : number
+socket : WebSocket
}
class FexiosSSEContext {
+url : string
+timeout : number
+eventSource : EventSource
}
Fexios --> FexiosConfigs
Fexios --> FexiosInterceptors
Fexios --> FexiosContext
FexiosContext --> FexiosRequestContext
FexiosContext --> FexiosRuntimeContext
FexiosContext --> FexiosResponse
FexiosFinalContext --> FexiosContext
FexiosInterceptors --> FexiosInterceptorBeforeRequest
FexiosInterceptors --> FexiosInterceptorAfterResponse
FexiosPlugin <|-- pluginCookieJar
FexiosPlugin <|-- pluginWebSocket
FexiosPlugin <|-- pluginSSE
Fexios --> FexiosPlugin : plugin()
Fexios --> FexiosPlugin : uninstall()
Fexios --> CookieJar : cookieJar
pluginCookieJar --> CookieJar
pluginWebSocket --> FexiosWebSocketContext
pluginSSE --> FexiosSSEContext
文件级变更
提示与命令与 Sourcery 交互
自定义你的体验访问你的 控制面板 可以:
获取帮助Original review guide in EnglishReviewer's GuideRefactors Fexios core request/response pipeline and context model, removes built‑in WebSocket/SSE support in favor of dedicated plugins (ws/sse/cookie‑jar), adds plugin uninstall and new lifecycle events, tightens URL/query handling, and updates docs/tests for the upcoming 6.x major release. Sequence diagram for updated HTTP request pipeline and context handlingsequenceDiagram
actor User
participant Fexios
participant Hooks as LifecycleHooks
participant Fetch as FetchImpl
participant Parser as createFexiosResponse
User->>Fexios: request(urlOrOptions, options)
Note over Fexios: Build initial FexiosContext with<br/>ctx.app, ctx.request, ctx.runtime
Fexios->>Hooks: emit(beforeInit, ctx)
Hooks-->>Fexios: ctx'
alt ctx' is final
Fexios-->>User: FexiosFinalContext
else continue
Note over Fexios: Apply runtime defaults (customEnv)
Fexios->>Fexios: applyDefaults(ctx.request)
Note over Fexios: Normalize body, headers, URL, query
Fexios->>Fexios: build Request
Fexios->>Fexios: ctx.request.rawRequest = Request
Fexios->>Hooks: emit(beforeActualFetch, ctx)
Hooks-->>Fexios: ctx''
alt ctx'' is final
Fexios-->>User: FexiosFinalContext
else perform fetch
Fexios->>Fetch: fetch(ctx.request.rawRequest)
Fetch-->>Fexios: rawResponse
Fexios->>Fexios: ctx.rawResponse = rawResponse
Fexios->>Hooks: emit(afterRawResponse, ctx)
Fexios->>Parser: createFexiosResponse(rawResponse, ctx.request.responseType, onProgress, shouldThrow, timeout)
Parser-->>Fexios: response
Fexios->>Fexios: ctx.response = response
Fexios->>Fexios: ctx.rawResponse = response.rawResponse
Fexios->>Fexios: finalizeContext(ctx, finalURL)
Fexios->>Hooks: emit(afterResponse, ctx)
Hooks-->>Fexios: finalCtx
Fexios-->>User: FexiosFinalContext
end
end
Sequence diagram for WebSocket connection via pluginWebSocketsequenceDiagram
actor User
participant Fexios
participant WSPlugin as pluginWebSocket
participant Hooks as LifecycleHooks
participant WS as WebSocket
User->>Fexios: ws(url, options)
Note over Fexios: pluginWebSocket installed adds ws method
Fexios->>WSPlugin: normalizeWsURL(url, baseURL, query)
WSPlugin-->>Fexios: wsUrl
Fexios->>Fexios: baseCtx = { url, protocols, timeout }
Fexios->>Hooks: emit(websocket:beforeConnect, baseCtx)
Hooks-->>Fexios: ctx
Fexios->>WS: new WebSocket(ctx.url, ctx.protocols)
Fexios->>Fexios: ctx.socket = WebSocket
WS->>Fexios: open
Fexios->>Hooks: emit(websocket:open, ctx)
Note over Fexios,WS: ws message events
WS-->>Fexios: message(event)
Fexios->>Hooks: emit(websocket:message, ctx + event)
Note over Fexios: waitForWsOpen(ws, ctx.timeout)
Fexios-->>User: WebSocket (opened)
WS-->>Fexios: error(event)
Fexios->>Hooks: emit(websocket:error, ctx + event)
WS-->>Fexios: close(event)
Fexios->>Hooks: emit(websocket:close, ctx + event)
Sequence diagram for SSE connection via pluginSSEsequenceDiagram
actor User
participant Fexios
participant SSEPlugin as pluginSSE
participant Hooks as LifecycleHooks
participant ES as EventSource
User->>Fexios: sse(url, options)
Note over Fexios: pluginSSE installed adds sse method
Fexios->>SSEPlugin: normalizeSseURL(url, baseURL, query)
SSEPlugin-->>Fexios: sseUrl
Fexios->>Fexios: baseCtx = { url, timeout }
Fexios->>Hooks: emit(sse:beforeConnect, baseCtx)
Hooks-->>Fexios: ctx
Fexios->>ES: new EventSource(ctx.url)
Fexios->>Fexios: ctx.eventSource = EventSource
ES->>Fexios: open(event)
Fexios->>Hooks: emit(sse:open, ctx + event)
Note over Fexios,ES: sse message events
ES-->>Fexios: message(event)
Fexios->>Hooks: emit(sse:message, ctx + event)
ES-->>Fexios: error(event)
Fexios->>Hooks: emit(sse:error, ctx + event)
Note over Fexios: waitForSseOpen(es, ctx.timeout)
Fexios-->>User: EventSource (opened)
Class diagram for updated Fexios core context and plugin systemclassDiagram
class Fexios {
+baseConfigs : FexiosConfigs
-_plugins : Map~string, FexiosPlugin~
+request(url, options) FexiosFinalContext
+plugin(plugin) Fexios
+uninstall(pluginOrName) Fexios
+on(event, handler) void
+off(event, handler) void
+interceptors : FexiosInterceptors
+cookieJar : CookieJar
+ws(url, options) WebSocket
+sse(url, options) EventSource
-attachLegacyAliases(ctx) void
-finalizeContext(ctx, fallbackURL) void
-applyDefaults(request) any
}
class FexiosConfigs {
+baseURL : string
+headers : Record~string, unknown~
+timeout : number
+responseType : FexiosResponseType
+shouldThrow : (response) => boolean
+fetch : FetchLike
+customEnv : any
}
class FexiosRequestOptions {
+url : string | URL
+method : FexiosMethods
+headers : Record~string, unknown~
+query : Record~string, any~ | URLSearchParams
+body : any
+timeout : number
+credentials : RequestCredentials
+cache : RequestCache
+mode : RequestMode
+responseType : FexiosResponseType
+fetch : FetchLike
+customEnv : any
+abortController : AbortController
+onProgress(progress, buffer) void
}
class FexiosRequestContext {
+url : string
+method : FexiosMethods
+headers : Record~string, unknown~
+query : Record~string, any~ | URLSearchParams
+body : any
+timeout : number
+credentials : RequestCredentials
+cache : RequestCache
+mode : RequestMode
+responseType : FexiosResponseType
+fetch : FetchLike
+rawRequest : Request
}
class FexiosRuntimeContext {
+abortController : AbortController
+onProgress(progress, buffer) void
+customEnv : any
}
class FexiosResponseContext {
<<alias>>
}
class FexiosContext {
+app : Fexios
+request : FexiosRequestContext
+runtime : FexiosRuntimeContext
+response : FexiosResponse
+rawResponse : Response
%% legacy aliases
+url : string
+method : FexiosMethods
+headers : Record~string, unknown~
+query : Record~string, any~ | URLSearchParams
+body : any
+abortController : AbortController
+onProgress(progress, buffer) void
+customEnv : any
+rawRequest : Request
+data : any
}
class FexiosFinalContext {
+app : Fexios
+request : FexiosRequestContext
+runtime : FexiosRuntimeContext
+response : FexiosResponse
+rawRequest : Request
+rawResponse : Response
+headers : Headers
+url : string
+responseType : FexiosResponseType
+data : any
}
class FexiosResponse {
+ok : boolean
+status : number
+statusText : string
+headers : Headers
+data : any
+rawResponse : Response
+responseType : FexiosResponseType
}
class FexiosInterceptors {
+request : FexiosInterceptorBeforeRequest
+response : FexiosInterceptorAfterResponse
}
class FexiosInterceptorBeforeRequest {
+handlers() FexiosHookHandlerBeforeRequest[]
+use(hook, prepend) any
+clear() void
}
class FexiosInterceptorAfterResponse {
+handlers() FexiosHookHandlerAfterResponse[]
+use(hook, prepend) any
+clear() void
}
class FexiosPlugin {
+name : string
+install(fx) Fexios
+uninstall(fx) void
}
class CookieJar {
+setCookie(cookie, domain, path) void
+getCookie(name, domain, path) any
+getCookies(domain, path) any[]
+getCookieHeader(domain, path) string
+clear() void
+cleanExpiredCookies() void
}
class pluginCookieJar {
+name : string
+install(fx) void
+uninstall(fx) void
-COOKIE_JAR_PLUGIN_UNINSTALLER : symbol
}
class pluginWebSocket {
+name : string
+install(fx) Fexios
+uninstall(fx) void
}
class pluginSSE {
+name : string
+install(fx) Fexios
+uninstall(fx) void
}
class FexiosWebSocketContext {
+url : string
+protocols : string | string[]
+timeout : number
+socket : WebSocket
}
class FexiosSSEContext {
+url : string
+timeout : number
+eventSource : EventSource
}
Fexios --> FexiosConfigs
Fexios --> FexiosInterceptors
Fexios --> FexiosContext
FexiosContext --> FexiosRequestContext
FexiosContext --> FexiosRuntimeContext
FexiosContext --> FexiosResponse
FexiosFinalContext --> FexiosContext
FexiosInterceptors --> FexiosInterceptorBeforeRequest
FexiosInterceptors --> FexiosInterceptorAfterResponse
FexiosPlugin <|-- pluginCookieJar
FexiosPlugin <|-- pluginWebSocket
FexiosPlugin <|-- pluginSSE
Fexios --> FexiosPlugin : plugin()
Fexios --> FexiosPlugin : uninstall()
Fexios --> CookieJar : cookieJar
pluginCookieJar --> CookieJar
pluginWebSocket --> FexiosWebSocketContext
pluginSSE --> FexiosSSEContext
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - 我发现了 4 个安全问题,以及另外 10 个问题。
安全问题:
- 检测到不安全的 WebSocket。所有 WebSocket 连接都应该使用安全的 WebSocket(wss)。(link)
- 检测到不安全的 WebSocket。所有 WebSocket 连接都应该使用安全的 WebSocket(wss)。(link)
- 检测到不安全的 WebSocket。所有 WebSocket 连接都应该使用安全的 WebSocket(wss)。(link)
- 检测到不安全的 WebSocket。所有 WebSocket 连接都应该使用安全的 WebSocket(wss)。(link)
给 AI Agent 的提示词
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/fexios.ts:566-567` </location>
<code_context>
}
}
- return this
+ return () => {
+ this.uninstall(plugin)
+ }
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** 现在 `plugin()` 在数组调用和单插件调用之间的返回类型不一致,可能会破坏现有的链式调用用法。
对于数组调用,`plugin([...])` 仍然返回 `Fexios` 实例,但对于单插件调用,现在返回的是一个卸载函数。这会导致像 `fx.plugin(pluginWebSocket).get(...)` 这样的单插件链式调用失效,而数组调用仍然可用。
如果你并没有明确打算引入这种变更,建议要么始终返回 `Fexios` 实例(并依赖显式的 `fx.uninstall()` API),要么返回一个兼容旧版的结构,例如 `{ fx, uninstall }`。即使这是有意的破坏式变更,也最好让数组和单插件分支返回相同的结构,以保持 API 的可预测性。
</issue_to_address>
### Comment 2
<location> `src/plugins/cookie-jar/index.ts:74-75` </location>
<code_context>
+ install(fx) {
+ fx.hello = (name) => `Hello, ${name}`
+ },
+ uninstall(fx) {
+ fx.hello = undefined as any
+ },
</code_context>
<issue_to_address>
**suggestion (bug_risk):** cookie-jar 插件的 `uninstall` 目前只清理了 `cookieJar`,但保留了拦截器,这可能导致细微的行为问题和内存泄漏。
现在 `uninstall` 只清除插件状态(`fx.cookieJar`),但请求/响应拦截器仍然是注册状态。它们会在每次调用时继续执行,只是在 `!fx.cookieJar` 时立刻返回,这会保留不必要的钩子,带来额外开销并可能导致泄漏。
如果你的拦截器 API 提供移除/清理函数,请在 `install` 期间保存这些函数,并在 `uninstall` 中调用它们,这样插件在移除时可以完全卸载自己的拦截器。
建议实现:
```typescript
uninstall(fx) {
// 清理插件状态
fx.cookieJar = undefined
// 如果请求/响应拦截器已安装,解绑它们
if (fx.__cookieJarRequestInterceptor) {
fx.__cookieJarRequestInterceptor()
}
if (fx.__cookieJarResponseInterceptor) {
fx.__cookieJarResponseInterceptor()
}
// 清除对拦截器清理函数的引用
fx.__cookieJarRequestInterceptor = undefined
fx.__cookieJarResponseInterceptor = undefined
},
```
要完整实现你描述的行为,还应:
1. 在此插件的 `install(fx)` 中,捕获拦截器 API 返回的移除/清理函数并存到 `fx` 上,名称与上面保持一致。例如,如果你的 API 是 `fx.interceptors.request.use(handler)` 并返回一个移除函数,你可以:
- `fx.__cookieJarRequestInterceptor = fx.interceptors.request.use(requestHandler)`
- `fx.__cookieJarResponseInterceptor = fx.interceptors.response.use(responseHandler)`
2. 如果你的拦截器 API 使用不同的命名,或返回的是 ID 而不是函数,请在 `uninstall` 中相应地调整属性名和调用方式(例如调用 `fx.interceptors.request.eject(id)` 而不是直接调用函数)。
3. 更新 `fx` 的相关 TypeScript 类型定义,加入可选的 `__cookieJarRequestInterceptor` 和 `__cookieJarResponseInterceptor` 属性(或你选择的名称),以保证这些新属性在类型层面是安全的。
</issue_to_address>
### Comment 3
<location> `src/plugins/ws/index.ts:83-92` </location>
<code_context>
+async function waitForWsOpen(ws: WebSocket, delay: number) {
</code_context>
<issue_to_address>
**suggestion:** `waitForWsOpen` 没有处理在 `open` 之前套接字关闭的情况,这会让 Promise 一直挂起直到超时。
由于它目前只监听 `open` 和 `error`(外加超时),如果连接在 `open` 之前就被关闭,Promise 会一直保持未完成状态直到超时。建议同时监听 `close`,若在 `open` 之前触发就拒绝,并在 `cleanup` 中移除该监听器,从而实现「要么立刻连接成功,要么立刻失败」的行为。
建议实现:
```typescript
async function waitForWsOpen(ws: WebSocket, delay: number) {
// 如果已经是打开状态,立即 resolve
if ((ws as any).readyState === 1) return
await new Promise<void>((resolve, reject) => {
const cleanup = () => {
ws.removeEventListener("open", handleOpen)
ws.removeEventListener("error", handleError)
ws.removeEventListener("close", handleClose)
if (timer) clearTimeout(timer)
}
const handleOpen = () => {
cleanup()
resolve()
}
const handleError = (event: Event | any) => {
cleanup()
// 保留现有错误处理行为,但要保证总是 reject
reject(
event instanceof Error
? event
: new FexiosError(
"WebSocket connection failed",
"ECONNABORTED",
undefined,
undefined,
undefined
)
)
}
const handleClose = () => {
// 在 open 之前关闭 -> 立即失败,而不是等到超时
cleanup()
reject(
new FexiosError(
"WebSocket closed before connection was established",
"ECONNABORTED",
undefined,
undefined,
undefined
)
)
}
const timer =
delay > 0
? setTimeout(() => {
cleanup()
try {
ws.close()
} catch {
// 在超时路径中忽略 close 错误
}
reject(
new FexiosError(
"WebSocket connection timed out",
"ECONNABORTED",
undefined,
undefined,
undefined
)
)
}, delay)
: undefined
ws.addEventListener("open", handleOpen)
ws.addEventListener("error", handleError)
ws.addEventListener("close", handleClose)
})
}
```
上述 REPLACE 代码块假设:
- 本文件中已经导入了 `FexiosError`;
- 之前的实现只监听 `open` 和 `error`,并通过超时关闭套接字。
如果你现有的 `waitForWsOpen` 使用了不同的错误信息、状态码或不同的 `FexiosError` 构造方式,请让新的 `FexiosError` 调用与当前约定保持一致。同时确保移除旧的 `waitForWsOpen` 函数体,使得只保留新实现。
</issue_to_address>
### Comment 4
<location> `src/plugins/sse/index.ts:57-66` </location>
<code_context>
+async function waitForSseOpen(es: EventSource, delay: number) {
</code_context>
<issue_to_address>
**suggestion:** `waitForSseOpen` 没有考虑提前关闭的情况,这会让失败处理被推迟到超时。
目前它的行为和 WebSocket 插件类似:等待 `open` 或 `error`,或者超时。如果服务器在没有发出 `error` 的情况下就干净地关闭了 SSE 连接,Promise 会一直处于 pending 状态,直到超时。
如果运行环境暴露了类似 close 的信号(或将 close 映射为 `error`),建议显式处理这种情况,或者至少在那些干净关闭不会触发 `error` 的环境中进行说明/防护,以避免失败处理被延迟。
建议实现:
```typescript
async function waitForSseOpen(es: EventSource, delay: number) {
// readyState: 0 = CONNECTING, 1 = OPEN, 2 = CLOSED
if ((es as any).readyState === 1) return
await new Promise<void>((resolve, reject) => {
let settled = false
const handleOpen = () => {
if (settled) return
settled = true
cleanup()
resolve()
}
const handleError = (event: Event) => {
if (settled) return
settled = true
cleanup()
reject(event)
}
const handleClose = () => {
if (settled) return
settled = true
cleanup()
reject(
new Error('SSE connection was closed before it was fully opened')
)
}
const cleanup = () => {
clearTimeout(timer)
es.removeEventListener('open', handleOpen as any)
es.removeEventListener('error', handleError as any)
// 某些环境提供 EventSource 的独立 "close" 事件,
// 另一些环境会通过 "error" 暴露干净关闭。这里防御性地
// 尝试注销 "close" 监听器(若实现支持)。
try {
es.removeEventListener('close', handleClose as any)
} catch {
// 忽略:该实现不支持 "close" 事件
}
}
es.addEventListener('open', handleOpen as any)
es.addEventListener('error', handleError as any)
// 如果平台提供独立的 "close" 事件(或内部映射),
// 监听它以避免干净关闭拖到超时才被视为失败。
try {
es.addEventListener('close', handleClose as any)
} catch {
// 忽略:不支持 "close" 事件的环境应通过 "error" 暴露失败
}
const timer =
delay > 0
? setTimeout(() => {
if (settled) return
settled = true
cleanup()
try {
es.close()
} catch {
// 忽略;连接可能已经关闭
}
reject(new Error('opening SSE connection timed out'))
}, delay)
: (undefined as unknown as ReturnType<typeof setTimeout>)
})
}
```
由于原函数体只有一部分可见,请:
1. 删除此代码块下面任何残留的旧 `waitForSseOpen` 实现(如之前的 `es.onopen` / `es.onerror` / `setTimeout` 逻辑),以避免重复的处理逻辑和语法错误;
2. 如果代码库偏好使用 `onopen` / `onerror` 赋值而不是 `addEventListener`,可以将上述处理器适配为那种用法;关键是要处理某种形式的 close 信号(在可用时),并让超时/清理逻辑与现有约定保持一致;
3. 如果你想要更严格的类型,也可以把错误/关闭的拒绝类型抽出来,而不是直接使用 `Event` / `Error`,但行为应保持不变。
</issue_to_address>
### Comment 5
<location> `test/websocket.spec.ts:60-69` </location>
<code_context>
+ it('Honors timeout in plugin when socket never opens', async () => {
</code_context>
<issue_to_address>
**suggestion (testing):** 通过断言具体的错误码/错误信息,而不仅仅是错误类型,来增强超时测试的力度。
当前测试只验证了当 WebSocket 从未打开时会抛出 `FexiosError`。既然超时路径会使用 `FexiosErrorCodes.TIMEOUT`,请同时断言错误的数据结构(例如 `rejects.toMatchObject({ code: FexiosErrorCodes.TIMEOUT })`,并可选地断言 message),这样即使错误类型保持不变,也能捕获超时代码/路径上的回归。
建议实现:
```typescript
it('Honors timeout in plugin when socket never opens', async () => {
class HangingWebSocket {
public url: string
public readyState: number = 0 // CONNECTING
private listeners = new Map<string, Set<(event: any) => void>>()
constructor(url: string) {
this.url = url
}
addEventListener(type: string, listener: (event: any) => void) {
if (!this.listeners.has(type)) this.listeners.set(type, new Set())
this.listeners.get(type)!.add(listener)
```
```typescript
// 更严格的断言:明确验证走到了超时路径
await expect(connectWithPlugin()).rejects.toMatchObject({
code: FexiosErrorCodes.TIMEOUT,
})
```
1. 确保在 `test/websocket.spec.ts` 顶部导入了 `FexiosErrorCodes`,例如:
<<<<<<< SEARCH
import { FexiosError } from '../src/errors'
=======
import { FexiosError, FexiosErrorCodes } from '../src/errors'
>>>>>>> REPLACE
根据你的错误模块的实际路径/命名进行调整。
2. 如果实际测试使用的 Promise 名称不是 `connectWithPlugin`,或初始断言不同,请根据这个具体测试中已有的 `await expect(...).rejects...` 行来调整 SEARCH 块。
</issue_to_address>
### Comment 6
<location> `test/websocket.spec.ts:24-26` </location>
<code_context>
- })
- expect(data).to.be.instanceOf(MockWebSocket)
- data?.close()
+ it('Legacy usage should throw plugin guidance error', async () => {
+ const fx = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
+ await expect(
+ fx.get(WS_URL.replace(/^http/, 'ws') as any)
+ ).rejects.toMatchObject({ code: FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN })
</code_context>
<issue_to_address>
**suggestion (testing):** 为旧版 `responseType: "ws"` 用法抛出插件提示错误添加一个测试。
目前只覆盖了 `ws://` URL 的旧路径(`ctx.url.startsWith('ws')`)。请同时添加一个针对 `responseType: 'ws'` 的独立旧分支测试,例如 `fx.get('/path', { responseType: 'ws' as any })`,并断言其以 `{ code: FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN }` 拒绝,这样两个旧入口都能被覆盖。
</issue_to_address>
### Comment 7
<location> `test/sse.spec.ts:61-65` </location>
<code_context>
- it('Honors resolved timeout when EventSource never opens', async () => {
- const originalEventSource = (globalThis as any).EventSource
+ it('Honors timeout in plugin when EventSource never opens', async () => {
+ const originalEventSource2 = (globalThis as any).EventSource
class HangingEventSource {
</code_context>
<issue_to_address>
**suggestion (testing):** 通过检查超时错误码,而不仅仅是错误类,来收紧 SSE 超时测试。
由于插件在超时时会设置 `FexiosErrorCodes.TIMEOUT`,请断言该错误码(例如 `await expect(...).rejects.toMatchObject({ code: FexiosErrorCodes.TIMEOUT })`,也可以顺便检查 message),而不是只断言抛出了 `FexiosError`。这会更精准地验证超时行为。
建议实现:
```typescript
import { FexiosError, FexiosErrorCodes } from '../src/errors'
```
```typescript
it('Honors timeout in plugin when EventSource never opens', async () => {
const originalEventSource2 = (globalThis as any).EventSource
class HangingEventSource {
url: string
listeners = new Map<string, Set<(event: any) => void>>()
constructor(url: string) {
this.url = url
}
addEventListener(type: string, listener: (event: any) => void) {
if (!this.listeners.has(type)) {
this.listeners.set(type, new Set())
}
this.listeners.get(type)!.add(listener)
}
removeEventListener(type: string, listener: (event: any) => void) {
this.listeners.get(type)?.delete(listener)
}
close() {
/* noop: this EventSource never opens or emits anything */
}
}
;(globalThis as any).EventSource = HangingEventSource as any
try {
// 使用其它 SSE 测试中相同的方式启动基于 EventSource 的请求
const promise = fexios.get('/sse', {
// 插件级别的超时配置
sse: {
timeout: 3,
},
})
await expect(promise).rejects.toMatchObject({
code: FexiosErrorCodes.TIMEOUT,
})
} finally {
;(globalThis as any).EventSource = originalEventSource2
}
})
```
1. 根据错误定义的实际位置调整 `import { FexiosError, FexiosErrorCodes } from '../src/errors'` 的路径;
2. 将 `fexios.get('/sse', { sse: { timeout: 3 } })` 替换为当前 SSE 测试中实际用来发起 SSE 请求的辅助函数或 API(例如 `createSSE`、`client.sse` 等),以确保返回的是在超时时会被拒绝的那个 Promise;
3. 如果现有测试已经为 SSE 调用定义了 `promise` 变量且只断言 `FexiosError`,你也可以不替换整个测试体,而是简单地将:
`await expect(promise).rejects.toBeInstanceOf(FexiosError)` 改为:
`await expect(promise).rejects.toMatchObject({ code: FexiosErrorCodes.TIMEOUT })`。
</issue_to_address>
### Comment 8
<location> `test/sse.spec.ts:37-45` </location>
<code_context>
})
+ })
+
+ it('Server Sent Events via plugin (fx.sse)', async () => {
+ expect(pluginSSE).toBeDefined()
+ expect(typeof (pluginSSE as any).install).to.equal('function')
+ let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
+ fx = await fx.plugin(pluginSSE)
+ const sse = (await fx.sse(SSE_URL, {
+ query: { timeout: 3 },
+ })) as any as MockEventSource
let messages: any[] = []
await new Promise<void>((resolve) => {
sse.onmessage = (event) => {
</code_context>
<issue_to_address>
**suggestion (testing):** 建议增加一个测试来覆盖新的 SSE 生命周期事件(例如 `sse:beforeConnect`、`sse:open`、`sse:message`)。
目前只测试了 `fx.sse` 的正常路径,但没有覆盖插件的生命周期钩子(`sse:beforeConnect`、`sse:open`、`sse:message`、`sse:error`、`sse:close`)。请添加一个聚焦的测试,注册监听器(或通过 `fx.on(...)`)以确认 `sse:beforeConnect` 可以修改 URL/timeout,并且 `sse:open`/`sse:message` 会按预期触发。
建议实现:
```typescript
})
it('emits SSE lifecycle events via plugin hooks', async () => {
expect(pluginSSE).toBeDefined()
expect(typeof (pluginSSE as any).install).to.equal('function')
let beforeConnectCalled = false
let openCalled = false
const messagePayloads: any[] = []
let beforeConnectCtx: any
let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
fx = await fx.plugin(pluginSSE)
// 允许 beforeConnect 钩子在创建 EventSource 前修改 URL / 选项
fx.on('sse:beforeConnect', (ctx: any) => {
beforeConnectCalled = true
beforeConnectCtx = ctx
// 示例修改:提高 timeout,方便后面断言
// 如果插件使用不同的结构,请相应调整这些属性。
if (ctx.options?.query) {
ctx.options.query.timeout = 5
}
})
fx.on('sse:open', () => {
openCalled = true
})
fx.on('sse:message', (ctx: any) => {
// 预期 ctx.event 是 DOM Event / MessageEvent
messagePayloads.push(ctx.event?.data ?? ctx.data)
})
const sse = (await fx.sse(SSE_URL, {
query: { timeout: 1 },
})) as any as MockEventSource
// 等待若干条消息从 MockEventSource 中流出
await new Promise<void>((resolve) => {
let seen = 0
sse.onmessage = (event: any) => {
seen++
if (seen >= 3) {
resolve()
}
}
})
expect(beforeConnectCalled).to.equal(true)
expect(openCalled).to.equal(true)
expect(beforeConnectCtx).to.be.ok
expect(beforeConnectCtx.options?.query?.timeout).to.equal(5)
// 底层 EventSource 的 URL 或配置应反映 timeout 的修改。
// 如果 MockEventSource 以不同方式暴露 timeout,请调整下面的断言。
if (typeof sse.url === 'string') {
expect(sse.url).to.include('timeout=5')
}
expect(messagePayloads.length).to.be.greaterThan(0)
})
it('Server Sent Events via plugin (fx.sse)', async () => {
expect(pluginSSE).toBeDefined()
expect(typeof (pluginSSE as any).install).to.equal('function')
let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
fx = await fx.plugin(pluginSSE)
const sse = (await fx.sse(SSE_URL, {
query: { timeout: 3 },
})) as any as MockEventSource
let messages: any[] = []
await new Promise<void>((resolve) => {
sse.onmessage = (event) => {
expect(messages[2]).to.include('Message 3')
})
it('Honors timeout in plugin when EventSource never opens', async () => {
```
上面使用的钩子上下文对象形状(`ctx.url`、`ctx.options`、`ctx.event`、`ctx.data`)可能与实际的 `pluginSSE` 实现有所不同:
1. 请根据插件实际传递的内容调整 `sse:beforeConnect` 处理器的参数(例如 `{ url, init }` 或 `{ url, options }`),并相应修改 timeout 的变更方式;
2. 如果 `sse:message` 提供不同的负载(如 `{ event }`、`{ data }` 或 `{ event, parsedData }`),请调整 `messagePayloads.push(...)` 和相关断言;
3. 如果 `MockEventSource` 以不同的方式暴露 timeout 或配置(例如通过单独的 `options` 字段而不是编码进 `url`),请将当前检查 `sse.url` 的断言改为检查对应的属性;
4. 如果你的事件派发 API 与 `fx.on('event', handler)` 不同,请相应调整注册调用(`fx.on(...)`)。
</issue_to_address>
### Comment 9
<location> `test/raw-response-unread.spec.ts:33-30` </location>
<code_context>
})
- it('Text: ctx.response.rawResponse should remain unread for user-side handling', async () => {
+ it('Text: ctx.rawResponse should remain unread and equal ctx.response.rawResponse', async () => {
const fx = new Fexios({
fetch: async () => {
</code_context>
<issue_to_address>
**suggestion (testing):** 添加一个在解码 body 失败并回退到 text 的用例,以验证 `rawResponse` 在用户读取之前始终保持未读。
由于 `createFexiosResponse` 现在会在 JSON 解析失败时回退到 `.text()`,请同时添加一个 `content-type: application/json` 但 body 为非法 JSON 的用例。测试应断言:(1)`ctx.data` 为原始文本;(2)在用户访问前 `ctx.rawResponse` 仍保持未读;(3)只有在用户从 `rawResponse` 读取后,`bodyUsed` 才会变为 `true`。这会同时覆盖新的错误处理路径,并保持「rawResponse 在用户消费前保持未读」的不变式。
建议实现:
```typescript
// 用户仍然可以读取它(一次)
const userJson = await ctx.rawResponse.json()
expect(userJson).to.deep.equal({ ok: true, n: 1 })
expect(ctx.rawResponse.bodyUsed).to.equal(true)
expect(ctx.response.rawResponse.bodyUsed).to.equal(true)
})
it('JSON fallback: invalid JSON keeps rawResponse unread until user consumption', async () => {
let ctx: any
const fx = new Fexios({
fetch: async () =>
new Response('not valid json', {
status: 200,
headers: {
'content-type': 'application/json',
},
}),
})
fx.interceptors.response.use(async (_ctx) => {
ctx = _ctx
// createFexiosResponse 在 JSON 非法时应回退到 text
expect(ctx.data).to.equal('not valid json')
expect(ctx.response.responseType).to.equal('text')
// 暴露给用户的原始响应应保持未读
expect(ctx.rawResponse.bodyUsed).to.equal(false)
expect(ctx.response.rawResponse.bodyUsed).to.equal(false)
// 用户仍可读取(一次),并在此时 bodyUsed 才会变为 true
const userText = await ctx.rawResponse.text()
expect(userText).to.equal('not valid json')
expect(ctx.rawResponse.bodyUsed).to.equal(true)
expect(ctx.response.rawResponse.bodyUsed).to.equal(true)
return ctx
})
await fx.get('/invalid-json')
})
```
如果该文件尚未导入 `Fexios` 和 `Response`(来自你的测试环境中的 fetch 实现/ polyfill),请参考此文件中其他测试的方式,在 `test/raw-response-unread.spec.ts` 顶部补充相应的导入。
</issue_to_address>
### Comment 10
<location> `test/sse.spec.ts:24-32` </location>
<code_context>
- const { data: sse } = await fexios.get<MockEventSource>(SSE_URL, {
- query: { timeout: 3 },
- fetch: mockFetch,
+ it('Legacy usage should throw plugin guidance error (text/event-stream)', async () => {
+ const fx = new Fexios({
+ baseURL: MOCK_FETCH_BASE_URL,
+ fetch: async () =>
+ new Response(null, {
+ headers: { 'content-type': 'text/event-stream' },
+ }),
+ })
+ await expect(fx.get('/anything' as any)).rejects.toMatchObject({
+ code: FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN,
})
</code_context>
<issue_to_address>
**suggestion (testing):** 为配合新的核心保护逻辑,增加一个旧版 `responseType: "stream"` 用法的补充测试。
此测试覆盖了 `createFexiosResponse` 中新的 `text/event-stream` 检查分支,但还有一个针对 `expectedType === 'stream'` 时抛错的新分支。请添加一个测试,调用 `fx.get('/anything', { responseType: 'stream' as any })`,并断言 `FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN`,这样 SSE 的两个旧入口都能被覆盖,与 WebSocket 的旧用法测试保持一致。
</issue_to_address>
### Comment 11
<location> `CHANGELOG.md:12` </location>
<code_context>
- Request URLs starting with `ws://` or `wss://`
</code_context>
<issue_to_address>
**security (javascript.lang.security.detect-insecure-websocket):** 检测到不安全的 WebSocket。所有 WebSocket 连接都应该使用安全的 WebSocket(wss)。
*Source: opengrep*
</issue_to_address>
### Comment 12
<location> `src/plugins/ws/index.ts:55` </location>
<code_context>
if (s.startsWith('http://')) return s.replace(/^http:\/\//, 'ws://')
</code_context>
<issue_to_address>
**security (javascript.lang.security.detect-insecure-websocket):** 检测到不安全的 WebSocket。所有 WebSocket 连接都应该使用安全的 WebSocket(wss)。
*Source: opengrep*
</issue_to_address>
### Comment 13
<location> `src/plugins/ws/index.ts:67` </location>
<code_context>
input.startsWith('ws://') ||
</code_context>
<issue_to_address>
**security (javascript.lang.security.detect-insecure-websocket):** 检测到不安全的 WebSocket。所有 WebSocket 连接都应该使用安全的 WebSocket(wss)。
*Source: opengrep*
</issue_to_address>
### Comment 14
<location> `src/plugins/ws/index.ts:72` </location>
<code_context>
m === 'https://' ? 'wss://' : 'ws://'
</code_context>
<issue_to_address>
**security (javascript.lang.security.detect-insecure-websocket):** 检测到不安全的 WebSocket。所有 WebSocket 连接都应该使用安全的 WebSocket(wss)。
*Source: opengrep*
</issue_to_address>帮我变得更有用!请对每条评论点 👍 或 👎,我会根据你的反馈改进 review 质量。
Original comment in English
Hey - I've found 4 security issues, and 10 other issues
Security issues:
- Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections. (link)
- Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections. (link)
- Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections. (link)
- Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections. (link)
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `src/fexios.ts:566-567` </location>
<code_context>
}
}
- return this
+ return () => {
+ this.uninstall(plugin)
+ }
+ }
</code_context>
<issue_to_address>
**issue (bug_risk):** The `plugin()` return type is now inconsistent between array and single-plugin calls and may break existing chaining usage.
For array calls, `plugin([...])` still returns the `Fexios` instance, but for single-plugin calls it now returns an uninstall function. This breaks existing chains like `fx.plugin(pluginWebSocket).get(...)` for single plugins while still working for arrays.
If this change isn’t explicitly intended, consider either always returning the `Fexios` instance (and relying on the explicit `fx.uninstall()` API) or returning a backward-compatible structure like `{ fx, uninstall }`. Even if the breaking change is intentional, it’s worth making the array and single-plugin branches return the same shape so the API remains predictable.
</issue_to_address>
### Comment 2
<location> `src/plugins/cookie-jar/index.ts:74-75` </location>
<code_context>
+ install(fx) {
+ fx.hello = (name) => `Hello, ${name}`
+ },
+ uninstall(fx) {
+ fx.hello = undefined as any
+ },
</code_context>
<issue_to_address>
**suggestion (bug_risk):** The cookie-jar plugin `uninstall` only clears `cookieJar` but leaves interceptors installed, which may cause subtle behavior and memory leaks.
Right now `uninstall` only clears the plugin state (`fx.cookieJar`), but leaves the request/response interceptors registered. They still execute on every call and immediately return when `!fx.cookieJar`, which keeps unnecessary hooks around and can cause overhead and potential leaks.
If your interceptor API provides eject/cleanup functions, store them during `install` and invoke them from `uninstall` so the plugin fully detaches its interceptors when removed.
Suggested implementation:
```typescript
uninstall(fx) {
// Clear plugin state
fx.cookieJar = undefined
// Detach request/response interceptors if they were installed
if (fx.__cookieJarRequestInterceptor) {
fx.__cookieJarRequestInterceptor()
}
if (fx.__cookieJarResponseInterceptor) {
fx.__cookieJarResponseInterceptor()
}
// Clear references to interceptor cleanup functions
fx.__cookieJarRequestInterceptor = undefined
fx.__cookieJarResponseInterceptor = undefined
},
```
To fully implement the behavior you described, you should also:
1. In the `install(fx)` function of this plugin, capture the eject/cleanup functions returned by your interceptor API and store them on `fx`, matching the names used above. For example, if your API looks like `fx.interceptors.request.use(handler)` and returns an eject function, you would do something like:
- `fx.__cookieJarRequestInterceptor = fx.interceptors.request.use(requestHandler)`
- `fx.__cookieJarResponseInterceptor = fx.interceptors.response.use(responseHandler)`
2. If your interceptor API uses different names or returns an ID instead of a function, adapt the property names and invocation in `uninstall` accordingly (e.g., call `fx.interceptors.request.eject(id)` instead of invoking a function).
3. Update any relevant TypeScript typings for `fx` to include the optional `__cookieJarRequestInterceptor` and `__cookieJarResponseInterceptor` properties (or whatever names you choose) so the new properties are type-safe.
</issue_to_address>
### Comment 3
<location> `src/plugins/ws/index.ts:83-92` </location>
<code_context>
+async function waitForWsOpen(ws: WebSocket, delay: number) {
</code_context>
<issue_to_address>
**suggestion:** `waitForWsOpen` does not handle the socket closing before `open`, which can leave the promise hanging until timeout.
Because it only listens for `open` and `error` (plus the timeout), a connection that is closed before `open` will leave the promise unresolved until the timeout. Consider also listening for `close` and rejecting if it fires before `open`, and ensure that listener is removed in `cleanup` so we get immediate "connect or fail" behavior.
Suggested implementation:
```typescript
async function waitForWsOpen(ws: WebSocket, delay: number) {
// If already open, resolve immediately
if ((ws as any).readyState === 1) return
await new Promise<void>((resolve, reject) => {
const cleanup = () => {
ws.removeEventListener("open", handleOpen)
ws.removeEventListener("error", handleError)
ws.removeEventListener("close", handleClose)
if (timer) clearTimeout(timer)
}
const handleOpen = () => {
cleanup()
resolve()
}
const handleError = (event: Event | any) => {
cleanup()
// Preserve existing error handling behavior, but ensure we always reject
reject(
event instanceof Error
? event
: new FexiosError(
"WebSocket connection failed",
"ECONNABORTED",
undefined,
undefined,
undefined
)
)
}
const handleClose = () => {
// Closed before open -> immediate failure instead of waiting for timeout
cleanup()
reject(
new FexiosError(
"WebSocket closed before connection was established",
"ECONNABORTED",
undefined,
undefined,
undefined
)
)
}
const timer =
delay > 0
? setTimeout(() => {
cleanup()
try {
ws.close()
} catch {
// ignore close errors in timeout path
}
reject(
new FexiosError(
"WebSocket connection timed out",
"ECONNABORTED",
undefined,
undefined,
undefined
)
)
}, delay)
: undefined
ws.addEventListener("open", handleOpen)
ws.addEventListener("error", handleError)
ws.addEventListener("close", handleClose)
})
}
```
The REPLACE block above assumes:
- `FexiosError` is already imported in this file.
- The previous implementation only listened to `open` and `error` and used a timeout to close the socket.
If your existing `waitForWsOpen` used different error messages, status codes, or a different `FexiosError` constructor shape, align the new `FexiosError` calls with your existing conventions. Also ensure you remove the old body of `waitForWsOpen` so only the new implementation remains.
</issue_to_address>
### Comment 4
<location> `src/plugins/sse/index.ts:57-66` </location>
<code_context>
+async function waitForSseOpen(es: EventSource, delay: number) {
</code_context>
<issue_to_address>
**suggestion:** `waitForSseOpen` doesn’t consider premature closure, which can delay failure handling until timeout.
Right now it behaves like the WebSocket plugin: it waits for `open` or `error`, or times out. If the server cleanly closes the SSE connection without emitting `error`, the promise will stay pending until the timeout.
If the platform exposes a close-like signal (or maps close to `error`), consider handling that explicitly, or at least documenting/guarding against environments where a clean close does not emit `error` to avoid delayed failure handling.
Suggested implementation:
```typescript
async function waitForSseOpen(es: EventSource, delay: number) {
// readyState: 0 = CONNECTING, 1 = OPEN, 2 = CLOSED
if ((es as any).readyState === 1) return
await new Promise<void>((resolve, reject) => {
let settled = false
const handleOpen = () => {
if (settled) return
settled = true
cleanup()
resolve()
}
const handleError = (event: Event) => {
if (settled) return
settled = true
cleanup()
reject(event)
}
const handleClose = () => {
if (settled) return
settled = true
cleanup()
reject(
new Error('SSE connection was closed before it was fully opened')
)
}
const cleanup = () => {
clearTimeout(timer)
es.removeEventListener('open', handleOpen as any)
es.removeEventListener('error', handleError as any)
// Some environments expose a dedicated "close" event on EventSource,
// others surface a clean close via "error" instead. We defensively
// try to unregister the "close" listener when supported.
try {
es.removeEventListener('close', handleClose as any)
} catch {
// ignore: "close" not supported by this implementation
}
}
es.addEventListener('open', handleOpen as any)
es.addEventListener('error', handleError as any)
// If the platform exposes a dedicated "close" event (or maps it internally),
// listen for it so a clean close does not delay failure handling until timeout.
try {
es.addEventListener('close', handleClose as any)
} catch {
// ignore: environments without a "close" event are expected to surface failures via "error"
}
const timer =
delay > 0
? setTimeout(() => {
if (settled) return
settled = true
cleanup()
try {
es.close()
} catch {
// ignore; connection may already be closed
}
reject(new Error('opening SSE connection timed out'))
}, delay)
: (undefined as unknown as ReturnType<typeof setTimeout>)
})
}
```
Because only part of the original function body was visible, please:
1. Remove any remaining old `waitForSseOpen` implementation below this block (e.g. previous `es.onopen` / `es.onerror` / `setTimeout` logic) to avoid duplicate handlers and syntax errors.
2. If the codebase prefers `onopen` / `onerror` assignments over `addEventListener`, you can adapt the handlers to use those instead; the important part is that a `close`-like signal is handled (when available) and that timeout/cleanup logic matches your existing conventions.
3. If you want stricter typing, you can extract the error/close rejection types instead of using `Event` / `Error` directly, but behavior should remain the same.
</issue_to_address>
### Comment 5
<location> `test/websocket.spec.ts:60-69` </location>
<code_context>
+ it('Honors timeout in plugin when socket never opens', async () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Strengthen the timeout test by asserting the specific error code/message, not just the error type.
This test only verifies that a `FexiosError` is thrown when the WebSocket never opens. Since the timeout path uses `FexiosErrorCodes.TIMEOUT`, please also assert the error shape (e.g. `rejects.toMatchObject({ code: FexiosErrorCodes.TIMEOUT })`, and optionally the message) so regressions in the timeout code/path are caught even if the error type stays the same.
Suggested implementation:
```typescript
it('Honors timeout in plugin when socket never opens', async () => {
class HangingWebSocket {
public url: string
public readyState: number = 0 // CONNECTING
private listeners = new Map<string, Set<(event: any) => void>>()
constructor(url: string) {
this.url = url
}
addEventListener(type: string, listener: (event: any) => void) {
if (!this.listeners.has(type)) this.listeners.set(type, new Set())
this.listeners.get(type)!.add(listener)
```
```typescript
// Strengthened assertion: verify we hit the timeout path specifically
await expect(connectWithPlugin()).rejects.toMatchObject({
code: FexiosErrorCodes.TIMEOUT,
})
```
1. Ensure `FexiosErrorCodes` is imported at the top of `test/websocket.spec.ts`, e.g.:
<<<<<<< SEARCH
import { FexiosError } from '../src/errors'
=======
import { FexiosError, FexiosErrorCodes } from '../src/errors'
>>>>>>> REPLACE
Adjust the import path/names to match your existing error module.
2. If the actual test uses a different promise name than `connectWithPlugin` or a different initial assertion, update the SEARCH block to match the existing `await expect(...).rejects...` line in this specific test.
</issue_to_address>
### Comment 6
<location> `test/websocket.spec.ts:24-26` </location>
<code_context>
- })
- expect(data).to.be.instanceOf(MockWebSocket)
- data?.close()
+ it('Legacy usage should throw plugin guidance error', async () => {
+ const fx = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
+ await expect(
+ fx.get(WS_URL.replace(/^http/, 'ws') as any)
+ ).rejects.toMatchObject({ code: FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN })
</code_context>
<issue_to_address>
**suggestion (testing):** Add a test for legacy `responseType: "ws"` usage throwing the plugin guidance error.
This only exercises the `ws://` URL legacy path (`ctx.url.startsWith('ws')`). Please also add a test for the separate legacy branch where `responseType: 'ws'` is used, e.g. `fx.get('/path', { responseType: 'ws' as any })`, and assert it rejects with `{ code: FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN }` so both legacy entry points are covered.
</issue_to_address>
### Comment 7
<location> `test/sse.spec.ts:61-65` </location>
<code_context>
- it('Honors resolved timeout when EventSource never opens', async () => {
- const originalEventSource = (globalThis as any).EventSource
+ it('Honors timeout in plugin when EventSource never opens', async () => {
+ const originalEventSource2 = (globalThis as any).EventSource
class HangingEventSource {
</code_context>
<issue_to_address>
**suggestion (testing):** Tighten the SSE timeout test by checking the timeout error code, not only the error class.
Since the plugin sets `FexiosErrorCodes.TIMEOUT` on timeout, please assert the code (e.g. `await expect(...).rejects.toMatchObject({ code: FexiosErrorCodes.TIMEOUT })`, optionally also checking the message) instead of only asserting that a `FexiosError` is thrown. This will more accurately verify the timeout behavior.
Suggested implementation:
```typescript
import { FexiosError, FexiosErrorCodes } from '../src/errors'
```
```typescript
it('Honors timeout in plugin when EventSource never opens', async () => {
const originalEventSource2 = (globalThis as any).EventSource
class HangingEventSource {
url: string
listeners = new Map<string, Set<(event: any) => void>>()
constructor(url: string) {
this.url = url
}
addEventListener(type: string, listener: (event: any) => void) {
if (!this.listeners.has(type)) {
this.listeners.set(type, new Set())
}
this.listeners.get(type)!.add(listener)
}
removeEventListener(type: string, listener: (event: any) => void) {
this.listeners.get(type)?.delete(listener)
}
close() {
/* noop: this EventSource never opens or emits anything */
}
}
;(globalThis as any).EventSource = HangingEventSource as any
try {
// whatever call is used in other SSE tests to start an EventSource-backed request
const promise = fexios.get('/sse', {
// plugin-level timeout configuration
sse: {
timeout: 3,
},
})
await expect(promise).rejects.toMatchObject({
code: FexiosErrorCodes.TIMEOUT,
})
} finally {
;(globalThis as any).EventSource = originalEventSource2
}
})
```
1. Adjust the `import { FexiosError, FexiosErrorCodes } from '../src/errors'` path to match the actual location of your error definitions.
2. Replace `fexios.get('/sse', { sse: { timeout: 3 } })` with the actual helper or API used in the other SSE tests to initiate the SSE request (e.g., `createSSE`, `client.sse`, etc.), so that it returns the promise that rejects on timeout.
3. If the existing test already defines a `promise` variable for the SSE call and asserts only on `FexiosError`, instead of replacing the whole test body you can simply change:
`await expect(promise).rejects.toBeInstanceOf(FexiosError)` to:
`await expect(promise).rejects.toMatchObject({ code: FexiosErrorCodes.TIMEOUT })`.
</issue_to_address>
### Comment 8
<location> `test/sse.spec.ts:37-45` </location>
<code_context>
})
+ })
+
+ it('Server Sent Events via plugin (fx.sse)', async () => {
+ expect(pluginSSE).toBeDefined()
+ expect(typeof (pluginSSE as any).install).to.equal('function')
+ let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
+ fx = await fx.plugin(pluginSSE)
+ const sse = (await fx.sse(SSE_URL, {
+ query: { timeout: 3 },
+ })) as any as MockEventSource
let messages: any[] = []
await new Promise<void>((resolve) => {
sse.onmessage = (event) => {
</code_context>
<issue_to_address>
**suggestion (testing):** Consider adding a test that exercises the new SSE lifecycle events (e.g. `sse:beforeConnect`, `sse:open`, `sse:message`).
The happy path for `fx.sse` is tested, but the plugin lifecycle hooks (`sse:beforeConnect`, `sse:open`, `sse:message`, `sse:error`, `sse:close`) are not. Please add a focused test that attaches listeners (or uses `fx.on(...)`) to confirm that `sse:beforeConnect` can modify URL/timeout and that `sse:open`/`sse:message` are emitted as expected.
Suggested implementation:
```typescript
})
it('emits SSE lifecycle events via plugin hooks', async () => {
expect(pluginSSE).toBeDefined()
expect(typeof (pluginSSE as any).install).to.equal('function')
let beforeConnectCalled = false
let openCalled = false
const messagePayloads: any[] = []
let beforeConnectCtx: any
let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
fx = await fx.plugin(pluginSSE)
// Allow beforeConnect hook to modify URL / options before EventSource is created
fx.on('sse:beforeConnect', (ctx: any) => {
beforeConnectCalled = true
beforeConnectCtx = ctx
// Example mutation: bump timeout so we can assert it later
// Adapt these properties if your plugin uses a different shape.
if (ctx.options?.query) {
ctx.options.query.timeout = 5
}
})
fx.on('sse:open', () => {
openCalled = true
})
fx.on('sse:message', (ctx: any) => {
// ctx.event is expected to be the DOM Event / MessageEvent
messagePayloads.push(ctx.event?.data ?? ctx.data)
})
const sse = (await fx.sse(SSE_URL, {
query: { timeout: 1 },
})) as any as MockEventSource
// Wait for a few messages to flow through the mock EventSource
await new Promise<void>((resolve) => {
let seen = 0
sse.onmessage = (event: any) => {
seen++
if (seen >= 3) {
resolve()
}
}
})
expect(beforeConnectCalled).to.equal(true)
expect(openCalled).to.equal(true)
expect(beforeConnectCtx).to.be.ok
expect(beforeConnectCtx.options?.query?.timeout).to.equal(5)
// The underlying EventSource URL or configuration should reflect the timeout change.
// Adapt these expectations if your MockEventSource exposes timeout differently.
if (typeof sse.url === 'string') {
expect(sse.url).to.include('timeout=5')
}
expect(messagePayloads.length).to.be.greaterThan(0)
})
it('Server Sent Events via plugin (fx.sse)', async () => {
expect(pluginSSE).toBeDefined()
expect(typeof (pluginSSE as any).install).to.equal('function')
let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
fx = await fx.plugin(pluginSSE)
const sse = (await fx.sse(SSE_URL, {
query: { timeout: 3 },
})) as any as MockEventSource
let messages: any[] = []
await new Promise<void>((resolve) => {
sse.onmessage = (event) => {
expect(messages[2]).to.include('Message 3')
})
it('Honors timeout in plugin when EventSource never opens', async () => {
```
The exact shape of the hook context objects used above (`ctx.url`, `ctx.options`, `ctx.event`, `ctx.data`) may differ from your actual `pluginSSE` implementation:
1. Adjust the `sse:beforeConnect` handler parameters to match what the plugin actually passes (for example `{ url, init }` or `{ url, options }`), and update how the timeout is mutated accordingly.
2. If `sse:message` delivers a different payload (e.g. `{ event }`, `{ data }`, or `{ event, parsedData }`), adapt the `messagePayloads.push(...)` line and assertions.
3. If `MockEventSource` exposes timeout or configuration differently (for example via a separate `options` field instead of encoding it into `url`), update the assertion that currently checks `sse.url` to the appropriate property.
4. If your event emitter API differs from `fx.on('event', handler)`, update the registration calls (`fx.on(...)`) accordingly.
</issue_to_address>
### Comment 9
<location> `test/raw-response-unread.spec.ts:33-30` </location>
<code_context>
})
- it('Text: ctx.response.rawResponse should remain unread for user-side handling', async () => {
+ it('Text: ctx.rawResponse should remain unread and equal ctx.response.rawResponse', async () => {
const fx = new Fexios({
fetch: async () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Add a case where body decoding fails and falls back to text, to verify `rawResponse` remains unread until user consumption.
Since `createFexiosResponse` now falls back to `.text()` when JSON parsing fails, please also add a case with `content-type: application/json` but invalid JSON. The test should assert that (1) `ctx.data` is the raw text, (2) `ctx.rawResponse` remains unread prior to user access, and (3) `bodyUsed` only becomes `true` once the user reads from `rawResponse`. That will exercise the new error-handling path while preserving the “unread rawResponse” invariant.
Suggested implementation:
```typescript
// user can still read it (once)
const userJson = await ctx.rawResponse.json()
expect(userJson).to.deep.equal({ ok: true, n: 1 })
expect(ctx.rawResponse.bodyUsed).to.equal(true)
expect(ctx.response.rawResponse.bodyUsed).to.equal(true)
})
it('JSON fallback: invalid JSON keeps rawResponse unread until user consumption', async () => {
let ctx: any
const fx = new Fexios({
fetch: async () =>
new Response('not valid json', {
status: 200,
headers: {
'content-type': 'application/json',
},
}),
})
fx.interceptors.response.use(async (_ctx) => {
ctx = _ctx
// createFexiosResponse should fall back to text on invalid JSON
expect(ctx.data).to.equal('not valid json')
expect(ctx.response.responseType).to.equal('text')
// raw response exposed to user should remain unread
expect(ctx.rawResponse.bodyUsed).to.equal(false)
expect(ctx.response.rawResponse.bodyUsed).to.equal(false)
// user can still read it (once) and that is when bodyUsed flips
const userText = await ctx.rawResponse.text()
expect(userText).to.equal('not valid json')
expect(ctx.rawResponse.bodyUsed).to.equal(true)
expect(ctx.response.rawResponse.bodyUsed).to.equal(true)
return ctx
})
await fx.get('/invalid-json')
})
```
If this file does not already import `Fexios` and `Response` (from the appropriate fetch implementation / polyfill in your test setup), ensure they are imported at the top of `test/raw-response-unread.spec.ts` in the same way as the other tests in this file.
</issue_to_address>
### Comment 10
<location> `test/sse.spec.ts:24-32` </location>
<code_context>
- const { data: sse } = await fexios.get<MockEventSource>(SSE_URL, {
- query: { timeout: 3 },
- fetch: mockFetch,
+ it('Legacy usage should throw plugin guidance error (text/event-stream)', async () => {
+ const fx = new Fexios({
+ baseURL: MOCK_FETCH_BASE_URL,
+ fetch: async () =>
+ new Response(null, {
+ headers: { 'content-type': 'text/event-stream' },
+ }),
+ })
+ await expect(fx.get('/anything' as any)).rejects.toMatchObject({
+ code: FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN,
})
</code_context>
<issue_to_address>
**suggestion (testing):** Add a complementary test for legacy `responseType: "stream"` usage to match the new core guard.
This test covers the new `text/event-stream` guard in `createFexiosResponse`, but there’s also a new branch that throws when `expectedType === 'stream'`. Please add a test that calls `fx.get('/anything', { responseType: 'stream' as any })` and asserts `FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN`, mirroring the WebSocket legacy tests so both SSE entry points are covered.
</issue_to_address>
### Comment 11
<location> `CHANGELOG.md:12` </location>
<code_context>
- Request URLs starting with `ws://` or `wss://`
</code_context>
<issue_to_address>
**security (javascript.lang.security.detect-insecure-websocket):** Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections.
*Source: opengrep*
</issue_to_address>
### Comment 12
<location> `src/plugins/ws/index.ts:55` </location>
<code_context>
if (s.startsWith('http://')) return s.replace(/^http:\/\//, 'ws://')
</code_context>
<issue_to_address>
**security (javascript.lang.security.detect-insecure-websocket):** Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections.
*Source: opengrep*
</issue_to_address>
### Comment 13
<location> `src/plugins/ws/index.ts:67` </location>
<code_context>
input.startsWith('ws://') ||
</code_context>
<issue_to_address>
**security (javascript.lang.security.detect-insecure-websocket):** Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections.
*Source: opengrep*
</issue_to_address>
### Comment 14
<location> `src/plugins/ws/index.ts:72` </location>
<code_context>
m === 'https://' ? 'wss://' : 'ws://'
</code_context>
<issue_to_address>
**security (javascript.lang.security.detect-insecure-websocket):** Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections.
*Source: opengrep*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| async function waitForWsOpen(ws: WebSocket, delay: number) { | ||
| if ((ws as any).readyState === 1) return | ||
| await new Promise<void>((resolve, reject) => { | ||
| const timer = | ||
| delay > 0 | ||
| ? setTimeout(() => { | ||
| try { | ||
| ws.close() | ||
| } catch {} | ||
| reject( |
There was a problem hiding this comment.
suggestion: waitForWsOpen 没有处理在 open 之前套接字关闭的情况,这会让 Promise 一直挂起直到超时。
由于它目前只监听 open 和 error(外加超时),如果连接在 open 之前就被关闭,Promise 会一直保持未完成状态直到超时。建议同时监听 close,若在 open 之前触发就拒绝,并在 cleanup 中移除该监听器,从而实现「要么立刻连接成功,要么立刻失败」的行为。
建议实现:
async function waitForWsOpen(ws: WebSocket, delay: number) {
// 如果已经是打开状态,立即 resolve
if ((ws as any).readyState === 1) return
await new Promise<void>((resolve, reject) => {
const cleanup = () => {
ws.removeEventListener("open", handleOpen)
ws.removeEventListener("error", handleError)
ws.removeEventListener("close", handleClose)
if (timer) clearTimeout(timer)
}
const handleOpen = () => {
cleanup()
resolve()
}
const handleError = (event: Event | any) => {
cleanup()
// 保留现有错误处理行为,但要保证总是 reject
reject(
event instanceof Error
? event
: new FexiosError(
"WebSocket connection failed",
"ECONNABORTED",
undefined,
undefined,
undefined
)
)
}
const handleClose = () => {
// 在 open 之前关闭 -> 立即失败,而不是等到超时
cleanup()
reject(
new FexiosError(
"WebSocket closed before connection was established",
"ECONNABORTED",
undefined,
undefined,
undefined
)
)
}
const timer =
delay > 0
? setTimeout(() => {
cleanup()
try {
ws.close()
} catch {
// 在超时路径中忽略 close 错误
}
reject(
new FexiosError(
"WebSocket connection timed out",
"ECONNABORTED",
undefined,
undefined,
undefined
)
)
}, delay)
: undefined
ws.addEventListener("open", handleOpen)
ws.addEventListener("error", handleError)
ws.addEventListener("close", handleClose)
})
}上述 REPLACE 代码块假设:
- 本文件中已经导入了
FexiosError; - 之前的实现只监听
open和error,并通过超时关闭套接字。
如果你现有的 waitForWsOpen 使用了不同的错误信息、状态码或不同的 FexiosError 构造方式,请让新的 FexiosError 调用与当前约定保持一致。同时确保移除旧的 waitForWsOpen 函数体,使得只保留新实现。
Original comment in English
suggestion: waitForWsOpen does not handle the socket closing before open, which can leave the promise hanging until timeout.
Because it only listens for open and error (plus the timeout), a connection that is closed before open will leave the promise unresolved until the timeout. Consider also listening for close and rejecting if it fires before open, and ensure that listener is removed in cleanup so we get immediate "connect or fail" behavior.
Suggested implementation:
async function waitForWsOpen(ws: WebSocket, delay: number) {
// If already open, resolve immediately
if ((ws as any).readyState === 1) return
await new Promise<void>((resolve, reject) => {
const cleanup = () => {
ws.removeEventListener("open", handleOpen)
ws.removeEventListener("error", handleError)
ws.removeEventListener("close", handleClose)
if (timer) clearTimeout(timer)
}
const handleOpen = () => {
cleanup()
resolve()
}
const handleError = (event: Event | any) => {
cleanup()
// Preserve existing error handling behavior, but ensure we always reject
reject(
event instanceof Error
? event
: new FexiosError(
"WebSocket connection failed",
"ECONNABORTED",
undefined,
undefined,
undefined
)
)
}
const handleClose = () => {
// Closed before open -> immediate failure instead of waiting for timeout
cleanup()
reject(
new FexiosError(
"WebSocket closed before connection was established",
"ECONNABORTED",
undefined,
undefined,
undefined
)
)
}
const timer =
delay > 0
? setTimeout(() => {
cleanup()
try {
ws.close()
} catch {
// ignore close errors in timeout path
}
reject(
new FexiosError(
"WebSocket connection timed out",
"ECONNABORTED",
undefined,
undefined,
undefined
)
)
}, delay)
: undefined
ws.addEventListener("open", handleOpen)
ws.addEventListener("error", handleError)
ws.addEventListener("close", handleClose)
})
}The REPLACE block above assumes:
FexiosErroris already imported in this file.- The previous implementation only listened to
openanderrorand used a timeout to close the socket.
If your existing waitForWsOpen used different error messages, status codes, or a different FexiosError constructor shape, align the new FexiosError calls with your existing conventions. Also ensure you remove the old body of waitForWsOpen so only the new implementation remains.
| it('Honors timeout in plugin when socket never opens', async () => { | ||
| class HangingWebSocket { | ||
| public url: string | ||
| public readyState: number = 0 // CONNECTING | ||
| private listeners = new Map<string, Set<(event: any) => void>>() | ||
| constructor(url: string) { | ||
| this.url = url | ||
| } | ||
| addEventListener(type: string, listener: (event: any) => void) { | ||
| if (!this.listeners.has(type)) this.listeners.set(type, new Set()) |
There was a problem hiding this comment.
suggestion (testing): 通过断言具体的错误码/错误信息,而不仅仅是错误类型,来增强超时测试的力度。
当前测试只验证了当 WebSocket 从未打开时会抛出 FexiosError。既然超时路径会使用 FexiosErrorCodes.TIMEOUT,请同时断言错误的数据结构(例如 rejects.toMatchObject({ code: FexiosErrorCodes.TIMEOUT }),并可选地断言 message),这样即使错误类型保持不变,也能捕获超时代码/路径上的回归。
建议实现:
it('Honors timeout in plugin when socket never opens', async () => {
class HangingWebSocket {
public url: string
public readyState: number = 0 // CONNECTING
private listeners = new Map<string, Set<(event: any) => void>>()
constructor(url: string) {
this.url = url
}
addEventListener(type: string, listener: (event: any) => void) {
if (!this.listeners.has(type)) this.listeners.set(type, new Set())
this.listeners.get(type)!.add(listener) // 更严格的断言:明确验证走到了超时路径
await expect(connectWithPlugin()).rejects.toMatchObject({
code: FexiosErrorCodes.TIMEOUT,
})-
确保在
test/websocket.spec.ts顶部导入了FexiosErrorCodes,例如:<<<<<<< SEARCH
import { FexiosError } from '../src/errors'import { FexiosError, FexiosErrorCodes } from '../src/errors'
REPLACE
根据你的错误模块的实际路径/命名进行调整。
-
如果实际测试使用的 Promise 名称不是
connectWithPlugin,或初始断言不同,请根据这个具体测试中已有的await expect(...).rejects...行来调整 SEARCH 块。
Original comment in English
suggestion (testing): Strengthen the timeout test by asserting the specific error code/message, not just the error type.
This test only verifies that a FexiosError is thrown when the WebSocket never opens. Since the timeout path uses FexiosErrorCodes.TIMEOUT, please also assert the error shape (e.g. rejects.toMatchObject({ code: FexiosErrorCodes.TIMEOUT }), and optionally the message) so regressions in the timeout code/path are caught even if the error type stays the same.
Suggested implementation:
it('Honors timeout in plugin when socket never opens', async () => {
class HangingWebSocket {
public url: string
public readyState: number = 0 // CONNECTING
private listeners = new Map<string, Set<(event: any) => void>>()
constructor(url: string) {
this.url = url
}
addEventListener(type: string, listener: (event: any) => void) {
if (!this.listeners.has(type)) this.listeners.set(type, new Set())
this.listeners.get(type)!.add(listener) // Strengthened assertion: verify we hit the timeout path specifically
await expect(connectWithPlugin()).rejects.toMatchObject({
code: FexiosErrorCodes.TIMEOUT,
})-
Ensure
FexiosErrorCodesis imported at the top oftest/websocket.spec.ts, e.g.:<<<<<<< SEARCH
import { FexiosError } from '../src/errors'import { FexiosError, FexiosErrorCodes } from '../src/errors'
REPLACE
Adjust the import path/names to match your existing error module.
-
If the actual test uses a different promise name than
connectWithPluginor a different initial assertion, update the SEARCH block to match the existingawait expect(...).rejects...line in this specific test.
| it('Server Sent Events via plugin (fx.sse)', async () => { | ||
| expect(pluginSSE).toBeDefined() | ||
| expect(typeof (pluginSSE as any).install).to.equal('function') | ||
| let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL }) | ||
| fx = await fx.plugin(pluginSSE) | ||
| const sse = (await fx.sse(SSE_URL, { | ||
| query: { timeout: 3 }, | ||
| })) as any as MockEventSource | ||
| let messages: any[] = [] |
There was a problem hiding this comment.
suggestion (testing): 建议增加一个测试来覆盖新的 SSE 生命周期事件(例如 sse:beforeConnect、sse:open、sse:message)。
目前只测试了 fx.sse 的正常路径,但没有覆盖插件的生命周期钩子(sse:beforeConnect、sse:open、sse:message、sse:error、sse:close)。请添加一个聚焦的测试,注册监听器(或通过 fx.on(...))以确认 sse:beforeConnect 可以修改 URL/timeout,并且 sse:open/sse:message 会按预期触发。
建议实现:
})
it('emits SSE lifecycle events via plugin hooks', async () => {
expect(pluginSSE).toBeDefined()
expect(typeof (pluginSSE as any).install).to.equal('function')
let beforeConnectCalled = false
let openCalled = false
const messagePayloads: any[] = []
let beforeConnectCtx: any
let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
fx = await fx.plugin(pluginSSE)
// 允许 beforeConnect 钩子在创建 EventSource 前修改 URL / 选项
fx.on('sse:beforeConnect', (ctx: any) => {
beforeConnectCalled = true
beforeConnectCtx = ctx
// 示例修改:提高 timeout,方便后面断言
// 如果插件使用不同的结构,请相应调整这些属性。
if (ctx.options?.query) {
ctx.options.query.timeout = 5
}
})
fx.on('sse:open', () => {
openCalled = true
})
fx.on('sse:message', (ctx: any) => {
// 预期 ctx.event 是 DOM Event / MessageEvent
messagePayloads.push(ctx.event?.data ?? ctx.data)
})
const sse = (await fx.sse(SSE_URL, {
query: { timeout: 1 },
})) as any as MockEventSource
// 等待若干条消息从 MockEventSource 中流出
await new Promise<void>((resolve) => {
let seen = 0
sse.onmessage = (event: any) => {
seen++
if (seen >= 3) {
resolve()
}
}
})
expect(beforeConnectCalled).to.equal(true)
expect(openCalled).to.equal(true)
expect(beforeConnectCtx).to.be.ok
expect(beforeConnectCtx.options?.query?.timeout).to.equal(5)
// 底层 EventSource 的 URL 或配置应反映 timeout 的修改。
// 如果 MockEventSource 以不同方式暴露 timeout,请调整下面的断言。
if (typeof sse.url === 'string') {
expect(sse.url).to.include('timeout=5')
}
expect(messagePayloads.length).to.be.greaterThan(0)
})
it('Server Sent Events via plugin (fx.sse)', async () => {
expect(pluginSSE).toBeDefined()
expect(typeof (pluginSSE as any).install).to.equal('function')
let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
fx = await fx.plugin(pluginSSE)
const sse = (await fx.sse(SSE_URL, {
query: { timeout: 3 },
})) as any as MockEventSource
let messages: any[] = []
await new Promise<void>((resolve) => {
sse.onmessage = (event) => {
expect(messages[2]).to.include('Message 3')
})
it('Honors timeout in plugin when EventSource never opens', async () => {上面使用的钩子上下文对象形状(ctx.url、ctx.options、ctx.event、ctx.data)可能与实际的 pluginSSE 实现有所不同:
- 请根据插件实际传递的内容调整
sse:beforeConnect处理器的参数(例如{ url, init }或{ url, options }),并相应修改 timeout 的变更方式; - 如果
sse:message提供不同的负载(如{ event }、{ data }或{ event, parsedData }),请调整messagePayloads.push(...)和相关断言; - 如果
MockEventSource以不同的方式暴露 timeout 或配置(例如通过单独的options字段而不是编码进url),请将当前检查sse.url的断言改为检查对应的属性; - 如果你的事件派发 API 与
fx.on('event', handler)不同,请相应调整注册调用(fx.on(...))。
Original comment in English
suggestion (testing): Consider adding a test that exercises the new SSE lifecycle events (e.g. sse:beforeConnect, sse:open, sse:message).
The happy path for fx.sse is tested, but the plugin lifecycle hooks (sse:beforeConnect, sse:open, sse:message, sse:error, sse:close) are not. Please add a focused test that attaches listeners (or uses fx.on(...)) to confirm that sse:beforeConnect can modify URL/timeout and that sse:open/sse:message are emitted as expected.
Suggested implementation:
})
it('emits SSE lifecycle events via plugin hooks', async () => {
expect(pluginSSE).toBeDefined()
expect(typeof (pluginSSE as any).install).to.equal('function')
let beforeConnectCalled = false
let openCalled = false
const messagePayloads: any[] = []
let beforeConnectCtx: any
let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
fx = await fx.plugin(pluginSSE)
// Allow beforeConnect hook to modify URL / options before EventSource is created
fx.on('sse:beforeConnect', (ctx: any) => {
beforeConnectCalled = true
beforeConnectCtx = ctx
// Example mutation: bump timeout so we can assert it later
// Adapt these properties if your plugin uses a different shape.
if (ctx.options?.query) {
ctx.options.query.timeout = 5
}
})
fx.on('sse:open', () => {
openCalled = true
})
fx.on('sse:message', (ctx: any) => {
// ctx.event is expected to be the DOM Event / MessageEvent
messagePayloads.push(ctx.event?.data ?? ctx.data)
})
const sse = (await fx.sse(SSE_URL, {
query: { timeout: 1 },
})) as any as MockEventSource
// Wait for a few messages to flow through the mock EventSource
await new Promise<void>((resolve) => {
let seen = 0
sse.onmessage = (event: any) => {
seen++
if (seen >= 3) {
resolve()
}
}
})
expect(beforeConnectCalled).to.equal(true)
expect(openCalled).to.equal(true)
expect(beforeConnectCtx).to.be.ok
expect(beforeConnectCtx.options?.query?.timeout).to.equal(5)
// The underlying EventSource URL or configuration should reflect the timeout change.
// Adapt these expectations if your MockEventSource exposes timeout differently.
if (typeof sse.url === 'string') {
expect(sse.url).to.include('timeout=5')
}
expect(messagePayloads.length).to.be.greaterThan(0)
})
it('Server Sent Events via plugin (fx.sse)', async () => {
expect(pluginSSE).toBeDefined()
expect(typeof (pluginSSE as any).install).to.equal('function')
let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
fx = await fx.plugin(pluginSSE)
const sse = (await fx.sse(SSE_URL, {
query: { timeout: 3 },
})) as any as MockEventSource
let messages: any[] = []
await new Promise<void>((resolve) => {
sse.onmessage = (event) => {
expect(messages[2]).to.include('Message 3')
})
it('Honors timeout in plugin when EventSource never opens', async () => {The exact shape of the hook context objects used above (ctx.url, ctx.options, ctx.event, ctx.data) may differ from your actual pluginSSE implementation:
- Adjust the
sse:beforeConnecthandler parameters to match what the plugin actually passes (for example{ url, init }or{ url, options }), and update how the timeout is mutated accordingly. - If
sse:messagedelivers a different payload (e.g.{ event },{ data }, or{ event, parsedData }), adapt themessagePayloads.push(...)line and assertions. - If
MockEventSourceexposes timeout or configuration differently (for example via a separateoptionsfield instead of encoding it intourl), update the assertion that currently checkssse.urlto the appropriate property. - If your event emitter API differs from
fx.on('event', handler), update the registration calls (fx.on(...)) accordingly.
…ype safety in cookie-jar plugin
…t and introduce legacy aliases for backward compatibility
|
@sourcery-ai dismiss |
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - I've found 4 security issues, 8 other issues, and left some high level feedback:
Security issues:
- Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections. (link)
- Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections. (link)
- Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections. (link)
- Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections. (link)
General comments:
- 在
request()中你调用了this.emit('afterRawResponse', ctx)却没有使用await,这意味着afterRawResponse钩子无法中断流程,也无法可靠地看到已完全更新的上下文;建议对这次 emit 使用await,并像其他生命周期事件一样处理 FINAL_SYMBOL。 - CHANGELOG 中描述
fx.plugin(plugin)会返回一个卸载 helper,但当前的plugin()实现始终返回this/Fexios;建议将实现(以及它的返回类型)与文档中描述的卸载 helper 行为对齐,或者更新文档以匹配实际 API。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `request()` you call `this.emit('afterRawResponse', ctx)` without `await`, which means `afterRawResponse` hooks can’t short-circuit or reliably see a fully-updated context; consider awaiting this emit and handling the FINAL_SYMBOL the same way as the other lifecycle events.
- The CHANGELOG describes `fx.plugin(plugin)` returning an uninstall helper, but the current `plugin()` implementation always returns `this`/`Fexios`; consider aligning the implementation (and its return type) with the documented uninstall helper behavior or updating the docs to match the actual API.
## Individual Comments
### Comment 1
<location> `src/fexios.ts:407-408` </location>
<code_context>
if (timer) clearTimeout(timer)
ctx.rawResponse = rawResponse
+ this.emit('afterRawResponse', ctx)
+
ctx.response = await createFexiosResponse(
</code_context>
<issue_to_address>
**issue (bug_risk):** `afterRawResponse` 钩子没有被 `await`,因此该事件上的异步处理函数以及通过该事件进行的短路控制都不会按预期工作。
其他生命周期事件(`beforeInit`、`afterBodyTransformed`、`beforeActualFetch`、`afterResponse`)都是被 `await` 的,并且可以通过 `FINAL_SYMBOL` 来短路,但 `afterRawResponse` 是“发射后不管”的方式,其返回值被忽略。
因此,异步的 `afterRawResponse` 钩子无法影响后续控制流,也无法修改后续逻辑所看到的上下文/响应。
为了对齐行为并让该钩子真正有用,应该对它进行 `await` 并支持短路,例如:
```ts
ctx.rawResponse = rawResponse
ctx = await this.emit('afterRawResponse', ctx)
if ((ctx as any)[Fexios.FINAL_SYMBOL]) return ctx as any
```
(如果你希望在这里支持短路,可以使用等价的处理方式)。
</issue_to_address>
### Comment 2
<location> `src/fexios.ts:385-387` </location>
<code_context>
- const rawResponse = await fetch(ctx.rawRequest!).catch((err) => {
- if (timer) clearTimeout(timer)
- if (abortController?.signal.aborted) {
+ const fetch =
+ (ctx.request as any).fetch || this.baseConfigs.fetch || globalThis.fetch
+ const rawResponse = await fetch(ctx.request.rawRequest!).catch(
+ (err: any) => {
+ if (timer) clearTimeout(timer)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** 重新抛出 fetch 错误时会丢失非 `Error` 类型拒绝值的细节,并且可能在不存在 `err.message` 时依赖它。
`.catch((err: any) => { ... err.message ... })` 这条分支假设 `err` 类似 `Error`。某些 fetch 实现或 shim 可能会用字符串或其他值进行 reject,这种情况下 `err.message` 可能为 `undefined`,从而产生信息很少的 `FexiosError`。
建议像 SSE 插件那样对错误消息进行归一化处理,例如:
```ts
const msg =
err instanceof Error ? err.message :
typeof err === 'string' ? err :
'Network request failed'
throw new FexiosError(FexiosErrorCodes.NETWORK_ERROR, msg, ctx)
```
这样可以在不同环境中保留更有用的错误信息。
</issue_to_address>
### Comment 3
<location> `src/plugins/ws/index.ts:83-92` </location>
<code_context>
+ ).toString()
+}
+
+async function waitForWsOpen(ws: WebSocket, delay: number) {
+ if ((ws as any).readyState === 1) return
+ await new Promise<void>((resolve, reject) => {
+ const timer =
+ delay > 0
+ ? setTimeout(() => {
+ try {
+ ws.close()
+ } catch {}
+ reject(
+ new FexiosError(
+ FexiosErrorCodes.TIMEOUT,
+ `WebSocket connection timed out after ${delay}ms`
+ )
+ )
+ }, delay)
+ : undefined
+
+ const cleanup = () => {
+ if (timer) clearTimeout(timer)
+ ws.removeEventListener('open', onOpen)
+ ws.removeEventListener('error', onError)
+ }
+
</code_context>
<issue_to_address>
**suggestion:** WebSocket 等待打开的逻辑没有对 `open` 之前的 `close` 作出响应,会导致超时时间比必要的更长。
`waitForWsOpen` 只监听了 `open` 和 `error`。如果 socket 在没有触发 `error` 的情况下进入 `CLOSING`/`CLOSED` 状态,该 promise 会一直等到超时才 reject,尽管我们已经知道连接不会成功。
为了快速失败并与 SSE 插件行为保持一致,建议同时监听 `close` 并立刻 reject,例如:
```ts
const onClose = (event: CloseEvent) => {
cleanup()
reject(
new FexiosError(
FexiosErrorCodes.NETWORK_ERROR,
`WebSocket connection closed before open (code: ${event.code}, reason: ${event.reason})`,
undefined,
{ cause: event }
)
)
}
ws.addEventListener('close', onClose)
// and remove it in cleanup()
```
这样可以在服务器提前关闭连接时避免不必要的等待。
建议实现:
```typescript
const onClose = (event: CloseEvent) => {
cleanup()
reject(
new FexiosError(
FexiosErrorCodes.NETWORK_ERROR,
`WebSocket connection closed before open (code: ${event.code}, reason: ${event.reason})`,
undefined,
{ cause: event }
)
)
}
const cleanup = () => {
if (timer) clearTimeout(timer)
ws.removeEventListener('open', onOpen)
ws.removeEventListener('error', onError)
ws.removeEventListener('close', onClose)
}
```
```typescript
const timer =
delay > 0
? setTimeout(() => {
try {
ws.close()
} catch {}
reject(
new FexiosError(
FexiosErrorCodes.TIMEOUT,
`WebSocket connection timed out after ${delay}ms`
)
)
}, delay)
: undefined
```
```typescript
const onClose = (event: CloseEvent) => {
cleanup()
reject(
new FexiosError(
FexiosErrorCodes.NETWORK_ERROR,
`WebSocket connection closed before open (code: ${event.code}, reason: ${event.reason})`,
undefined,
{ cause: event }
)
)
}
const cleanup = () => {
if (timer) clearTimeout(timer)
ws.removeEventListener('open', onOpen)
ws.removeEventListener('error', onError)
ws.removeEventListener('close', onClose)
}
```
```typescript
ws.addEventListener('open', onOpen)
ws.addEventListener('error', onError)
ws.addEventListener('close', onClose)
```
</issue_to_address>
### Comment 4
<location> `test/websocket.spec.ts:24-26` </location>
<code_context>
- })
- expect(data).to.be.instanceOf(MockWebSocket)
- data?.close()
+ it('Legacy usage should throw plugin guidance error', async () => {
+ const fx = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
+ await expect(
+ fx.get(WS_URL.replace(/^http/, 'ws') as any)
+ ).rejects.toMatchObject({ code: FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN })
</code_context>
<issue_to_address>
**suggestion (testing):** 为应当现在抛出 `FEATURE_MOVED_TO_PLUGIN` 的旧版 `responseType: 'ws'` 和 upgrade-header 路径增加测试覆盖。
当前测试覆盖了旧版 `ws://` URL 的场景。为了完整验证新行为,请再添加以下测试:
1. 调用 `fx.get('/ws-endpoint', { responseType: 'ws' as any })`,并断言拒绝错误的 `code === FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN`。
2. 使用一个 mock 的 `fetch`,返回带有 `upgrade: websocket` 和 `connection: upgrade` 头的 `Response`,并断言 `createFexiosResponse`(或 `fx.get`)会以同样的错误码进行 reject。
这样可以确认所有旧版 WebSocket 入口现在都会一致地引导用户使用基于插件的 API。
建议实现:
```typescript
}
})
it('Legacy usage should throw plugin guidance error', async () => {
const fx = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
await expect(
fx.get(WS_URL.replace(/^http/, 'ws') as any)
).rejects.toMatchObject({ code: FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN })
})
it('Legacy responseType "ws" should throw plugin guidance error', async () => {
const fx = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
await expect(
fx.get('/ws-endpoint', { responseType: 'ws' as any })
).rejects.toMatchObject({ code: FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN })
})
it('Legacy upgrade-header path should throw plugin guidance error', async () => {
const fx = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
const originalFetch = (global as any).fetch
const mockFetch = jest.fn().mockResolvedValue(
new Response(null, {
status: 101,
headers: {
upgrade: 'websocket',
connection: 'upgrade',
},
})
)
;(global as any).fetch = mockFetch
try {
await expect(fx.get('/ws-upgrade-endpoint')).rejects.toMatchObject({
code: FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN,
})
} finally {
;(global as any).fetch = originalFetch
}
})
it('Should connect via plugin (fx.ws)', async () => {
expect(pluginWebSocket).toBeDefined()
```
如果你的测试环境中还没有全局可用的 `Response`,需要在 `test/websocket.spec.ts` 顶部引入或进行 polyfill,例如:
- `import { Response } from 'cross-fetch'` 或
- `import { Response } from 'node-fetch'`
具体取决于你的测试套件其余部分所使用的 fetch/Response 实现。
</issue_to_address>
### Comment 5
<location> `test/websocket.spec.ts:31-38` </location>
<code_context>
- )
- expect(data).to.be.instanceOf(MockWebSocket)
- data?.close()
+ it('Should connect via plugin (fx.ws)', async () => {
+ expect(pluginWebSocket).toBeDefined()
+ expect(typeof (pluginWebSocket as any).install).to.equal('function')
+ let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
+ fx = await fx.plugin(pluginWebSocket)
+ const ws = await fx.ws(WS_URL.replace(/^http/, 'ws'))
+ expect(ws).to.be.instanceOf(MockWebSocket)
+ ws.close()
})
</code_context>
<issue_to_address>
**suggestion (testing):** 扩展插件 WebSocket 测试,以覆盖生命周期事件以及在 `open` 之前出现错误的行为。
为了更好地覆盖基于插件的新 WebSocket API,请再添加:
1. 一个测试:注册 `websocket:beforeConnect`、`websocket:open`、`websocket:message`、`websocket:error` 和 `websocket:close` 处理器,然后运行一次基本的 `fx.ws(...)` + 发送/关闭流程,用于断言钩子触发顺序以及上下文(URL、超时时间、socket 等)。
2. 一个测试:`WebSocket` 在 `open` 之前触发 `error`,断言 `fx.ws(...)` 会以 `FexiosError` reject,且 `code === FexiosErrorCodes.NETWORK_ERROR`。
这些测试配合现有的成功用例,将更全面地覆盖生命周期和错误映射行为。
建议实现:
```typescript
fx.get(WS_URL.replace(/^http/, 'ws') as any)
).rejects.toMatchObject({ code: FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN })
})
it('Should connect via plugin (fx.ws)', async () => {
expect(pluginWebSocket).toBeDefined()
expect(typeof (pluginWebSocket as any).install).to.equal('function')
let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
fx = await fx.plugin(pluginWebSocket)
const ws = await fx.ws(WS_URL.replace(/^http/, 'ws'))
expect(ws).to.be.instanceOf(MockWebSocket)
ws.close()
})
it('Should emit websocket lifecycle hooks in order via plugin', async () => {
const events: string[] = []
const beforeConnectContexts: any[] = []
const openContexts: any[] = []
const messageContexts: any[] = []
const errorContexts: any[] = []
const closeContexts: any[] = []
let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
fx = await fx.plugin(pluginWebSocket)
// Adjust hook registration to your actual hook API if needed
fx.hooks.on('websocket:beforeConnect', (ctx: any) => {
events.push('beforeConnect')
beforeConnectContexts.push(ctx)
})
fx.hooks.on('websocket:open', (ctx: any) => {
events.push('open')
openContexts.push(ctx)
})
fx.hooks.on('websocket:message', (ctx: any) => {
events.push('message')
messageContexts.push(ctx)
})
fx.hooks.on('websocket:error', (ctx: any) => {
events.push('error')
errorContexts.push(ctx)
})
fx.hooks.on('websocket:close', (ctx: any) => {
events.push('close')
closeContexts.push(ctx)
})
const url = WS_URL.replace(/^http/, 'ws')
const ws: any = await fx.ws(url)
expect(ws).to.be.instanceOf(MockWebSocket)
expect(events[0]).to.equal('beforeConnect')
expect(beforeConnectContexts[0]).to.have.property('url', url)
// Drive the mock socket through a simple lifecycle; adapt to MockWebSocket API
if (typeof ws.mockOpen === 'function') {
ws.mockOpen()
}
if (typeof ws.mockMessage === 'function') {
ws.mockMessage('test-message')
}
if (typeof ws.mockError === 'function') {
ws.mockError(new Error('test-error'))
}
ws.close()
expect(events).to.deep.equal(['beforeConnect', 'open', 'message', 'error', 'close'])
// Basic context shape assertions; tweak expected properties to match your implementation
expect(openContexts[0]).to.have.property('url', url)
expect(openContexts[0]).to.have.property('socket')
expect(messageContexts[0]).to.have.property('socket')
expect(messageContexts[0]).to.have.property('data')
expect(closeContexts[0]).to.have.property('socket')
})
it('Should reject fx.ws with NETWORK_ERROR when WebSocket errors before open', async () => {
let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
fx = await fx.plugin(pluginWebSocket)
const url = WS_URL.replace(/^http/, 'ws')
// Arrange the next WebSocket instance to error before open; adapt to MockWebSocket API
if (typeof (MockWebSocket as any).mockNextErrorBeforeOpen === 'function') {
;(MockWebSocket as any).mockNextErrorBeforeOpen(new Error('connect-failure'))
}
await expect(fx.ws(url)).rejects.toMatchObject({
code: FexiosErrorCodes.NETWORK_ERROR,
})
})
const WS_URL = `${MOCK_FETCH_BASE_URL}/_ws`
}
})
```
1. 如有需要,请根据你实际的钩子 API 调整钩子注册方式(例如 `fx.on(...)` 或 `fx.hooks.websocket.onOpen(...)`)。
2. 将 `ws.mockOpen`、`ws.mockMessage` 和 `ws.mockError` 替换为你的 `MockWebSocket` 实现实际提供的辅助方法(或直接触发事件)。关键是驱动一次 `open` → `message` → `error` → `close` 的顺序,以便按顺序触发钩子。
3. 如果目前不存在 `MockWebSocket.mockNextErrorBeforeOpen`(或等价方法),需要实现它。它应确保下一次创建的 WebSocket 实例会在 `open` 之前触发一次 `error` 事件,从而让 `fx.ws(...)` reject,并使测试能够断言该错误被映射为 `FexiosErrorCodes.NETWORK_ERROR`。
4. 如果 `FexiosError` 是公开错误契约的一部分,并且你希望同时断言错误类型,可以在正确引入该类型后,将断言扩展为同时检查 `instanceof FexiosError`。
</issue_to_address>
### Comment 6
<location> `test/websocket.spec.ts:60-69` </location>
<code_context>
+ it('Honors timeout in plugin when socket never opens', async () => {
</code_context>
<issue_to_address>
**suggestion (testing):** 也要测试 WebSocket 连接的每次调用级别超时覆盖行为。
为了完整覆盖超时语义,请添加一个测试:客户端有更大的默认超时时间,但在单次调用中对其进行了覆盖,例如:
```ts
let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL, timeout: 1000 })
fx = await fx.plugin(pluginWebSocket)
await expect(
fx.ws(WS_URL.replace(/^http/, 'ws'), { timeout: 10 })
).rejects.toMatchObject({ code: FexiosErrorCodes.TIMEOUT })
```
这将验证每次调用的 `timeout` 优先于 `fx.baseConfigs.timeout`,并且覆盖逻辑有被测试到。
建议实现:
```typescript
it('Honors timeout in plugin when socket never opens', async () => {
```
`.
以下是变更内容:
<file_operations>
<file_operation operation="edit" file_path="test/websocket.spec.ts">
<<<<<<< SEARCH
it('Honors timeout in plugin when socket never opens', async () => {
=======
it('Honors timeout in plugin when socket never opens', async () => {
>>>>>>> REPLACE
</file_operation>
</file_operations>
<additional_changes>
请在 `test/websocket.spec.ts` 中的其他 WebSocket 超时测试附近(例如紧跟在 `it('Honors timeout in plugin when socket never opens', ...)` 后面)添加如下测试:
```ts
it('honors per-call timeout override for WebSocket connections', async () => {
const WS_URL = MOCK_FETCH_BASE_URL.replace(/^http/, 'ws')
let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL, timeout: 1000 })
fx = await fx.plugin(pluginWebSocket)
await expect(
fx.ws(WS_URL, { timeout: 10 })
).rejects.toMatchObject({ code: FexiosErrorCodes.TIMEOUT })
})
```
注意:
1. 这里假设 `MOCK_FETCH_BASE_URL`、`Fexios`、`pluginWebSocket` 和 `FexiosErrorCodes` 已经在该 spec 文件中导入/可用(根据现有测试,这很可能已经成立)。
2. 如果文件中已经定义了顶层常量 `WS_URL`,可以重用它而不是在测试内部重新定义:
```ts
await expect(
fx.ws(WS_URL, { timeout: 10 })
).rejects.toMatchObject({ code: FexiosErrorCodes.TIMEOUT })
```
3. 请将该测试放在与其他 WebSocket 插件测试相同的 `describe` 块中,这样可以复用相同的上下文和 setup/teardown。
</additional_changes>
</issue_to_address>
### Comment 7
<location> `test/sse.spec.ts:37-46` </location>
<code_context>
})
+ })
+
+ it('Server Sent Events via plugin (fx.sse)', async () => {
+ expect(pluginSSE).toBeDefined()
+ expect(typeof (pluginSSE as any).install).to.equal('function')
</code_context>
<issue_to_address>
**suggestion (testing):** 为插件中的 SSE 生命周期钩子和错误映射添加测试。
现有的 happy-path 测试很好,但请再为新插件的生命周期及错误/超时行为增加覆盖:
1. 一个测试:订阅 `sse:beforeConnect`、`sse:open`、`sse:message`、`sse:error` 和 `sse:close`,调用 `fx.sse(...)`,并断言:
- `sse:beforeConnect` 能看到归一化后的 URL 以及最终的超时时间;
- `sse:open` / `sse:message` 收到同一个上下文对象以及正确的 `Event`/`MessageEvent`。
2. 一个测试:mock 的 `EventSource` 在 `open` 之前触发 `error`,断言 `fx.sse(...)` 会以 `FexiosError` 和 `FexiosErrorCodes.NETWORK_ERROR` 进行 reject。
3. 一个测试:验证单次调用的 `timeout` 会覆盖 `fx.baseConfigs.timeout`(类似 WebSocket 的测试)。
这将验证插件的事件、错误和超时行为都符合预期契约。
建议实现:
```typescript
await new Promise<void>((resolve) => {
sse.onmessage = (event) => {
messages.push(event.data)
if (messages.length === 3) {
resolve()
}
}
})
expect(messages[0]).to.include('Message 1')
expect(messages[1]).to.include('Message 2')
expect(messages[2]).to.include('Message 3')
})
it('emits SSE lifecycle hooks with normalized URL and context', async () => {
let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL, timeout: 1234 })
fx = await fx.plugin(pluginSSE)
const events: {
beforeConnect?: any
open?: any
messages: any[]
errors: any[]
closes: any[]
} = {
messages: [],
errors: [],
closes: [],
}
fx.on('sse:beforeConnect', (ctx: any) => {
events.beforeConnect = ctx
})
fx.on('sse:open', (ctx: any, ev: Event) => {
events.open = { ctx, ev }
})
fx.on('sse:message', (ctx: any, ev: MessageEvent) => {
events.messages.push({ ctx, ev })
})
fx.on('sse:error', (ctx: any, err: any) => {
events.errors.push({ ctx, err })
})
fx.on('sse:close', (ctx: any) => {
events.closes.push(ctx)
})
const sse = (await fx.sse(SSE_URL, {
query: { timeout: 3 },
timeout: 5000,
})) as any as MockEventSource
// Wait until we receive at least one message so all hooks have fired
await new Promise<void>((resolve) => {
let count = 0
sse.onmessage = (event: MessageEvent) => {
count++
if (count === 1) {
resolve()
}
}
})
expect(events.beforeConnect).to.exist
expect(events.beforeConnect.url).to.include(SSE_URL)
expect(events.beforeConnect.timeout).to.equal(5000)
expect(events.open).to.exist
expect(events.open.ctx).to.deep.equal(events.beforeConnect)
expect(events.open.ev).to.be.instanceOf(Event)
expect(events.messages.length).to.be.greaterThan(0)
expect(events.messages[0].ctx).to.deep.equal(events.beforeConnect)
expect(events.messages[0].ev).to.be.instanceOf(MessageEvent)
})
it('maps SSE connection errors to FexiosError with NETWORK_ERROR code', async () => {
let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
fx = await fx.plugin(pluginSSE)
// Arrange the mock so it emits "error" before "open"
MockEventSource.nextErrorBeforeOpen(new Error('boom'))
await expect(
fx.sse(SSE_URL, {
query: { timeout: 3 },
}),
)
.to.be.rejectedWith(FexiosError)
.and.to.have.property('code', FexiosErrorCodes.NETWORK_ERROR)
})
it('allows per-call timeout to override baseConfigs.timeout for SSE', async () => {
const baseTimeout = 10_000
const perCallTimeout = 1_234
let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL, timeout: baseTimeout })
fx = await fx.plugin(pluginSSE)
let beforeConnectTimeout: number | undefined
fx.on('sse:beforeConnect', (ctx: any) => {
beforeConnectTimeout = ctx.timeout
})
const sse = (await fx.sse(SSE_URL, {
timeout: perCallTimeout,
query: { timeout: 3 },
})) as any as MockEventSource
// Ensure connection actually starts so beforeConnect hook runs
await new Promise<void>((resolve) => {
sse.onopen = () => resolve()
})
expect(beforeConnectTimeout).to.equal(perCallTimeout)
})
```
由于目前仅能看到 `test/sse.spec.ts` 的部分片段,你可能需要根据实际的测试工具做一些对齐调整:
1. **`MockEventSource.nextErrorBeforeOpen`**:
- 需要在 `MockEventSource` 上实现该辅助方法(或者根据你现在模拟 `open` 前 `error` 的方式调整测试)。
- 它应当配置下一次创建的 `MockEventSource` 实例,使其在 `open` 事件之前立即触发一次 `error`,从而让 `fx.sse(...)` reject。
2. **生命周期事件名称和函数签名**:
- 测试假设存在 `fx.on('sse:beforeConnect' | 'sse:open' | 'sse:message' | 'sse:error' | 'sse:close', (ctx, payload?) => {})` 这样的 API,这是 SSE 插件暴露钩子的方式。
- 如果你的实际 API 不同(例如 `fx.onSSE(...)` 或使用不同的事件名),请相应调整 `fx.on(...)` 调用以及参数结构,但请保留对 URL、timeout 以及相同 `ctx` 传递的断言。
3. **断言库的风格**:
- 示例使用的是 `chai`/`chai-as-promised` 风格(例如 `expect(...).to.be.rejectedWith(...)`、`expect(...).to.exist`)。
- 如果文件其他部分使用的是不同风格(例如 Jest),请将断言改写为对应风格(例如 `await expect(...).rejects.toMatchObject({ code: FexiosErrorCodes.NETWORK_ERROR })`)。
4. **`Event` / `MessageEvent` 的类型**:
- 在 Node 测试环境中,如果全局没有 `Event` 和 `MessageEvent`,可能需要引入或 polyfill。
- 或者,如果现有测试没有断言 `instanceOf` DOM 类型,可以放宽这些检查。
5. **原有 SSE 测试主体的还原**:
- 上面的片段对现有的 `"Server Sent Events via plugin (fx.sse)"` 测试(收集 3 条消息并断言内容)做了推测还原。如果你的实际实现有所不同,请在合并生命周期/错误/超时相关测试时保留你已经存在的测试逻辑。
</issue_to_address>
### Comment 8
<location> `test/sse.spec.ts:24-34` </location>
<code_context>
- const { data: sse } = await fexios.get<MockEventSource>(SSE_URL, {
- query: { timeout: 3 },
- fetch: mockFetch,
+ it('Legacy usage should throw plugin guidance error (text/event-stream)', async () => {
+ const fx = new Fexios({
+ baseURL: MOCK_FETCH_BASE_URL,
+ fetch: async () =>
+ new Response(null, {
+ headers: { 'content-type': 'text/event-stream' },
+ }),
+ })
+ await expect(fx.get('/anything' as any)).rejects.toMatchObject({
+ code: FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN,
})
</code_context>
<issue_to_address>
**suggestion (testing):** 覆盖应当现在触发 `FEATURE_MOVED_TO_PLUGIN` 的旧版 `responseType: 'stream'` 路径。
`createFexiosResponse` 中还有一条针对 `responseType === 'stream'` 的新旧版保护逻辑。请添加类似下面的测试:
```ts
const fx = new Fexios({ baseURL: MOCK_FETCH_BASE_URL, fetch: async () => new Response('') })
await expect(
fx.get('/anything' as any, { responseType: 'stream' as any })
).rejects.toMatchObject({ code: FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN })
```
这样可以覆盖显式指定 `responseType: 'stream'` 的路径,并引导用户使用基于插件的 SSE API。
```suggestion
it('Legacy usage should throw plugin guidance error (text/event-stream)', async () => {
const fx = new Fexios({
baseURL: MOCK_FETCH_BASE_URL,
fetch: async () =>
new Response(null, {
headers: { 'content-type': 'text/event-stream' },
}),
})
await expect(fx.get('/anything' as any)).rejects.toMatchObject({
code: FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN,
})
})
it('Legacy usage should throw plugin guidance error when using responseType="stream"', async () => {
const fx = new Fexios({
baseURL: MOCK_FETCH_BASE_URL,
fetch: async () => new Response(''),
})
await expect(
fx.get('/anything' as any, { responseType: 'stream' as any })
).rejects.toMatchObject({
code: FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN,
})
```
</issue_to_address>
### Comment 9
<location> `CHANGELOG.md:12` </location>
<code_context>
- Request URLs starting with `ws://` or `wss://`
</code_context>
<issue_to_address>
**security (javascript.lang.security.detect-insecure-websocket):** 检测到不安全的 WebSocket。所有 WebSocket 连接都应使用安全的 WebSocket(wss)。
*Source: opengrep*
</issue_to_address>
### Comment 10
<location> `src/plugins/ws/index.ts:55` </location>
<code_context>
if (s.startsWith('http://')) return s.replace(/^http:\/\//, 'ws://')
</code_context>
<issue_to_address>
**security (javascript.lang.security.detect-insecure-websocket):** 检测到不安全的 WebSocket。所有 WebSocket 连接都应使用安全的 WebSocket(wss)。
*Source: opengrep*
</issue_to_address>
### Comment 11
<location> `src/plugins/ws/index.ts:67` </location>
<code_context>
input.startsWith('ws://') ||
</code_context>
<issue_to_address>
**security (javascript.lang.security.detect-insecure-websocket):** 检测到不安全的 WebSocket。所有 WebSocket 连接都应使用安全的 WebSocket(wss)。
*Source: opengrep*
</issue_to_address>
### Comment 12
<location> `src/plugins/ws/index.ts:72` </location>
<code_context>
m === 'https://' ? 'wss://' : 'ws://'
</code_context>
<issue_to_address>
**security (javascript.lang.security.detect-insecure-websocket):** 检测到不安全的 WebSocket。所有 WebSocket 连接都应使用安全的 WebSocket(wss)。
*Source: opengrep*
</issue_to_address>Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Original comment in English
Hey - I've found 4 security issues, 8 other issues, and left some high level feedback:
Security issues:
- Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections. (link)
- Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections. (link)
- Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections. (link)
- Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections. (link)
General comments:
- In
request()you callthis.emit('afterRawResponse', ctx)withoutawait, which meansafterRawResponsehooks can’t short-circuit or reliably see a fully-updated context; consider awaiting this emit and handling the FINAL_SYMBOL the same way as the other lifecycle events. - The CHANGELOG describes
fx.plugin(plugin)returning an uninstall helper, but the currentplugin()implementation always returnsthis/Fexios; consider aligning the implementation (and its return type) with the documented uninstall helper behavior or updating the docs to match the actual API.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `request()` you call `this.emit('afterRawResponse', ctx)` without `await`, which means `afterRawResponse` hooks can’t short-circuit or reliably see a fully-updated context; consider awaiting this emit and handling the FINAL_SYMBOL the same way as the other lifecycle events.
- The CHANGELOG describes `fx.plugin(plugin)` returning an uninstall helper, but the current `plugin()` implementation always returns `this`/`Fexios`; consider aligning the implementation (and its return type) with the documented uninstall helper behavior or updating the docs to match the actual API.
## Individual Comments
### Comment 1
<location> `src/fexios.ts:407-408` </location>
<code_context>
if (timer) clearTimeout(timer)
ctx.rawResponse = rawResponse
+ this.emit('afterRawResponse', ctx)
+
ctx.response = await createFexiosResponse(
</code_context>
<issue_to_address>
**issue (bug_risk):** `afterRawResponse` hooks are not awaited, so async handlers and short-circuiting via that event won't behave as expected.
Other lifecycle events (`beforeInit`, `afterBodyTransformed`, `beforeActualFetch`, `afterResponse`) are awaited and can short-circuit via `FINAL_SYMBOL`, but `afterRawResponse` is fire-and-forget and its return value is ignored.
As a result, async `afterRawResponse` hooks can’t affect control flow or modify the context/response seen by later logic.
To align behavior and make this hook useful, it should be awaited and support short-circuiting, e.g.:
```ts
ctx.rawResponse = rawResponse
ctx = await this.emit('afterRawResponse', ctx)
if ((ctx as any)[Fexios.FINAL_SYMBOL]) return ctx as any
```
(or equivalent handling if you want short-circuiting here).
</issue_to_address>
### Comment 2
<location> `src/fexios.ts:385-387` </location>
<code_context>
- const rawResponse = await fetch(ctx.rawRequest!).catch((err) => {
- if (timer) clearTimeout(timer)
- if (abortController?.signal.aborted) {
+ const fetch =
+ (ctx.request as any).fetch || this.baseConfigs.fetch || globalThis.fetch
+ const rawResponse = await fetch(ctx.request.rawRequest!).catch(
+ (err: any) => {
+ if (timer) clearTimeout(timer)
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Re-throwing fetch errors loses non-`Error` rejection details and may rely on `err.message` when it does not exist.
The `.catch((err: any) => { ... err.message ... })` path assumes `err` is `Error`-like. Some fetch implementations or shims may reject with strings or other values, so `err.message` can be `undefined`, producing an unhelpful `FexiosError`.
Consider normalizing the message, as in the SSE plugin:
```ts
const msg =
err instanceof Error ? err.message :
typeof err === 'string' ? err :
'Network request failed'
throw new FexiosError(FexiosErrorCodes.NETWORK_ERROR, msg, ctx)
```
This keeps more useful error information across different environments.
</issue_to_address>
### Comment 3
<location> `src/plugins/ws/index.ts:83-92` </location>
<code_context>
+ ).toString()
+}
+
+async function waitForWsOpen(ws: WebSocket, delay: number) {
+ if ((ws as any).readyState === 1) return
+ await new Promise<void>((resolve, reject) => {
+ const timer =
+ delay > 0
+ ? setTimeout(() => {
+ try {
+ ws.close()
+ } catch {}
+ reject(
+ new FexiosError(
+ FexiosErrorCodes.TIMEOUT,
+ `WebSocket connection timed out after ${delay}ms`
+ )
+ )
+ }, delay)
+ : undefined
+
+ const cleanup = () => {
+ if (timer) clearTimeout(timer)
+ ws.removeEventListener('open', onOpen)
+ ws.removeEventListener('error', onError)
+ }
+
</code_context>
<issue_to_address>
**suggestion:** WebSocket open-wait logic does not react to `close` before `open`, leading to longer-than-necessary timeouts.
`waitForWsOpen` only listens for `open` and `error`. If the socket moves to `CLOSING`/`CLOSED` without an `error`, the promise won’t reject until the timeout, even though we know the connection won’t succeed.
To fail fast and match the SSE plugin behavior, also listen for `close` and reject immediately, e.g.:
```ts
const onClose = (event: CloseEvent) => {
cleanup()
reject(
new FexiosError(
FexiosErrorCodes.NETWORK_ERROR,
`WebSocket connection closed before open (code: ${event.code}, reason: ${event.reason})`,
undefined,
{ cause: event }
)
)
}
ws.addEventListener('close', onClose)
// and remove it in cleanup()
```
This prevents unnecessary waiting when the server closes the connection early.
Suggested implementation:
```typescript
const onClose = (event: CloseEvent) => {
cleanup()
reject(
new FexiosError(
FexiosErrorCodes.NETWORK_ERROR,
`WebSocket connection closed before open (code: ${event.code}, reason: ${event.reason})`,
undefined,
{ cause: event }
)
)
}
const cleanup = () => {
if (timer) clearTimeout(timer)
ws.removeEventListener('open', onOpen)
ws.removeEventListener('error', onError)
ws.removeEventListener('close', onClose)
}
```
```typescript
const timer =
delay > 0
? setTimeout(() => {
try {
ws.close()
} catch {}
reject(
new FexiosError(
FexiosErrorCodes.TIMEOUT,
`WebSocket connection timed out after ${delay}ms`
)
)
}, delay)
: undefined
```
```typescript
const onClose = (event: CloseEvent) => {
cleanup()
reject(
new FexiosError(
FexiosErrorCodes.NETWORK_ERROR,
`WebSocket connection closed before open (code: ${event.code}, reason: ${event.reason})`,
undefined,
{ cause: event }
)
)
}
const cleanup = () => {
if (timer) clearTimeout(timer)
ws.removeEventListener('open', onOpen)
ws.removeEventListener('error', onError)
ws.removeEventListener('close', onClose)
}
```
```typescript
ws.addEventListener('open', onOpen)
ws.addEventListener('error', onError)
ws.addEventListener('close', onClose)
```
</issue_to_address>
### Comment 4
<location> `test/websocket.spec.ts:24-26` </location>
<code_context>
- })
- expect(data).to.be.instanceOf(MockWebSocket)
- data?.close()
+ it('Legacy usage should throw plugin guidance error', async () => {
+ const fx = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
+ await expect(
+ fx.get(WS_URL.replace(/^http/, 'ws') as any)
+ ).rejects.toMatchObject({ code: FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN })
</code_context>
<issue_to_address>
**suggestion (testing):** Add coverage for legacy `responseType: 'ws'` and upgrade-header paths that should now throw `FEATURE_MOVED_TO_PLUGIN`.
This covers the legacy `ws://` URL case. To fully exercise the new behavior, please also add tests that:
1. Call `fx.get('/ws-endpoint', { responseType: 'ws' as any })` and assert the rejection has `code === FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN`.
2. Use a mocked `fetch` that returns a `Response` with `upgrade: websocket` and `connection: upgrade` headers and assert that `createFexiosResponse` (or `fx.get`) rejects with the same code.
That will confirm all legacy WebSocket entry points now consistently direct users to the plugin-based API.
Suggested implementation:
```typescript
}
})
it('Legacy usage should throw plugin guidance error', async () => {
const fx = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
await expect(
fx.get(WS_URL.replace(/^http/, 'ws') as any)
).rejects.toMatchObject({ code: FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN })
})
it('Legacy responseType "ws" should throw plugin guidance error', async () => {
const fx = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
await expect(
fx.get('/ws-endpoint', { responseType: 'ws' as any })
).rejects.toMatchObject({ code: FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN })
})
it('Legacy upgrade-header path should throw plugin guidance error', async () => {
const fx = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
const originalFetch = (global as any).fetch
const mockFetch = jest.fn().mockResolvedValue(
new Response(null, {
status: 101,
headers: {
upgrade: 'websocket',
connection: 'upgrade',
},
})
)
;(global as any).fetch = mockFetch
try {
await expect(fx.get('/ws-upgrade-endpoint')).rejects.toMatchObject({
code: FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN,
})
} finally {
;(global as any).fetch = originalFetch
}
})
it('Should connect via plugin (fx.ws)', async () => {
expect(pluginWebSocket).toBeDefined()
```
If `Response` is not already globally available in your test environment, you will need to import or polyfill it at the top of `test/websocket.spec.ts`, for example:
- `import { Response } from 'cross-fetch'` or
- `import { Response } from 'node-fetch'`
depending on what the rest of your test suite uses for fetch/Response.
</issue_to_address>
### Comment 5
<location> `test/websocket.spec.ts:31-38` </location>
<code_context>
- )
- expect(data).to.be.instanceOf(MockWebSocket)
- data?.close()
+ it('Should connect via plugin (fx.ws)', async () => {
+ expect(pluginWebSocket).toBeDefined()
+ expect(typeof (pluginWebSocket as any).install).to.equal('function')
+ let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
+ fx = await fx.plugin(pluginWebSocket)
+ const ws = await fx.ws(WS_URL.replace(/^http/, 'ws'))
+ expect(ws).to.be.instanceOf(MockWebSocket)
+ ws.close()
})
</code_context>
<issue_to_address>
**suggestion (testing):** Extend plugin WebSocket tests to cover lifecycle events and error-before-open behavior.
To better cover the new plugin-based WebSocket API, please also add:
1. A test that registers `websocket:beforeConnect`, `websocket:open`, `websocket:message`, `websocket:error`, and `websocket:close` handlers, then runs a basic `fx.ws(...)` + send/close flow to assert hook order and context (URL, timeout, socket, etc.).
2. A test where `WebSocket` emits `error` before `open`, asserting that `fx.ws(...)` rejects with `FexiosError` and `code === FexiosErrorCodes.NETWORK_ERROR`.
These, along with the existing success test, will more fully exercise lifecycle and error mapping behavior.
Suggested implementation:
```typescript
fx.get(WS_URL.replace(/^http/, 'ws') as any)
).rejects.toMatchObject({ code: FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN })
})
it('Should connect via plugin (fx.ws)', async () => {
expect(pluginWebSocket).toBeDefined()
expect(typeof (pluginWebSocket as any).install).to.equal('function')
let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
fx = await fx.plugin(pluginWebSocket)
const ws = await fx.ws(WS_URL.replace(/^http/, 'ws'))
expect(ws).to.be.instanceOf(MockWebSocket)
ws.close()
})
it('Should emit websocket lifecycle hooks in order via plugin', async () => {
const events: string[] = []
const beforeConnectContexts: any[] = []
const openContexts: any[] = []
const messageContexts: any[] = []
const errorContexts: any[] = []
const closeContexts: any[] = []
let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
fx = await fx.plugin(pluginWebSocket)
// Adjust hook registration to your actual hook API if needed
fx.hooks.on('websocket:beforeConnect', (ctx: any) => {
events.push('beforeConnect')
beforeConnectContexts.push(ctx)
})
fx.hooks.on('websocket:open', (ctx: any) => {
events.push('open')
openContexts.push(ctx)
})
fx.hooks.on('websocket:message', (ctx: any) => {
events.push('message')
messageContexts.push(ctx)
})
fx.hooks.on('websocket:error', (ctx: any) => {
events.push('error')
errorContexts.push(ctx)
})
fx.hooks.on('websocket:close', (ctx: any) => {
events.push('close')
closeContexts.push(ctx)
})
const url = WS_URL.replace(/^http/, 'ws')
const ws: any = await fx.ws(url)
expect(ws).to.be.instanceOf(MockWebSocket)
expect(events[0]).to.equal('beforeConnect')
expect(beforeConnectContexts[0]).to.have.property('url', url)
// Drive the mock socket through a simple lifecycle; adapt to MockWebSocket API
if (typeof ws.mockOpen === 'function') {
ws.mockOpen()
}
if (typeof ws.mockMessage === 'function') {
ws.mockMessage('test-message')
}
if (typeof ws.mockError === 'function') {
ws.mockError(new Error('test-error'))
}
ws.close()
expect(events).to.deep.equal(['beforeConnect', 'open', 'message', 'error', 'close'])
// Basic context shape assertions; tweak expected properties to match your implementation
expect(openContexts[0]).to.have.property('url', url)
expect(openContexts[0]).to.have.property('socket')
expect(messageContexts[0]).to.have.property('socket')
expect(messageContexts[0]).to.have.property('data')
expect(closeContexts[0]).to.have.property('socket')
})
it('Should reject fx.ws with NETWORK_ERROR when WebSocket errors before open', async () => {
let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
fx = await fx.plugin(pluginWebSocket)
const url = WS_URL.replace(/^http/, 'ws')
// Arrange the next WebSocket instance to error before open; adapt to MockWebSocket API
if (typeof (MockWebSocket as any).mockNextErrorBeforeOpen === 'function') {
;(MockWebSocket as any).mockNextErrorBeforeOpen(new Error('connect-failure'))
}
await expect(fx.ws(url)).rejects.toMatchObject({
code: FexiosErrorCodes.NETWORK_ERROR,
})
})
const WS_URL = `${MOCK_FETCH_BASE_URL}/_ws`
}
})
```
1. Adjust the hook registration calls (`fx.hooks.on('websocket:...')`) to match your actual hook API. If your hook system differs (e.g. `fx.on('websocket:open', ...)` or `fx.hooks.websocket.onOpen(...)`), update those lines accordingly.
2. Replace the `ws.mockOpen`, `ws.mockMessage`, and `ws.mockError` calls with the actual helper methods (or direct event triggering) provided by your `MockWebSocket` implementation. The key requirement is to drive a sequence of `open` → `message` → `error` → `close` so the hooks are invoked in order.
3. Implement `MockWebSocket.mockNextErrorBeforeOpen` (or equivalent) if it does not already exist. It should ensure that the created WebSocket instance emits an `error` event before `open`, causing `fx.ws(...)` to reject and allowing the test to assert that this is mapped to `FexiosErrorCodes.NETWORK_ERROR`.
4. If `FexiosError` is part of the public error contract and you want to assert on the error type as well, you can extend the rejection assertion to also verify `instanceof FexiosError` once the correct import is available in this test file.
</issue_to_address>
### Comment 6
<location> `test/websocket.spec.ts:60-69` </location>
<code_context>
+ it('Honors timeout in plugin when socket never opens', async () => {
</code_context>
<issue_to_address>
**suggestion (testing):** Also test per-call timeout overrides for WebSocket connections.
To fully cover the timeout semantics, please add a test where a client with a higher default timeout overrides it per call, e.g.:
```ts
let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL, timeout: 1000 })
fx = await fx.plugin(pluginWebSocket)
await expect(
fx.ws(WS_URL.replace(/^http/, 'ws'), { timeout: 10 })
).rejects.toMatchObject({ code: FexiosErrorCodes.TIMEOUT })
```
This will verify that the per-call `timeout` takes precedence over `fx.baseConfigs.timeout` and that the override logic is properly covered by tests.
Suggested implementation:
```typescript
it('Honors timeout in plugin when socket never opens', async () => {
```
`.
Here are the changes:
<file_operations>
<file_operation operation="edit" file_path="test/websocket.spec.ts">
<<<<<<< SEARCH
it('Honors timeout in plugin when socket never opens', async () => {
=======
it('Honors timeout in plugin when socket never opens', async () => {
>>>>>>> REPLACE
</file_operation>
</file_operations>
<additional_changes>
Please add the following test near the other WebSocket timeout tests in `test/websocket.spec.ts` (for example, right after `it('Honors timeout in plugin when socket never opens', ...)`):
```ts
it('honors per-call timeout override for WebSocket connections', async () => {
const WS_URL = MOCK_FETCH_BASE_URL.replace(/^http/, 'ws')
let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL, timeout: 1000 })
fx = await fx.plugin(pluginWebSocket)
await expect(
fx.ws(WS_URL, { timeout: 10 })
).rejects.toMatchObject({ code: FexiosErrorCodes.TIMEOUT })
})
```
Notes:
1. This assumes `MOCK_FETCH_BASE_URL`, `Fexios`, `pluginWebSocket`, and `FexiosErrorCodes` are already imported/available in this spec file (they likely are, given the existing tests).
2. If `WS_URL` is already defined in the file (e.g., as a top-level constant), you can reuse it instead of redefining it inside the test:
```ts
await expect(
fx.ws(WS_URL, { timeout: 10 })
).rejects.toMatchObject({ code: FexiosErrorCodes.TIMEOUT })
```
3. Place the test within the same `describe` block as the other WebSocket plugin tests so it runs in the same context and uses the same setup/teardown.
</issue_to_address>
### Comment 7
<location> `test/sse.spec.ts:37-46` </location>
<code_context>
})
+ })
+
+ it('Server Sent Events via plugin (fx.sse)', async () => {
+ expect(pluginSSE).toBeDefined()
+ expect(typeof (pluginSSE as any).install).to.equal('function')
</code_context>
<issue_to_address>
**suggestion (testing):** Add tests for SSE lifecycle hooks and error mapping in the plugin.
The existing happy-path test is good, but please add coverage for the new plugin lifecycle and error/timeout behavior:
1. A test that subscribes to `sse:beforeConnect`, `sse:open`, `sse:message`, `sse:error`, and `sse:close`, calls `fx.sse(...)`, and asserts that:
- `sse:beforeConnect` sees the normalized URL and resolved timeout.
- `sse:open` / `sse:message` receive the same context plus the right `Event`/`MessageEvent`.
2. A test where a mocked `EventSource` emits `error` before `open`, asserting `fx.sse(...)` rejects with `FexiosError` and `FexiosErrorCodes.NETWORK_ERROR`.
3. A test verifying a per-call `timeout` overrides `fx.baseConfigs.timeout` for SSE, similar to the WebSocket tests.
This will validate that plugin events, errors, and timeouts match the intended contract.
Suggested implementation:
```typescript
await new Promise<void>((resolve) => {
sse.onmessage = (event) => {
messages.push(event.data)
if (messages.length === 3) {
resolve()
}
}
})
expect(messages[0]).to.include('Message 1')
expect(messages[1]).to.include('Message 2')
expect(messages[2]).to.include('Message 3')
})
it('emits SSE lifecycle hooks with normalized URL and context', async () => {
let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL, timeout: 1234 })
fx = await fx.plugin(pluginSSE)
const events: {
beforeConnect?: any
open?: any
messages: any[]
errors: any[]
closes: any[]
} = {
messages: [],
errors: [],
closes: [],
}
fx.on('sse:beforeConnect', (ctx: any) => {
events.beforeConnect = ctx
})
fx.on('sse:open', (ctx: any, ev: Event) => {
events.open = { ctx, ev }
})
fx.on('sse:message', (ctx: any, ev: MessageEvent) => {
events.messages.push({ ctx, ev })
})
fx.on('sse:error', (ctx: any, err: any) => {
events.errors.push({ ctx, err })
})
fx.on('sse:close', (ctx: any) => {
events.closes.push(ctx)
})
const sse = (await fx.sse(SSE_URL, {
query: { timeout: 3 },
timeout: 5000,
})) as any as MockEventSource
// Wait until we receive at least one message so all hooks have fired
await new Promise<void>((resolve) => {
let count = 0
sse.onmessage = (event: MessageEvent) => {
count++
if (count === 1) {
resolve()
}
}
})
expect(events.beforeConnect).to.exist
expect(events.beforeConnect.url).to.include(SSE_URL)
expect(events.beforeConnect.timeout).to.equal(5000)
expect(events.open).to.exist
expect(events.open.ctx).to.deep.equal(events.beforeConnect)
expect(events.open.ev).to.be.instanceOf(Event)
expect(events.messages.length).to.be.greaterThan(0)
expect(events.messages[0].ctx).to.deep.equal(events.beforeConnect)
expect(events.messages[0].ev).to.be.instanceOf(MessageEvent)
})
it('maps SSE connection errors to FexiosError with NETWORK_ERROR code', async () => {
let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL })
fx = await fx.plugin(pluginSSE)
// Arrange the mock so it emits "error" before "open"
MockEventSource.nextErrorBeforeOpen(new Error('boom'))
await expect(
fx.sse(SSE_URL, {
query: { timeout: 3 },
}),
)
.to.be.rejectedWith(FexiosError)
.and.to.have.property('code', FexiosErrorCodes.NETWORK_ERROR)
})
it('allows per-call timeout to override baseConfigs.timeout for SSE', async () => {
const baseTimeout = 10_000
const perCallTimeout = 1_234
let fx: any = new Fexios({ baseURL: MOCK_FETCH_BASE_URL, timeout: baseTimeout })
fx = await fx.plugin(pluginSSE)
let beforeConnectTimeout: number | undefined
fx.on('sse:beforeConnect', (ctx: any) => {
beforeConnectTimeout = ctx.timeout
})
const sse = (await fx.sse(SSE_URL, {
timeout: perCallTimeout,
query: { timeout: 3 },
})) as any as MockEventSource
// Ensure connection actually starts so beforeConnect hook runs
await new Promise<void>((resolve) => {
sse.onopen = () => resolve()
})
expect(beforeConnectTimeout).to.equal(perCallTimeout)
})
```
Because only a fragment of `test/sse.spec.ts` is visible, you may need to align a few details with the actual test utilities:
1. **`MockEventSource.nextErrorBeforeOpen`**:
- Implement this helper on `MockEventSource` (or adapt the test to however you currently simulate an `error` event before `open`).
- It should configure the next created `MockEventSource` instance to immediately emit an `error` event before any `open` event, causing `fx.sse(...)` to reject.
2. **Lifecycle event names and signature**:
- The tests assume `fx.on('sse:beforeConnect' | 'sse:open' | 'sse:message' | 'sse:error' | 'sse:close', (ctx, payload?) => {})` exists and is how the SSE plugin exposes hooks.
- If your actual API differs (e.g. `fx.onSSE(...)` or different event names), adjust the `fx.on(...)` calls and expected argument shapes accordingly, but keep the same assertions about URL, timeout, and that the same `ctx` is passed through.
3. **Assertion library alignment**:
- These tests use `chai`/`chai-as-promised`-style expectations (e.g. `expect(...).to.be.rejectedWith(...)`, `expect(...).to.exist`).
- If the rest of the file uses a different style (e.g. Jest), adapt the assertions to match (e.g. `await expect(...).rejects.toMatchObject({ code: FexiosErrorCodes.NETWORK_ERROR })`).
4. **Types for `Event` / `MessageEvent`**:
- In Node test environments you may need to import or polyfill `Event` and `MessageEvent` if they are not globally available.
- Alternatively, loosen those checks if your existing tests do not assert `instanceOf` against DOM types.
5. **The reconstructed original SSE test body**:
- I reconstructed the missing part of the existing `"Server Sent Events via plugin (fx.sse)"` test (collecting 3 messages and asserting their contents). If your current implementation differs, merge the lifecycle/error/timeout tests while preserving your actual existing test logic.
</issue_to_address>
### Comment 8
<location> `test/sse.spec.ts:24-34` </location>
<code_context>
- const { data: sse } = await fexios.get<MockEventSource>(SSE_URL, {
- query: { timeout: 3 },
- fetch: mockFetch,
+ it('Legacy usage should throw plugin guidance error (text/event-stream)', async () => {
+ const fx = new Fexios({
+ baseURL: MOCK_FETCH_BASE_URL,
+ fetch: async () =>
+ new Response(null, {
+ headers: { 'content-type': 'text/event-stream' },
+ }),
+ })
+ await expect(fx.get('/anything' as any)).rejects.toMatchObject({
+ code: FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN,
})
</code_context>
<issue_to_address>
**suggestion (testing):** Cover legacy `responseType: 'stream'` path that should now emit `FEATURE_MOVED_TO_PLUGIN`.
There’s also a new legacy guard in `createFexiosResponse` for `responseType === 'stream'`. Please add a test like:
```ts
const fx = new Fexios({ baseURL: MOCK_FETCH_BASE_URL, fetch: async () => new Response('') })
await expect(
fx.get('/anything' as any, { responseType: 'stream' as any })
).rejects.toMatchObject({ code: FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN })
```
so the explicit `responseType: 'stream'` path is covered and users are guided to the plugin-based SSE API.
```suggestion
it('Legacy usage should throw plugin guidance error (text/event-stream)', async () => {
const fx = new Fexios({
baseURL: MOCK_FETCH_BASE_URL,
fetch: async () =>
new Response(null, {
headers: { 'content-type': 'text/event-stream' },
}),
})
await expect(fx.get('/anything' as any)).rejects.toMatchObject({
code: FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN,
})
})
it('Legacy usage should throw plugin guidance error when using responseType="stream"', async () => {
const fx = new Fexios({
baseURL: MOCK_FETCH_BASE_URL,
fetch: async () => new Response(''),
})
await expect(
fx.get('/anything' as any, { responseType: 'stream' as any })
).rejects.toMatchObject({
code: FexiosErrorCodes.FEATURE_MOVED_TO_PLUGIN,
})
```
</issue_to_address>
### Comment 9
<location> `CHANGELOG.md:12` </location>
<code_context>
- Request URLs starting with `ws://` or `wss://`
</code_context>
<issue_to_address>
**security (javascript.lang.security.detect-insecure-websocket):** Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections.
*Source: opengrep*
</issue_to_address>
### Comment 10
<location> `src/plugins/ws/index.ts:55` </location>
<code_context>
if (s.startsWith('http://')) return s.replace(/^http:\/\//, 'ws://')
</code_context>
<issue_to_address>
**security (javascript.lang.security.detect-insecure-websocket):** Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections.
*Source: opengrep*
</issue_to_address>
### Comment 11
<location> `src/plugins/ws/index.ts:67` </location>
<code_context>
input.startsWith('ws://') ||
</code_context>
<issue_to_address>
**security (javascript.lang.security.detect-insecure-websocket):** Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections.
*Source: opengrep*
</issue_to_address>
### Comment 12
<location> `src/plugins/ws/index.ts:72` </location>
<code_context>
m === 'https://' ? 'wss://' : 'ws://'
</code_context>
<issue_to_address>
**security (javascript.lang.security.detect-insecure-websocket):** Insecure WebSocket Detected. WebSocket Secure (wss) should be used for all WebSocket connections.
*Source: opengrep*
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Summary by Sourcery
重构核心请求/响应上下文和插件架构,为 6.x 主要版本做准备,将 WebSocket 和 SSE 支持移入独立插件,同时通过遗留别名和改进的生命周期钩子来保持向后兼容性。
新功能:
fx.ws和fx.sse)以及用于实时特性的生命周期事件。ctx.app,并新增afterRawResponse钩子,以提供更丰富的钩子扩展能力,同时加入类型化的钩子处理辅助工具。错误修复:
rawRequest/rawResponse不变式以及上下文终结行为上保持一致性。FexiosError错误码。增强改进:
FexiosContext重构为request/runtime/response子对象,并保留遗留属性别名,同时优化默认应用行为以及 header/query 合并逻辑。plugin.install变为同步方法,新增plugin.uninstall,并简化插件 API 和文档以便于扩展。FexiosHookHandler优化拦截器类型定义,并扩展生命周期事件以覆盖afterRawResponse和实时插件事件。data、headers、rawRequest和rawResponse提供只读的快捷访问,同时保留便捷访问器。文档:
测试:
ctx.request/ctx.runtime结构和最终上下文行为。杂务:
checkIsPlainObject再导出,并从核心响应模型中清理遗留的 WS/SSE 响应辅助工具。Original summary in English
Summary by Sourcery
Refactor the core request/response context and plugin architecture for the 6.x major release, moving WebSocket and SSE support into dedicated plugins while preserving backward compatibility via legacy aliases and improved lifecycle hooks.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
Chores:
Original summary in English
Summary by Sourcery
重构核心请求/响应上下文和插件架构,为 6.x 主要版本做准备,将 WebSocket 和 SSE 支持移入独立插件,同时通过遗留别名和改进的生命周期钩子来保持向后兼容性。
新功能:
fx.ws和fx.sse)以及用于实时特性的生命周期事件。ctx.app,并新增afterRawResponse钩子,以提供更丰富的钩子扩展能力,同时加入类型化的钩子处理辅助工具。错误修复:
rawRequest/rawResponse不变式以及上下文终结行为上保持一致性。FexiosError错误码。增强改进:
FexiosContext重构为request/runtime/response子对象,并保留遗留属性别名,同时优化默认应用行为以及 header/query 合并逻辑。plugin.install变为同步方法,新增plugin.uninstall,并简化插件 API 和文档以便于扩展。FexiosHookHandler优化拦截器类型定义,并扩展生命周期事件以覆盖afterRawResponse和实时插件事件。data、headers、rawRequest和rawResponse提供只读的快捷访问,同时保留便捷访问器。文档:
测试:
ctx.request/ctx.runtime结构和最终上下文行为。杂务:
checkIsPlainObject再导出,并从核心响应模型中清理遗留的 WS/SSE 响应辅助工具。Original summary in English
Summary by Sourcery
Refactor the core request/response context and plugin architecture for the 6.x major release, moving WebSocket and SSE support into dedicated plugins while preserving backward compatibility via legacy aliases and improved lifecycle hooks.
New Features:
Bug Fixes:
Enhancements:
Documentation:
Tests:
Chores: