Skip to content

Support STREAMED response body#25

Merged
ronenkat merged 5 commits into
ms-llmd:mainfrom
Mohammad-nassar10:pr2-stream-response
Jun 10, 2026
Merged

Support STREAMED response body#25
ronenkat merged 5 commits into
ms-llmd:mainfrom
Mohammad-nassar10:pr2-stream-response

Conversation

@Mohammad-nassar10

Copy link
Copy Markdown
Contributor

What type of PR is this?
/kind bug

What this PR does / why we need it:
Stacked on #24.
Getting a bug: Without per-chunk acks, Envoy stops forwarding response bytes after the first chunk, which surfaces to clients as ClientPayloadError: Not enough data to satisfy transfer length header.

  • pkg/handlers/server.go: ack every non-EoS response body chunk so Envoy keeps streaming.
  • pkg/handlers/response.go: always respond to response headers (in STREAMED mode Envoy blocks until we ack); on JSON parse failure, fall back to parseSSEResponseBody so streaming SSE responses still populate the response plugin pipeline; generateEmptyResponseBodyResponse now returns a single empty BodyResponse since the bytes were already forwarded by the per-chunk acks.
  • config/charts/payload-processor/templates/istio.yaml: switch response_body_mode to STREAMED so Istio actually installs the EnvoyFilter.

Which issue(s) this PR fixes:

Fixes #

Release note (write NONE if no user-facing change):

NONE

Signed-off-by: Mohammad <mohammad.nassar@ibm.com>
Signed-off-by: Mohammad <mohammad.nassar@ibm.com>
@Mohammad-nassar10 Mohammad-nassar10 changed the title Pr2 stream response Support STREAMED response body in ext-proc Jun 9, 2026
@Mohammad-nassar10 Mohammad-nassar10 changed the title Support STREAMED response body in ext-proc Support STREAMED response body Jun 9, 2026
@Mohammad-nassar10

Copy link
Copy Markdown
Contributor Author

/cc @davidbreitgand @ronenkat

@ronenkat

ronenkat commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

What is the relation to #4
is #4 needed? should we merge it?

@Mohammad-nassar10

Copy link
Copy Markdown
Contributor Author

This is replacing #4 (have part of the changes of #4 with more fixes).

@ronenkat

Copy link
Copy Markdown
Contributor

@Mohammad-nassar10 do you have a test that currently fails? and now passes with these changes?

Comment thread pkg/handlers/response.go Outdated
Comment thread pkg/handlers/response.go
Signed-off-by: Mohammad <mohammad.nassar@ibm.com>
@Mohammad-nassar10

Copy link
Copy Markdown
Contributor Author

@Mohammad-nassar10 do you have a test that currently fails? and now passes with these changes?

@ronenkat no test that fails on main and passes with this PR, The bug is noticed in real Envoy in STREAMED mode (when we run the benchmark).

Signed-off-by: Mohammad <mohammad.nassar@ibm.com>
Comment thread config/charts/payload-processor/templates/istio.yaml
@ronenkat

Copy link
Copy Markdown
Contributor

/approve

@ronenkat

Copy link
Copy Markdown
Contributor

/lgtm

@github-actions github-actions Bot added the lgtm label Jun 10, 2026
@ronenkat ronenkat merged commit d84bd44 into ms-llmd:main Jun 10, 2026
12 of 13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants