Skip to content

fix: harden Horizon SSE stream handling#791

Merged
overcat merged 1 commit intomasterfrom
chore/sse-public-api
Apr 22, 2026
Merged

fix: harden Horizon SSE stream handling#791
overcat merged 1 commit intomasterfrom
chore/sse-public-api

Conversation

@overcat
Copy link
Copy Markdown
Member

@overcat overcat commented Apr 21, 2026

No description provided.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Refactors Horizon SSE streaming to use OkHttp’s public EventSources factory instead of the internal RealEventSource, reducing reliance on OkHttp internal APIs.

Changes:

  • Remove okhttp3.internal.sse.RealEventSource usage/import.
  • Create SSE streams via EventSources.createFactory(okHttpClient).newEventSource(...).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/main/java/org/stellar/sdk/requests/SSEStream.java
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 21, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.01%. Comparing base (6df16f8) to head (9e8b3a9).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@             Coverage Diff              @@
##             master     #791      +/-   ##
============================================
+ Coverage     81.42%   83.01%   +1.58%     
- Complexity     1473     1488      +15     
============================================
  Files           220      220              
  Lines          5530     5529       -1     
  Branches        568      568              
============================================
+ Hits           4503     4590      +87     
+ Misses          717      616     -101     
- Partials        310      323      +13     
Files with missing lines Coverage Δ
.../main/java/org/stellar/sdk/requests/SSEStream.java 81.55% <100.00%> (+81.55%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

- use OkHttp's public SSE factory instead of the internal RealEventSource

- add more test cases
@overcat overcat force-pushed the chore/sse-public-api branch from 22b5257 to 9e8b3a9 Compare April 22, 2026 00:36
@overcat overcat requested a review from Copilot April 22, 2026 00:39
@overcat overcat changed the title refactor: use okhttp public SSE factory instead of internal RealEventSource fix: harden Horizon SSE stream handling Apr 22, 2026
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refactors Horizon SSE streaming to use OkHttp’s public SSE API (EventSources) instead of relying on OkHttp’s internal RealEventSource, and adds focused tests to validate streaming, reconnect, and failure behaviors.

Changes:

  • Switch SSE stream creation to EventSources.createFactory(okHttpClient).newEventSource(...).
  • Force SSE requests to bypass caches via CacheControl.FORCE_NETWORK.
  • Add a comprehensive SSEStreamTest suite covering parsing, resume behavior, and failure propagation.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/main/java/org/stellar/sdk/requests/SSEStream.java Uses OkHttp public SSE factory and forces network (no-cache) for streaming requests; clarifies liveness behavior.
src/test/kotlin/org/stellar/sdk/requests/SSEStreamTest.kt Adds MockWebServer-based tests for SSE event parsing, reconnect/resume, and failure handling.
CHANGELOG.md Notes the fix/refactor in the Pending section.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@overcat overcat merged commit 77e6d9c into master Apr 22, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants