feature: implement proxy connectivity testing and improve Android sys…#405
Merged
rainxchzed merged 2 commits intomainfrom Apr 10, 2026
Merged
feature: implement proxy connectivity testing and improve Android sys…#405rainxchzed merged 2 commits intomainfrom
rainxchzed merged 2 commits intomainfrom
Conversation
…tem proxy resolution - Add `ProxyTester` interface and `ProxyTesterImpl` to verify network reachability through a configured proxy using the GitHub API. - Implement a "Test" button in the network settings UI to trigger proxy validation and display latency or specific error messages (DNS, timeout, auth, etc.). - Add localized strings for proxy test outcomes in English, Arabic, Bengali, Spanish, French, Hindi, Italian, Japanese, Korean, Polish, Russian, Turkish, and Simplified Chinese. - Improve Android system proxy support by explicitly resolving `https.proxyHost` and `http.proxyHost` properties, ensuring traffic (including downloads) correctly follows device-level proxy settings. - Update `TweaksViewModel` and `TweaksState` to manage proxy testing logic and loading states.
Contributor
WalkthroughAdded a proxy connectivity test feature: domain interface and outcome types, data-layer tester implementation and Android proxy resolution, DI binding, localized strings (13 locales), and Tweaks UI/VM/state/event wiring including a test button and result snackbars. Changes
Sequence DiagramsequenceDiagram
actor User
participant UI as "Network UI\n(Composable)"
participant VM as "TweaksViewModel"
participant Tester as "ProxyTester / ProxyTesterImpl"
participant Client as "Ktor/OkHttp Client"
participant Server as "Remote Server"
User->>UI: Click "Proxy test"
UI->>VM: dispatch OnProxyTest
VM->>VM: validate host/port
VM->>VM: build ProxyConfig, set isProxyTestInProgress=true
VM->>Tester: test(config)
Tester->>Client: create client with proxy + timeouts
Client->>Server: GET test URL (via proxy)
alt HTTP 200-299
Server-->>Client: 2xx response
Client-->>Tester: response received
Tester->>Tester: measure latency
Tester-->>VM: Success(latencyMs)
else HTTP 407
Server-->>Client: 407 Proxy Authentication Required
Client-->>Tester: 407 response
Tester-->>VM: Failure.ProxyAuthRequired
else Network/Timeout/Error
Client--xTester: exception (DNS/Connect/Timeout/IO)
Tester->>Tester: map exception to Failure.*
Tester-->>VM: Failure.<type>
end
VM->>VM: convert outcome -> event, set isProxyTestInProgress=false
VM->>UI: emit OnProxyTestSuccess or OnProxyTestError
UI->>User: show snackbar with result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.claude/settings.local.json:
- Line 4: Replace the machine-specific absolute path in the command string
"Bash(grep -r \"\\.FLATPAK\\|Flatpak\\|flatpak\" D:/development/Github-Store
--include=*.kt)" with a repo-relative path (e.g., use "." or "${repoRoot}") so
the command becomes portable across environments; update the string in
.claude/settings.local.json to use the relative target and keep the rest of the
grep flags identical.
In
`@core/data/src/androidMain/kotlin/zed/rainxch/core/data/network/HttpClientFactory.android.kt`:
- Around line 84-93: The proxy port values (httpsPort/httpPort) are parsed with
toIntOrNull but not range-validated, so invalid ports (e.g. -1 or >65535) can
cause InetSocketAddress to throw; update the logic in
HttpClientFactory.android.kt around httpsHost/httpsPort and httpHost/httpPort to
accept the parsed Int only if it is within 0..65535 (e.g., check httpsPort in
0..65535 and httpPort in 0..65535 before returning Proxy(Proxy.Type.HTTP,
InetSocketAddress(...))), and only create/return the Proxy when both host is
non-blank and port is in that valid range.
In
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ProxyTesterImpl.kt`:
- Around line 59-60: The catch block in ProxyTesterImpl.kt currently catches
Throwable and converts fatal errors into ProxyTestOutcome.Failure.Unknown;
change the catch to catch (e: Exception) so only recoverable exceptions are
caught and fatal errors (e.g., OutOfMemoryError, StackOverflowError) will
propagate; update the catch clause that returns ProxyTestOutcome.Failure.Unknown
to use Exception instead of Throwable and leave the rest of the handling for
ProxyTestOutcome.Failure.Unknown unchanged.
- Around line 11-14: ProxyTesterImpl currently imports JVM-only exceptions
(IOException, ConnectException, SocketTimeoutException, UnknownHostException) in
commonMain and uses them in its catch blocks; remove those java.* imports and
the JVM-specific catches from
core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ProxyTesterImpl.kt,
create an expect/actual mapping or platform-specific helper (e.g., expect fun
mapNetworkException(e: Throwable): ProxyTestResult or expect class
NetworkExceptionMapper with actual implementations in androidMain/jvmMain) and
replace the direct exception checks in the ProxyTesterImpl test method with a
call to that platform mapper (or use Ktor-native exception types if available)
so that platform-specific exception handling lives in actual implementations
while commonMain only depends on the abstract mapping API.
In
`@feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.kt`:
- Around line 580-583: In TweaksViewModel, when handling
ProxyTestOutcome.Failure.Unknown, stop passing the platform-specific raw message
into TweaksEvent.OnProxyTestError; always use the localized fallback via
getString(Res.string.proxy_test_error_unknown) and, if you need diagnostics, log
the raw message separately (e.g., using logger.debug or processLogger) rather
than forwarding it to the UI. Ensure the branch that matches
ProxyTestOutcome.Failure.Unknown constructs the event with
getString(Res.string.proxy_test_error_unknown) only and move any use of the
original message into a non-UI log call.
- Around line 408-418: The OnProxyTest handler currently wraps the suspending
proxyTester.test(config) in runCatching which swallows CancellationException and
also fails to reliably reset isProxyTestInProgress; replace the runCatching
block inside viewModelScope.launch with an explicit try { val outcome =
proxyTester.test(config); _events.send(outcome.toEvent()) } catch (e:
CancellationException) { throw e } catch (t: Throwable) {
_events.send(ProxyTestOutcome.Failure.Unknown(t.message).toEvent()) } finally {
_state.update { it.copy(isProxyTestInProgress = false) } } (use the existing
symbols TweaksAction.OnProxyTest, buildProxyConfigForTest(),
proxyTester.test(config), _state and _events.send) so cancellation is propagated
and the in-progress flag is always cleared; optionally track/cancel the Job if
you need to prevent late results from earlier jobs.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 3d734487-86f9-4cf2-8753-7843c1b0220d
📒 Files selected for processing (27)
.claude/settings.local.jsoncomposeApp/release/baselineProfiles/0/composeApp-release.dmcomposeApp/release/baselineProfiles/1/composeApp-release.dmcore/data/src/androidMain/kotlin/zed/rainxch/core/data/network/HttpClientFactory.android.ktcore/data/src/androidMain/kotlin/zed/rainxch/core/data/services/AndroidDownloader.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/di/SharedModule.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ProxyTesterImpl.ktcore/domain/src/commonMain/kotlin/zed/rainxch/core/domain/network/ProxyTester.ktcore/presentation/src/commonMain/composeResources/values-ar/strings-ar.xmlcore/presentation/src/commonMain/composeResources/values-bn/strings-bn.xmlcore/presentation/src/commonMain/composeResources/values-es/strings-es.xmlcore/presentation/src/commonMain/composeResources/values-fr/strings-fr.xmlcore/presentation/src/commonMain/composeResources/values-hi/strings-hi.xmlcore/presentation/src/commonMain/composeResources/values-it/strings-it.xmlcore/presentation/src/commonMain/composeResources/values-ja/strings-ja.xmlcore/presentation/src/commonMain/composeResources/values-ko/strings-ko.xmlcore/presentation/src/commonMain/composeResources/values-pl/strings-pl.xmlcore/presentation/src/commonMain/composeResources/values-ru/strings-ru.xmlcore/presentation/src/commonMain/composeResources/values-tr/strings-tr.xmlcore/presentation/src/commonMain/composeResources/values-zh-rCN/strings-zh-rCN.xmlcore/presentation/src/commonMain/composeResources/values/strings.xmlfeature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksAction.ktfeature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksEvent.ktfeature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksRoot.ktfeature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksState.ktfeature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.ktfeature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/components/sections/Network.kt
core/data/src/androidMain/kotlin/zed/rainxch/core/data/network/HttpClientFactory.android.kt
Show resolved
Hide resolved
Comment on lines
+11
to
+14
| import java.io.IOException | ||
| import java.net.ConnectException | ||
| import java.net.SocketTimeoutException | ||
| import java.net.UnknownHostException |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Declared Kotlin targets =="
rg -n --glob '**/*.gradle.kts' --glob '**/*.gradle' 'ios\(|macos|linuxX64|mingwX64|js\(|wasm|androidTarget|jvm\('
echo
echo "== JVM imports inside commonMain =="
rg -n --glob '**/src/commonMain/**/*.kt' '^import java\.'Repository: OpenHub-Store/GitHub-Store
Length of output: 99
🏁 Script executed:
# Confirm the file exists and view its imports
cat -n "core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ProxyTesterImpl.kt" | head -20Repository: OpenHub-Store/GitHub-Store
Length of output: 995
🏁 Script executed:
# Search for Kotlin Multiplatform configuration
find . -name "build.gradle.kts" -o -name "build.gradle" -o -name "settings.gradle.kts" | head -10Repository: OpenHub-Store/GitHub-Store
Length of output: 412
🏁 Script executed:
# Check for kotlin block in any gradle files more broadly
rg -l "kotlin\s*\{" --glob "**/*.gradle.kts" --glob "**/*.gradle"Repository: OpenHub-Store/GitHub-Store
Length of output: 1434
🏁 Script executed:
# Read the core/data/build.gradle.kts to check KMP target configuration
cat "core/data/build.gradle.kts"Repository: OpenHub-Store/GitHub-Store
Length of output: 1285
🏁 Script executed:
# Check the convention plugin definition for KMP library
cat "build-logic/convention/build.gradle.kts"Repository: OpenHub-Store/GitHub-Store
Length of output: 2739
🏁 Script executed:
# Also search for where the convention plugin targets are defined
find "build-logic" -name "*.gradle.kts" -o -name "*.kt" | grep -i kmp | head -5Repository: OpenHub-Store/GitHub-Store
Length of output: 138
🏁 Script executed:
# Read the KMP Library convention plugin
cat "build-logic/convention/src/main/kotlin/KmpLibraryConventionPlugin.kt"Repository: OpenHub-Store/GitHub-Store
Length of output: 1325
🏁 Script executed:
# Search for the configureKotlinMultiplatform function
rg -n "fun configureKotlinMultiplatform" "build-logic/convention/src/main/kotlin/"Repository: OpenHub-Store/GitHub-Store
Length of output: 52
🏁 Script executed:
# Look for convention files that define multiplatform config
fd "\.kt$" "build-logic/convention/src/main/kotlin/" | xargs grep -l "kotlin\s*\{" | head -10Repository: OpenHub-Store/GitHub-Store
Length of output: 88
🏁 Script executed:
# Search for configureKotlinMultiplatform more broadly
rg "configureKotlinMultiplatform" "build-logic/convention/src/main/kotlin/"Repository: OpenHub-Store/GitHub-Store
Length of output: 643
🏁 Script executed:
# List all .kt files in the convention directory
fd "\.kt$" "build-logic/convention/src/main/kotlin/" | sortRepository: OpenHub-Store/GitHub-Store
Length of output: 1352
🏁 Script executed:
# Read the KotlinMultiplatform.kt file
cat "build-logic/convention/src/main/kotlin/zed/rainxch/githubstore/convention/KotlinMultiplatform.kt"Repository: OpenHub-Store/GitHub-Store
Length of output: 900
🏁 Script executed:
# Read the full ProxyTesterImpl.kt to see how java.* exceptions are used
cat "core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ProxyTesterImpl.kt"Repository: OpenHub-Store/GitHub-Store
Length of output: 2699
🏁 Script executed:
# Search for other ProxyTester implementations
rg -n "class.*ProxyTester" --glob "**/*.kt"Repository: OpenHub-Store/GitHub-Store
Length of output: 191
🏁 Script executed:
# Check if these java.* exceptions are actually needed or if Ktor exceptions would suffice
rg -n "SocketTimeoutException|UnknownHostException|ConnectException|IOException" "core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ProxyTesterImpl.kt"Repository: OpenHub-Store/GitHub-Store
Length of output: 394
Move JVM-specific exception handling out of commonMain.
Lines 11-14 import java.* exceptions in commonMain, violating the platform API guideline. These imports are actively used in catch blocks (lines 49, 53, 55, 57). Move JVM-only exception mapping to androidMain and jvmMain via expect/actual pattern, or replace with Ktor-native exception handling where available.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ProxyTesterImpl.kt`
around lines 11 - 14, ProxyTesterImpl currently imports JVM-only exceptions
(IOException, ConnectException, SocketTimeoutException, UnknownHostException) in
commonMain and uses them in its catch blocks; remove those java.* imports and
the JVM-specific catches from
core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ProxyTesterImpl.kt,
create an expect/actual mapping or platform-specific helper (e.g., expect fun
mapNetworkException(e: Throwable): ProxyTestResult or expect class
NetworkExceptionMapper with actual implementations in androidMain/jvmMain) and
replace the direct exception checks in the ProxyTesterImpl test method with a
call to that platform mapper (or use Ktor-native exception types if available)
so that platform-specific exception handling lives in actual implementations
while commonMain only depends on the abstract mapping API.
core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ProxyTesterImpl.kt
Outdated
Show resolved
Hide resolved
...tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.kt
Show resolved
Hide resolved
...tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.kt
Outdated
Show resolved
Hide resolved
…uration - Refactor `TweaksViewModel` to explicitly handle `CancellationException` during proxy tests to preserve structured concurrency. - Update `TweaksViewModel` to use localized fallback strings for unknown proxy test errors instead of leaking raw exception messages. - Strengthen proxy port validation in `resolveAndroidSystemProxy` to ensure values are within the valid 1-65535 range before creating `InetSocketAddress`. - Narrow exception catching in `ProxyTesterImpl` from `Throwable` to `Exception`. - Update `.claude/settings.local.json` to use relative paths in grep commands and add a combined Gradle build task.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ProxyTesterImpl.kt (1)
11-14:⚠️ Potential issue | 🔴 CriticalMove JVM exception mapping out of
commonMain.
java.*imports at Line 11–14 and JVM-specific catches at Line 49–58 couple shared code to JVM APIs. KeepProxyTesterImplin common by delegating exception mapping via anexpect/actualmapper (actuals inandroidMain/jvmMain), or map only common/Ktor exceptions in this layer.As per coding guidelines: Shared code must be in
commonMainsource set and use expect/actual for platform-specific APIs.#!/bin/bash set -euo pipefail echo "== JVM imports inside commonMain ==" rg -n --glob '**/src/commonMain/**/*.kt' '^import java\.' echo echo "== Declared multiplatform targets ==" rg -n --glob '**/*.gradle.kts' 'androidTarget|jvm\(|ios\(|macos|linuxX64|mingwX64|js\(|wasm'Also applies to: 49-58
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1e9225fd-deb8-4fae-9715-608707592054
📒 Files selected for processing (4)
.claude/settings.local.jsoncore/data/src/androidMain/kotlin/zed/rainxch/core/data/network/HttpClientFactory.android.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/network/ProxyTesterImpl.ktfeature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.kt
🚧 Files skipped from review as they are similar to previous changes (2)
- .claude/settings.local.json
- feature/tweaks/presentation/src/commonMain/kotlin/zed/rainxch/tweaks/presentation/TweaksViewModel.kt
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
1 participant
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.
…tem proxy resolution
ProxyTesterinterface andProxyTesterImplto verify network reachability through a configured proxy using the GitHub API.https.proxyHostandhttp.proxyHostproperties, ensuring traffic (including downloads) correctly follows device-level proxy settings.TweaksViewModelandTweaksStateto manage proxy testing logic and loading states.Summary by CodeRabbit
New Features
Bug Fixes