AlinsRan opened a new pull request, #13870:
URL: https://github.com/apache/apisix/pull/13870

   ### Description
   
   A streaming AI request whose upstream answers with an SSE content type and 
then closes without writing a single body byte crashes the gateway in 
`balancer_by_lua`.
   
   `body_reader()` returns EOF on its first call, the loop body never runs, 
nothing is written downstream, and `parse_streaming_response()` returns `nil`. 
`run_plugin()` only calls `core.response.exit()` when a plugin returns a status 
code, so returning `nil` leaves the request neither answered nor terminated. It 
falls through to `proxy_pass` and enters `balancer_by_lua`, where 
`upstream_conf` is always `nil` because `handle_upstream()` skips 
`set_upstream()` for `bypass_nginx_upstream` plugins:
   
   ```
   failed to run balancer_by_lua*: apisix/balancer.lua:
       attempt to index local 'up_conf' (a nil value)
       in function 'pick_server'
       in function 'run'
       in function 'http_balancer_phase'
   ```
   
   `header_filter` then fails as well, so the client gets no HTTP response at 
all and the connection is closed.
   
   The `output_sent` guard added in the EOF branch already covers this, but 
only when a converter is active:
   
   ```lua
   if converter and not output_sent then
       ...
       return 502, msg
   end
   return
   ```
   
   An OpenAI-format client against an OpenAI-compatible provider matches 
`caps[client_protocol]`, so `converter` is `nil`, the guard is skipped, and the 
bare `return` below hands `nil` back up — the passthrough case (no protocol 
conversion) was never covered.
   
   This drops the `converter` condition and words the log message by whether a 
converter was involved. 502 rather than 504: nothing timed out, the upstream 
closed cleanly and the request's own timeout never fired; 502 is also what this 
same guard already returns for the converter case.
   
   `t/plugin/ai-proxy-empty-stream.t` mocks an upstream that answers `200` + 
SSE headers and closes with a zero-byte body, and asserts a 502 with no 
`up_conf` error.
   
   #### Which issue(s) this PR fixes:
   
   Fixes #
   
   ### Checklist
   
   - [x] I have explained the need for this PR and the problem it solves
   - [x] I have explained the changes or the new features added to this PR
   - [x] I have added tests corresponding to this change
   - [ ] I have updated the documentation to reflect this change
   - [x] I have verified that this change is backward compatible (If not, 
please discuss on the [APISIX mailing 
list](https://github.com/apache/apisix/tree/master#community) first)
   
   <!--
   
   Note
   
   1. Mark the PR as draft until it's ready to be reviewed.
   2. Always add/update tests for any changes unless you have a good reason.
   3. Always update the documentation to reflect the changes made in the PR.
   4. Make a new commit to resolve conversations instead of `push -f`.
   5. To resolve merge conflicts, merge master instead of rebasing.
   6. Use "request review" to notify the reviewer after making changes.
   7. Only a reviewer can mark a conversation as resolved.
   
   -->
   


-- 
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]

Reply via email to