build: update OkHttp to 5.3.0 stable#9
Conversation
📝 WalkthroughWalkthroughThe ChangesOkHttp Version Upgrade with Internal API Migration
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~5 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
Comment |
|
@coderabbitai full review Please focus on OkHttp 5.x API compatibility and ProGuard/R8 behavior. Note on the timeout-API suggestion: OkHttp 5.x retains the (long, TimeUnit) timeout overloads, so the existing TimeUnit.SECONDS calls compile and run fine. Verified by clean debug AND release builds (R8 shrinking included). Migrating to kotlin.time.Duration is optional and would touch unrelated files, so it's intentionally out of scope for this dependency bump. |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== app/build.gradle.kts (OkHttp dependency line) ==="
grep -n -i "okhttp" app/build.gradle.kts || echo "Not found"
echo ""
echo "=== All OkHttp imports across the codebase ==="
rg -n "okhttp3" --type kotlin --type java -A1
echo ""
echo "=== All files that import OkHttp ==="
rg -l "okhttp3" --type kotlin --type java
echo ""
echo "=== OkHttpClient builder usages ==="
rg -n "OkHttpClient" --type kotlin --type java -C3
echo ""
echo "=== Timeout usages ==="
rg -n "Timeout\|connectTimeout\|readTimeout\|writeTimeout\|callTimeout" --type kotlin --type java -C2
echo ""
echo "=== Credentials.basic usages ==="
rg -n "Credentials" --type kotlin --type java -C3
echo ""
echo "=== MediaType usages ==="
rg -n "MediaType\|toMediaType\|toMediaTypeOrNull" --type kotlin --type java -C3
echo ""
echo "=== RequestBody usages ==="
rg -n "RequestBody\|toRequestBody" --type kotlin --type java -C3
echo ""
echo "=== HttpUrl usages ==="
rg -n "HttpUrl" --type kotlin --type java -C3
echo ""
echo "=== Any deprecated API usages ==="
rg -n "\.url\(|\.get\(\)\|\.post\(\)\|\.patch\(\)\|\.delete\(\)\|\.put\(\)" --type kotlin --type java -C2
echo ""
echo "=== ProGuard / R8 rules ==="
fd -e pro -e txt -e rules | head -20
fd "proguard" | head -20
echo ""
echo "=== ProGuard rules content ==="
fd "proguard" | xargs cat 2>/dev/null || echo "No proguard files found via fd"
echo ""
echo "=== consumer-rules and other R8 config ==="
fd "consumer-rules\|proguard-rules\|r8" | xargs cat 2>/dev/null || echo "none"
echo ""
echo "=== build.gradle.kts R8/minify config ==="
grep -n -i "minif\|proguard\|r8\|shrink\|obfuscat" app/build.gradle.kts || echo "Not found"
echo ""
echo "=== Full app/build.gradle.kts ==="
cat app/build.gradle.ktsLength of output: 13396 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== ConfigurationFragment.kt closeQuietly usage ==="
grep -n "closeQuietly\|internal" app/src/main/java/io/nekohasekai/sagernet/ui/ConfigurationFragment.kt -A3 -B3
echo ""
echo "=== Helpers.kt okhttp3 keep rule context ==="
sed -n '75,100p' buildSrc/src/main/kotlin/Helpers.kt
echo ""
echo "=== BackupFragment.kt OkHttpClient usage (full context) ==="
sed -n '190,280p' app/src/main/java/io/nekohasekai/sagernet/ui/BackupFragment.kt
echo ""
echo "=== BackupFragment.kt restore section ==="
sed -n '325,400p' app/src/main/java/io/nekohasekai/sagernet/ui/BackupFragment.kt
echo ""
echo "=== WebDAVSettingsActivity.kt OkHttpClient builder full context ==="
sed -n '125,180p' app/src/main/java/io/nekohasekai/sagernet/ui/WebDAVSettingsActivity.kt
echo ""
echo "=== Full Helpers.kt (R8/keep/proguard section) ==="
cat buildSrc/src/main/kotlin/Helpers.kt
echo ""
echo "=== app/build.gradle.kts full content ==="
cat app/build.gradle.kts
echo ""
echo "=== Any proguard-rules.pro files ==="
fd "proguard-rules" | xargs cat 2>/dev/null || echo "none"
echo ""
echo "=== consumer-rules.pro files ==="
fd "consumer-rules" | xargs cat 2>/dev/null || echo "none"Length of output: 26285
Full Review: OkHttp 5.0.0-alpha.3 → 5.4.0✅ API Compatibility — All Public APIsEvery OkHttp symbol used across the 22 affected files is part of the stable public surface and unchanged across the 5.x line:
|
Move off the pre-release alpha to the current stable OkHttp 5.x line. 5.3.0 is the latest stable whose okhttp-android artifact still supports compileSdk 35 (5.4.0 requires minCompileSdk=36). The network-client API used (OkHttpClient, newCall/execute, Request.Builder, Credentials.basic, toMediaType/toRequestBody, HttpUrl) is stable across 5.x; no source changes.
okhttp3.internal.closeQuietly is an unstable internal API. Replace it with a small private Closeable.closeQuietly() extension (same semantics) so the OkHttp upgrade path stays robust.
3c311e7 to
97efe89
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/main/java/io/nekohasekai/sagernet/ui/ConfigurationFragment.kt`:
- Around line 2060-2064: The closeQuietly() extension function currently catches
and suppresses all Exception types, which masks RuntimeException and other
programmer errors. Modify the exception handling in closeQuietly() to explicitly
catch and re-throw RuntimeException instances while continuing to silently
suppress checked exceptions like IOException. This matches OkHttp's safer
behavior by allowing real programming errors to surface while safely suppressing
recoverable IO errors.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: fb8e6676-a986-4e2d-9fb6-c1df0b20f729
📒 Files selected for processing (1)
app/src/main/java/io/nekohasekai/sagernet/ui/ConfigurationFragment.kt
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/build.gradle.kts`:
- Line 68: Update the OkHttp dependency version in the implementation
declaration from 5.3.0 to 5.4.0. Locate the com.squareup.okhttp3:okhttp
dependency line and change the version number at the end from 5.3.0 to 5.4.0 to
align with the PR objective of upgrading to the latest OkHttp release.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 75904c3e-277b-4054-afff-f1ea1ee7be4b
📒 Files selected for processing (2)
app/build.gradle.ktsapp/src/main/java/io/nekohasekai/sagernet/ui/ConfigurationFragment.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/io/nekohasekai/sagernet/ui/ConfigurationFragment.kt
|
@coderabbitai review Branch updated since last review:
Both debug and release builds pass on the builder. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== app/build.gradle.kts OkHttp dependency ==="
grep -n -i "okhttp\|compileSdk\|minSdk" app/build.gradle.kts
echo ""
echo "=== ConfigurationFragment.kt closeQuietly implementation ==="
grep -n "closeQuietly\|Closeable\|import okhttp3" app/src/main/java/io/nekohasekai/sagernet/ui/ConfigurationFragment.kt -A2 -B2
echo ""
echo "=== ConfigurationFragment.kt lines around new extension function ==="
# Find the new local closeQuietly definition
grep -n "fun Closeable\|closeQuietly\|private fun\|Exception\|ignore" app/src/main/java/io/nekohasekai/sagernet/ui/ConfigurationFragment.kt | head -30
echo ""
echo "=== Verify no remaining okhttp3.internal imports anywhere ==="
rg -n "okhttp3.internal" --type kotlin --type java
echo ""
echo "=== Full build.gradle.kts compileSdk / minSdk section ==="
grep -n "compileSdk\|minSdk\|targetSdk\|buildToolsVersion" app/build.gradle.ktsLength of output: 4385
Review: OkHttp bump +
|
Summary
Moves OkHttp off the pre-release
5.0.0-alpha.3to stable5.3.0, and removes anunstable OkHttp-internal API usage.
Why 5.3.0 and not 5.4.0
OkHttp 5.4.0's
okhttp-androidartifact declaresminCompileSdk=36, but thisproject compiles against
android-35. Verified from the published AAR metadata:okhttp-android5.0.0–5.3.0:minCompileSdk=1okhttp-android5.4.0:minCompileSdk=365.3.0 is the latest stable that works with the current
compileSdk 35. Moving to 5.4.0is deferred until a separate
compileSdk 36bump in the build-stack track (out of scopefor a dependency update).
API compatibility
The OkHttp surface used here is stable across 5.x:
OkHttpClient/Builder,newCall().execute(),Request.Builder,Credentials.basic,toMediaType/toRequestBody,HttpUrl. No source changes needed for the bump itself.Internal API cleanup (commit 2)
Replaced
okhttp3.internal.closeQuietly(an unstableokhttp3.internalAPI used on aZipInputStreamand aSocketinConfigurationFragment.kt) with a small privateCloseable.closeQuietly()extension of identical semantics — so future OkHttp upgradescan't break on an internal API removal. Flagged by review; in scope as OkHttp-robustness.
Verification (AWS Linux builder)
./gradlew app:assembleOssDebug app:assembleOssRelease— BUILD SUCCESSFUL.Release exercises R8 shrinking (OkHttp consumer ProGuard rules) and lintVital with
warningsAsErrors=true. ThecheckAarMetadatatask (which rejected 5.4.0) passes for 5.3.0.Scope
app/build.gradle.kts(1 line) +ConfigurationFragment.kt(import swap + tiny helper).No other dependencies, no compileSdk change.
Runtime testing note
Build-verified only (emulator blocked by EC2 vCPU quota; increase pending). OkHttp powers
subscription fetch, WebDAV backup/restore, and asset downloads — batched runtime QA planned.