Route anon GitHub reads through backend pool#505
Merged
Conversation
…se Retry-After on 429, share fallback policy
Contributor
WalkthroughThe PR integrates backend API client support across multiple feature modules with fallback to GitHub. It extends network DTOs with new fields, implements enhanced rate-limit tracking, establishes a centralized fallback policy, and updates repositories (StarredRepositoryImpl, AppsRepositoryImpl, DeveloperProfileRepositoryImpl) to query the backend first, falling back to GitHub on non-retriable errors. ChangesBackend Integration with Fallback Pattern
Sequence DiagramsequenceDiagram
participant Client
participant BackendAPI as Backend API
participant FallbackPolicy as Fallback Policy
participant GitHubAPI as GitHub API
Client->>BackendAPI: fetch repo/user/releases
BackendAPI-->>Client: response or error
alt Success Response
Client->>Client: map DTO to domain
Note over Client: Return result
else HTTP 429 (Rate Limited)
Client->>Client: parse Retry-After<br/>& Reset headers
Client->>Client: throw RateLimitedException<br/>(with timing)
Client->>FallbackPolicy: shouldFallbackToGithubOrRethrow()
alt Rate Limit (non-fallback)
FallbackPolicy-->>Client: throw domain RateLimitException
end
else HTTP 5xx (Server Error)
Client->>FallbackPolicy: shouldFallbackToGithubOrRethrow()
FallbackPolicy-->>Client: return true (allow fallback)
Client->>GitHubAPI: fetch from GitHub
GitHubAPI-->>Client: response
Client->>Client: map to domain
Note over Client: Return GitHub result
else Other Error
Client->>FallbackPolicy: shouldFallbackToGithubOrRethrow()
FallbackPolicy-->>Client: throw exception
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
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
🧪 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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/apps/data/src/commonMain/kotlin/zed/rainxch/apps/data/repository/AppsRepositoryImpl.kt (1)
98-119:⚠️ Potential issue | 🔴 CriticalRethrow coroutine cancellation in these fallback paths.
Lines 98, 161, and 211 currently fall through to
catch (e: Exception), which also catchesCancellationException. That turns a cancelled coroutine intonulland lets the call keep going after its parent scope has been cancelled.All three functions (
getLatestRelease,fetchRepoInfo, andresolveLatestTagViaReleases) are suspend functions where this matters.Suggested fix
+import kotlinx.coroutines.CancellationException import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.first ... } catch (e: RateLimitException) { throw e + } catch (e: CancellationException) { + throw e } catch (e: Exception) { logger.error("Failed to fetch latest release for $owner/$repo: ${e.message}") null } ... } catch (e: RateLimitException) { throw e + } catch (e: CancellationException) { + throw e } catch (e: Exception) { logger.error("Failed to fetch repo info for $owner/$repo: ${e.message}") null } ... + } catch (e: CancellationException) { + throw e } catch (_: Exception) { null }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@feature/apps/data/src/commonMain/kotlin/zed/rainxch/apps/data/repository/AppsRepositoryImpl.kt` around lines 98 - 119, The generic Exception catch blocks in suspend functions getLatestRelease, fetchRepoInfo, and resolveLatestTagViaReleases swallow CancellationException; modify each function to rethrow coroutine cancellations by adding an early check to if (e is CancellationException) throw e before handling/logging other exceptions so that cancellations propagate correctly (i.e., in each catch (e: Exception) block, rethrow when e is CancellationException, then perform logger.error and return the fallback/null as before).
🧹 Nitpick comments (1)
core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/BackendFallbackPolicy.kt (1)
6-6: 💤 Low valuePrefer
kotlinx.datetime.Clockfor consistency with the rest of the codebase.
kotlin.time.Clockis stable stdlib and produces the correctepochSecondsvalue, but the project's convention useskotlinx.datetime.Clock(e.g.,RateLimitInfo.kt). Inkotlinx-datetime0.6+ the two are interchangeable (kotlinx.datetime.Instantis a typealias forkotlin.time.Instant), so this is purely a consistency issue.♻️ Proposed change
-import kotlin.time.Clock +import kotlinx.datetime.Clock
Clock.System.now().epochSecondsusage on line 23 stays unchanged.🤖 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/BackendFallbackPolicy.kt` at line 6, Replace the stdlib Clock import with the kotlinx.datetime variant to match project conventions: change import kotlin.time.Clock to import kotlinx.datetime.Clock and keep using Clock.System.now().epochSeconds in BackendFallbackPolicy (the Clock.System.now().epochSeconds usage should remain unchanged).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@feature/dev-profile/data/src/commonMain/kotlin/zed/rainxch/devprofile/data/repository/DeveloperProfileRepositoryImpl.kt`:
- Around line 196-215: The backend-path currently returns stableRelease.tagName
even when no installable asset exists, causing inconsistency with the GitHub
fallback; update the backend success branch (inside the backendResult.fold
onSuccess lambda in DeveloperProfileRepositoryImpl) to only return the release
tag when hasInstallable is true (e.g., return Triple(true, hasInstallable, if
(hasInstallable) stableRelease.tagName else null)) so latestVersion is null for
unsupported platforms just like the GitHub branch.
---
Outside diff comments:
In
`@feature/apps/data/src/commonMain/kotlin/zed/rainxch/apps/data/repository/AppsRepositoryImpl.kt`:
- Around line 98-119: The generic Exception catch blocks in suspend functions
getLatestRelease, fetchRepoInfo, and resolveLatestTagViaReleases swallow
CancellationException; modify each function to rethrow coroutine cancellations
by adding an early check to if (e is CancellationException) throw e before
handling/logging other exceptions so that cancellations propagate correctly
(i.e., in each catch (e: Exception) block, rethrow when e is
CancellationException, then perform logger.error and return the fallback/null as
before).
---
Nitpick comments:
In
`@core/data/src/commonMain/kotlin/zed/rainxch/core/data/network/BackendFallbackPolicy.kt`:
- Line 6: Replace the stdlib Clock import with the kotlinx.datetime variant to
match project conventions: change import kotlin.time.Clock to import
kotlinx.datetime.Clock and keep using Clock.System.now().epochSeconds in
BackendFallbackPolicy (the Clock.System.now().epochSeconds usage should remain
unchanged).
🪄 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: 1e2afb19-31de-4eda-8bd5-348e94f8a90c
📒 Files selected for processing (12)
core/data/src/commonMain/kotlin/zed/rainxch/core/data/di/SharedModule.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/dto/GithubRepoNetworkModel.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/dto/UserProfileNetwork.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/network/BackendApiClient.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/network/BackendFallbackPolicy.ktcore/data/src/commonMain/kotlin/zed/rainxch/core/data/repository/StarredRepositoryImpl.ktfeature/apps/data/src/commonMain/kotlin/zed/rainxch/apps/data/di/SharedModule.ktfeature/apps/data/src/commonMain/kotlin/zed/rainxch/apps/data/repository/AppsRepositoryImpl.ktfeature/dev-profile/data/src/commonMain/kotlin/zed/rainxch/devprofile/data/di/SharedModule.ktfeature/dev-profile/data/src/commonMain/kotlin/zed/rainxch/devprofile/data/mappers/GithubRepoNetworkModelToGitHubRepoResponse.ktfeature/dev-profile/data/src/commonMain/kotlin/zed/rainxch/devprofile/data/mappers/UserProfileNetworkToDomain.ktfeature/dev-profile/data/src/commonMain/kotlin/zed/rainxch/devprofile/data/repository/DeveloperProfileRepositoryImpl.kt
Comment on lines
+196
to
+215
| val backendResult = backendApiClient.getReleases(owner, repoName, perPage = 10) | ||
| backendResult.fold( | ||
| onSuccess = { releases -> | ||
| val stableRelease = releases.firstOrNull { it.draft != true && it.prerelease != true } | ||
| if (stableRelease == null) return Triple(releases.isNotEmpty(), false, null) | ||
| val hasInstallable = stableRelease.assets.any { asset -> | ||
| val name = asset.name.lowercase() | ||
| when (platform) { | ||
| Platform.ANDROID -> name.endsWith(".apk") | ||
| Platform.WINDOWS -> name.endsWith(".msi") || name.endsWith(".exe") | ||
| Platform.MACOS -> name.endsWith(".dmg") || name.endsWith(".pkg") | ||
| Platform.LINUX -> name.endsWith(".appimage") || name.endsWith(".deb") || | ||
| name.endsWith(".rpm") || name.endsWith(".pkg.tar.zst") | ||
| } | ||
| } | ||
| return Triple(true, hasInstallable, stableRelease.tagName) | ||
| }, | ||
| onFailure = { error -> | ||
| if (!shouldFallbackToGithubOrRethrow(error)) return Triple(false, false, null) | ||
| }, |
Contributor
There was a problem hiding this comment.
Keep latestVersion consistent with the GitHub fallback path.
Line 211 returns stableRelease.tagName even when no installable asset exists, but the direct GitHub branch at Line 265 only exposes a version when hasInstallableAssets is true. Backend-first now changes what the profile can show for unsupported platforms.
Suggested fix
- return Triple(true, hasInstallable, stableRelease.tagName)
+ return Triple(
+ true,
+ hasInstallable,
+ if (hasInstallable) stableRelease.tagName else null,
+ )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@feature/dev-profile/data/src/commonMain/kotlin/zed/rainxch/devprofile/data/repository/DeveloperProfileRepositoryImpl.kt`
around lines 196 - 215, The backend-path currently returns stableRelease.tagName
even when no installable asset exists, causing inconsistency with the GitHub
fallback; update the backend success branch (inside the backendResult.fold
onSuccess lambda in DeveloperProfileRepositoryImpl) to only return the release
tag when hasInstallable is true (e.g., return Triple(true, hasInstallable, if
(hasInstallable) stableRelease.tagName else null)) so latestVersion is null for
unsupported platforms just like the GitHub branch.
6 tasks
This was referenced May 4, 2026
3 tasks
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.
Anon users were hitting GitHub's 60/hr direct limit because several feature repos called `api.github.com` directly instead of going through the backend's 4-token pool (20k/hr aggregate). This PR rewires those paths through `BackendApiClient` so anon traffic is amplified by the pool.
Builds on backend PR #4 which raised the per-IP search bucket 60→240/min and added `/v1/users/{u}/repos` + `/v1/users/{u}/starred` passthrough.
Changes
Backend client (`BackendApiClient`):
Shared fallback policy (`BackendFallbackPolicy.kt`):
`AppsRepositoryImpl`:
`StarredRepositoryImpl.checkForValidAssets`:
`DeveloperProfileRepositoryImpl`:
DTO compatibility:
Tradeoffs
Test plan
Summary by CodeRabbit
New Features
Bug Fixes