Copilot commented on code in PR #13254:
URL: https://github.com/apache/apisix/pull/13254#discussion_r3104758079
##########
apisix/plugin.lua:
##########
@@ -1397,13 +1397,22 @@ function _M.run_global_rules(api_ctx, global_rules,
conf_version, phase_name)
end
end
-function _M.lua_response_filter(api_ctx, headers, body)
+-- @param wait boolean When true, use synchronous flush (ngx.flush(true)) and
return
+-- (ok, err) so callers can detect client disconnection. Defaults to false
for
+-- backward compatibility (async flush, no return value).
+function _M.lua_response_filter(api_ctx, headers, body, wait)
local plugins = api_ctx.plugins
if not plugins or #plugins == 0 then
-- if there is no any plugin, just print the original body to
downstream
- ngx_print(body)
- ngx_flush()
- return
+ local ok, err = ngx_print(body)
+ if not ok then
+ return false, err
+ end
+ ok, err = ngx_flush(wait)
+ if not ok then
Review Comment:
`wait` is documented as defaulting to `false`, but when omitted it is
actually `nil` and gets passed through to `ngx_flush(wait)`. Consider
normalizing `wait` to a strict boolean and/or only passing an argument when
`wait == true` (otherwise call `ngx_flush()` with no args, as before) to keep
intent and backward-compatibility clearer.
##########
apisix/plugin.lua:
##########
@@ -1397,13 +1397,22 @@ function _M.run_global_rules(api_ctx, global_rules,
conf_version, phase_name)
end
end
-function _M.lua_response_filter(api_ctx, headers, body)
+-- @param wait boolean When true, use synchronous flush (ngx.flush(true)) and
return
+-- (ok, err) so callers can detect client disconnection. Defaults to false
for
+-- backward compatibility (async flush, no return value).
Review Comment:
The docstring says the default path uses async flush with “no return value”,
but this function now always returns a boolean (and possibly an error)
regardless of `wait`. Please update the comment to match the actual return
behavior to avoid misleading callers.
##########
apisix/plugins/ai-providers/base.lua:
##########
@@ -352,14 +352,36 @@ function _M.parse_streaming_response(self, ctx, res,
target_proto, converter)
::CONTINUE::
end
- -- Output: converter events or passthrough raw chunk
+ -- Output: converter events or passthrough raw chunk.
+ -- Pass wait=true for synchronous flush so we can detect client
disconnection.
if converter then
for _, c in ipairs(converted_chunks) do
- plugin.lua_response_filter(ctx, res.headers, c)
+ local ok, flush_err = plugin.lua_response_filter(ctx,
res.headers, c, true)
output_sent = true
+ if not ok then
+ core.log.info("client disconnected during AI streaming, ",
+ "aborting upstream read: ", flush_err)
+ if res._httpc then
+ res._httpc:close()
+ res._httpc = nil
+ end
+ ctx.var.llm_request_done = true
+ return
+ end
end
else
- plugin.lua_response_filter(ctx, res.headers, chunk)
+ local ok, flush_err = plugin.lua_response_filter(ctx, res.headers,
chunk, true)
+ output_sent = true
+ if not ok then
+ core.log.info("client disconnected during AI streaming, ",
+ "aborting upstream read: ", flush_err)
+ if res._httpc then
+ res._httpc:close()
+ res._httpc = nil
+ end
+ ctx.var.llm_request_done = true
+ return
+ end
Review Comment:
The client-disconnect handling logic (log + close `res._httpc` + set
`llm_request_done` + return) is duplicated in both the converter and
passthrough branches. Consider extracting it into a small local helper to
reduce duplication and keep future changes consistent.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
