Conversation
Kehrlann
force-pushed
the
dgarnier/issue-620-refactor-http-client-chains
branch
from
August 7, 2026 16:14
16fee90 to
30dc4d3
Compare
9 tasks
Kehrlann
force-pushed
the
dgarnier/issue-620-refactor-http-client-chains
branch
from
August 27, 2026 11:49
30dc4d3 to
bf47603
Compare
Kehrlann
marked this pull request as ready for review
August 31, 2026 15:11
Kehrlann
force-pushed
the
dgarnier/issue-620-refactor-http-client-chains
branch
from
August 31, 2026 15:11
b897c12 to
bbb3330
Compare
…leaking the httpClient reference due to a cycle Signed-off-by: Daniel Garnier-Moiroux <git@garnier.wf> Signed-off-by: Dariusz Jędrzejczyk <2554306+chemicL@users.noreply.github.com>
Fixes #1042 Signed-off-by: Daniel Garnier-Moiroux <git@garnier.wf>
Signed-off-by: Daniel Garnier-Moiroux <git@garnier.wf>
Signed-off-by: Daniel Garnier-Moiroux <git@garnier.wf>
Signed-off-by: Daniel Garnier-Moiroux <git@garnier.wf>
- This discards all BodySubscribers to align with a6c88fccf50277ddf705765154baae4caa906e15, which introduced a publisher-based architecture instead of using subscribers. - Bounding the reads now happens within the reactive chains, e.g. in Utf8LineDecoder, decodeAggregateResponse and drain* methods. Signed-off-by: Daniel Garnier-Moiroux <git@garnier.wf>
Kehrlann
force-pushed
the
dgarnier/issue-620-refactor-http-client-chains
branch
from
September 23, 2026 12:14
9df07a5 to
f8271c2
Compare
Also renamed ResponseSubscribers, refactored test code Signed-off-by: Dariusz Jędrzejczyk <dariusz.jedrzejczyk@broadcom.com>
chemicL
force-pushed
the
dgarnier/issue-620-refactor-http-client-chains
branch
from
September 29, 2026 13:23
81a3dc9 to
6a01d9b
Compare
The SSE parser now follows the SSE spec: an event's type resets after each event and defaults to "message", and an empty id clears the last event id. Before, the type carried over from the previous event, so a message following an "endpoint" event was taken for another endpoint. Behaviour changes compared to main: - Errors caused by the server (non-2xx responses, unknown content types, malformed messages) are McpTransportException instead of plain RuntimeException. Error messages no longer include the response body. - A 400 on the streamable GET stream no longer invalidates the session. - A non-2xx SSE connect with an empty body now fails instead of hanging, and sendMessage() completes when the server closes an SSE response without sending any events. - Errors that happen after connect() or sendMessage() has completed now reach the exception handler and stop showing up as Reactor onErrorDropped logs. Signed-off-by: Dariusz Jędrzejczyk <dariusz.jedrzejczyk@broadcom.com>
…nt session id Signed-off-by: Dariusz Jędrzejczyk <dariusz.jedrzejczyk@broadcom.com>
Signed-off-by: Dariusz Jędrzejczyk <dariusz.jedrzejczyk@broadcom.com>
Signed-off-by: Dariusz Jędrzejczyk <dariusz.jedrzejczyk@broadcom.com>
Member
|
I re-visited this PR @Kehrlann with some commits. Thank you for refreshing it and integrating it with the recent changes! Here's a summary of what is addressed here altogether: Reliability
Errors surface immediately instead of as timeouts
Performance
SSE parsing (now follows the spec)
Memory safety
API
|
Signed-off-by: Dariusz Jędrzejczyk <dariusz.jedrzejczyk@broadcom.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refactor HttpClient use to avoid capturing the sseSink that leads to leaking the httpClient reference due to a cycle (see #620).
This rewrites the
HttpClientstreaming support from aBodySubscriberpushing messages to aPublisher-based approach.This avoids the
HttpClientcyclic reference we had in the previous implementation. The tradeoff is that we have to perform line parsing manually with aUtf8LineDecoderwe maintain.We can batch this rework with performance fixes reading large SSE events (see #1042)
Fixes #1042
Fixes #620
(Rebased @chemicL 's work on issue #620)