Skip to content

refactor: replace default network client with OkHttp #1180

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+11 −21
Open

refactor: replace default network client with OkHttp #1180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions .github/workflows/integration-test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ on:
- main

jobs:
check-rest:
check-rest-httpurlconnection:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
Expand All @@ -24,15 +24,15 @@ jobs:
- name: Set up Gradle
uses: gradle/actions/setup-gradle@v3

- run: ./gradlew :java:testRestSuite
- run: ./gradlew :java:testRestSuite -PhttpURLConnection

- uses: actions/upload-artifact@v4
if: always()
with:
name: java-build-reports-rest
path: java/build/reports/
Comment on lines 29 to 33
Copy link

Copilot AI Nov 21, 2025

Choose a reason for hiding this comment

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

The artifact names for the REST and Realtime test reports will conflict between the httpurlconnection and okhttp test jobs. When both check-rest-httpurlconnection and check-rest-okhttp jobs run, they will both try to upload artifacts with the name java-build-reports-rest, and similarly for the realtime jobs. This will cause one artifact to overwrite the other.

Consider making the artifact names unique by including the HTTP client variant in the name, for example:

  • java-build-reports-rest-httpurlconnection and java-build-reports-rest-okhttp
  • java-build-reports-realtime-httpurlconnection and java-build-reports-realtime-okhttp

Copilot uses AI. Check for mistakes.

check-realtime:
check-realtime-httpurlconnection:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
Expand All @@ -48,7 +48,7 @@ jobs:
- name: Set up Gradle
uses: gradle/actions/setup-gradle@v3

- run: ./gradlew :java:testRealtimeSuite
- run: ./gradlew :java:testRealtimeSuite -PhttpURLConnection

- uses: actions/upload-artifact@v4
if: always()
Expand All @@ -71,7 +71,7 @@ jobs:
- name: Set up Gradle
uses: gradle/actions/setup-gradle@v3

- run: ./gradlew :java:testRestSuite -Pokhttp
- run: ./gradlew :java:testRestSuite

check-realtime-okhttp:
runs-on: ubuntu-latest
Expand All @@ -89,7 +89,7 @@ jobs:
- name: Set up Gradle
uses: gradle/actions/setup-gradle@v3

- run: ./gradlew :java:testRealtimeSuite -Pokhttp
- run: ./gradlew :java:testRealtimeSuite

check-liveobjects:
runs-on: ubuntu-latest
Expand Down
12 changes: 1 addition & 11 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,17 +134,7 @@ You can add proxy support to the Ably Java SDK by configuring `ProxyOptions` in
<details>
<summary>Proxy support setup details.</summary>

To enable proxy support for both REST and Realtime clients in the Ably SDK, use the OkHttp library to handle HTTP requests and WebSocket connections.

Add the following dependency to your `build.gradle` file:

```groovy
dependencies {
runtimeOnly("io.ably:network-client-okhttp:1.4.2")
}
```

After adding the OkHttp dependency, enable proxy support by specifying proxy settings in the ClientOptions when initializing your Ably client.
Enable proxy support by specifying proxy settings in the ClientOptions when initializing your Ably client.

The following example sets up a proxy using the Pub/Sub Java SDK:

Expand Down
2 changes: 1 addition & 1 deletion android/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ dependencies {
compileOnly(libs.jetbrains)
testImplementation(libs.bundles.tests)
implementation(project(":network-client-core"))
runtimeOnly(project(":network-client-default"))
runtimeOnly(project(":network-client-okhttp"))
implementation(libs.firebase.messaging)
androidTestImplementation(libs.bundles.instrumental.android)
}
Expand Down
6 changes: 3 additions & 3 deletions java/build.gradle.kts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,10 @@ dependencies {
implementation(libs.bundles.common)
compileOnly(libs.jetbrains)
implementation(project(":network-client-core"))
if (findProperty("okhttp") == null) {
runtimeOnly(project(":network-client-default"))
} else {
if (findProperty("httpURLConnection") == null) {
runtimeOnly(project(":network-client-okhttp"))
} else {
runtimeOnly(project(":network-client-default"))
}
Comment on lines +25 to 29
Copy link

@coderabbitai coderabbitai bot Nov 21, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

Breaking change: OkHttp is now the default network client.

The conditional logic now defaults to OkHttp when the httpURLConnection property is absent. Users who expect the previous HttpURLConnection-based default will need to explicitly pass -PhttpURLConnection to get that behavior. This is a significant behavioral change that should be clearly documented in release notes and may require user migration guidance.

Verify the following before release:

  1. A migration notice is added to CHANGELOG.md documenting this breaking change
  2. Users who specifically need HttpURLConnection behavior understand the -PhttpURLConnection flag requirement
  3. Integration tests comprehensively validate both variants to catch compatibility issues early
  4. Any user-facing documentation is updated to mention OkHttp as the default HTTP client

Run this script to verify integration test coverage:

🏁 Script executed:

#!/bin/bash
# Check that integration tests cover both HTTP client variants
grep -n "httpURLConnection\|okhttp" .github/workflows/integration-test.yml | head -20

Length of output: 187

🏁 Script executed:

#!/bin/bash
# Check for CHANGELOG entries about HTTP client changes
fd -e CHANGELOG -e CHANGELOG.md -e changelog.md -e HISTORY -e NEWS | head -10

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Look for migration guides, release notes, or docs about HTTP client
fd -e md -e txt | xargs grep -l "httpURLConnection\|network.*client\|HTTP.*client" 2>/dev/null | grep -E "(CHANGELOG|README|MIGRATION|RELEASE|GUIDE|NEWS)" | head -20

Length of output: 0

🏁 Script executed:

#!/bin/bash
# Check if there's a README mentioning the default HTTP client
cat README.md 2>/dev/null | grep -A5 -B5 -i "http.*client\|okhttp\|network" | head -40

Length of output: 831

🏁 Script executed:

#!/bin/bash
# Find all possible documentation files
find . -type f -name "*CHANGE*" -o -name "*RELEASE*" -o -name "*NEWS*" 2>/dev/null | head -20

Length of output: 14

🏁 Script executed:

#!/bin/bash
# Check the build.gradle.kts file for comments explaining this logic
cat java/build.gradle.kts | head -40

Length of output: 1013

🏁 Script executed:

#!/bin/bash
# Look for any docs directory with HTTP client documentation
find . -type d -name "docs" -o -name "doc" 2>/dev/null | xargs ls -la 2>/dev/null | head -30

Length of output: 1796

🏁 Script executed:

#!/bin/bash
# Search CHANGELOG.md for HTTP client or httpURLConnection related entries
grep -n -A3 -B1 "httpURLConnection\|okhttp\|network.*client\|HTTP.*client" CHANGELOG.md | head -50

Length of output: 0

Breaking change: OkHttp is now the default network client — add documentation before release.

The conditional logic defaults to OkHttp when the httpURLConnection property is absent. Integration tests properly cover both variants, but the breaking change lacks critical documentation:

  • CHANGELOG.md: No entry documenting this behavioral change
  • Migration guidance: No documentation for users who specifically need HttpURLConnection (must pass -PhttpURLConnection)
  • User-facing docs: README.md doesn't mention the default HTTP client or how to switch between implementations

Users upgrading will unexpectedly receive OkHttp instead of HttpURLConnection without explicit migration instructions.

Add entries to CHANGELOG.md documenting the default change and add a section to README.md explaining HTTP client selection and the -PhttpURLConnection flag for users who need the legacy behavior.

🤖 Prompt for AI Agents 
In java/build.gradle.kts around lines 25-29 the build now defaults to the OkHttp
network client when the -PhttpURLConnection property is absent; update docs
before release: add a CHANGELOG.md entry that clearly states the default changed
to OkHttp (include version, rationale, and note that HttpURLConnection remains
available via -PhttpURLConnection), add or update README.md with a new "HTTP
client selection" section that explains the default is OkHttp, shows the exact
flag to restore legacy HttpURLConnection (gradle -PhttpURLConnection or CI env
guidance), provides a short migration/compatibility note for users (e.g., known
behavior differences and testing advice), and ensure release notes / docs
reference any integration test coverage and how to run with the alternate client
for verification.

testImplementation(libs.bundles.tests)
}
Expand Down
Loading