-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
feat: Implement API fallback system #3343
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.
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
base: dev
Are you sure you want to change the base?
Changes from 4 commits
a146676
19c0712
afc2898
c4bb7f1
0b0633d
0cd3608
c2a87e8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| package app.revanced.manager.network.api | ||
|
|
||
| import android.util.Log | ||
| import app.revanced.manager.domain.manager.PreferencesManager | ||
| import app.revanced.manager.util.tag | ||
| import kotlinx.coroutines.flow.MutableStateFlow | ||
| import kotlinx.coroutines.flow.StateFlow | ||
| import kotlinx.coroutines.flow.asStateFlow | ||
|
|
||
| class EndpointState( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this class works for chained fallbacks and it is also overcomplicated. You simply need one persistence for a schema: API(url: String, fallback: String?, restore: API?) If current API is down -> fallback
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reworked EndpointState into the priority-chain setup you described. Endpoints are now represented as "ApiEndpoint(url, fallback)" and the behavior matches what we discussed on Discord, if i rememeber correctly. Requests always try the active endpoint first. if that one is down we walk the chain from the top and pick the first reachable endpoint, so higher-priority endpoints always win. for example if we are currently on C and it dies while A or B are available we will move back up to A/B instead of falling further down to D. I also added the recovery logic for backup endpoints. while we are running on a backup, every 5 minutes we check whether any higher-priority endpoints have come back. If they have, we switch to the highest priority one that's reachable. over time that means we naturally climb back up the chain (e.g C then to B tehn to A) until we're back on the primary then the recovery loop stops doing anything. I didnt keep an explicit "restore" link on the endpoint node. A doubly-linked immutable structure (A.fallback = B and B.restore = A) is awkward to construct with vals so the node only stores the fallback direction. when we need restore candidates we derive them by walking the chain from the head. Same behavior, just simpler to build and maintain ig. Also worth noting that /about only exposes a single fallback today, so in practice we are still running with two tiers. The code just no longer assumes that's the limit, so if we add more fallback levels later, it should work without any further changes. the literal "restore" field can be added back if you'd rather keep the structure closer to the original proposal, since we kinda....shifted a lot during the...journey? |
||
| private val prefs: PreferencesManager, | ||
| ) { | ||
| enum class ActiveEndpoint { PRIMARY, FALLBACK } | ||
|
|
||
| private val _active = MutableStateFlow(ActiveEndpoint.PRIMARY) | ||
| val active: StateFlow<ActiveEndpoint> = _active.asStateFlow() | ||
|
|
||
| private val _primaryRecoveryAvailable = MutableStateFlow(false) | ||
| val primaryRecoveryAvailable: StateFlow<Boolean> = _primaryRecoveryAvailable.asStateFlow() | ||
|
|
||
| suspend fun primaryUrl(): String = prefs.api.get().trimEnd('/') | ||
| suspend fun fallbackUrl(): String = prefs.apiFallback.get().trimEnd('/') | ||
|
|
||
| suspend fun endpoints(): List<Pair<ActiveEndpoint, String>> { | ||
| val primary = ActiveEndpoint.PRIMARY to primaryUrl() | ||
| val fallback = ActiveEndpoint.FALLBACK to fallbackUrl() | ||
| return when (_active.value) { | ||
| ActiveEndpoint.PRIMARY -> listOf(primary, fallback) | ||
| ActiveEndpoint.FALLBACK -> listOf(fallback) | ||
| } | ||
| } | ||
|
|
||
| fun switchToFallback(): Boolean = | ||
| _active.compareAndSet(ActiveEndpoint.PRIMARY, ActiveEndpoint.FALLBACK) | ||
|
|
||
| suspend fun markEndpointResponseSucceeded(endpoint: ActiveEndpoint) { | ||
| val usedFallback = endpoint == ActiveEndpoint.FALLBACK | ||
| if (prefs.lastSessionUsedFallback.get() != usedFallback) { | ||
| prefs.lastSessionUsedFallback.update(usedFallback) | ||
| } | ||
| } | ||
|
|
||
| suspend fun updateFallbackFromAbout(advertised: String?) { | ||
| val normalized = advertised?.trim()?.trimEnd('/').orEmpty() | ||
| if (normalized.isEmpty()) return | ||
| if (!normalized.startsWith("https://")) { | ||
| Log.w(tag, "EndpointState: ignoring non-HTTPS fallback URL from /about: $normalized") | ||
| return | ||
| } | ||
| if (normalized == fallbackUrl()) return | ||
| Log.i(tag, "EndpointState: updating persisted fallback endpoint to $normalized") | ||
| prefs.apiFallback.update(normalized) | ||
| } | ||
|
|
||
| fun signalPrimaryRecoveryDetected() { | ||
| _primaryRecoveryAvailable.value = true | ||
| } | ||
|
|
||
| fun dismissPrimaryRecoveryPrompt() { | ||
| _primaryRecoveryAvailable.value = false | ||
| } | ||
|
|
||
| suspend fun previousSessionUsedFallback(): Boolean = prefs.lastSessionUsedFallback.get() | ||
|
|
||
| companion object { | ||
| const val DEFAULT_PRIMARY_API_URL = "https://api.revanced.app" | ||
| const val DEFAULT_FALLBACK_API_URL = "https://backup-api.revanced.app" | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,5 @@ | ||
| package app.revanced.manager.network.api | ||
|
|
||
| import android.util.Log | ||
| import app.revanced.manager.domain.manager.PreferencesManager | ||
| import app.revanced.manager.domain.manager.base.Preference | ||
| import app.revanced.manager.network.dto.ReVancedAnnouncement | ||
|
|
@@ -9,56 +8,120 @@ import app.revanced.manager.network.dto.ReVancedAssetHistory | |
| import app.revanced.manager.network.dto.ReVancedGitRepository | ||
| import app.revanced.manager.network.dto.ReVancedInfo | ||
| import app.revanced.manager.network.service.HttpService | ||
| import app.revanced.manager.network.utils.APIFailure | ||
| import app.revanced.manager.network.utils.APIResponse | ||
| import io.ktor.client.plugins.retry | ||
| import io.ktor.client.request.url | ||
| import kotlinx.coroutines.Dispatchers | ||
| import kotlinx.coroutines.withContext | ||
| import io.ktor.client.request.url | ||
| import kotlinx.coroutines.withTimeoutOrNull | ||
|
|
||
| class ReVancedAPI( | ||
| private val client: HttpService, | ||
| private val prefs: PreferencesManager | ||
| private val http: HttpService, | ||
| private val prefs: PreferencesManager, | ||
| private val endpointState: EndpointState, | ||
| ) { | ||
| private suspend fun apiUrl() = prefs.api.get() | ||
| private val defaultApiVersion = "v5" | ||
|
|
||
| private suspend inline fun <reified T> request(api: String, apiVersion: String, route: String): APIResponse<T> = | ||
| withContext(Dispatchers.IO) { | ||
| val fullUrl = "$api/$apiVersion/$route" | ||
| try { | ||
| Log.d("API", "Requesting: $fullUrl") | ||
| suspend fun getAnnouncements(): APIResponse<List<ReVancedAnnouncement>> = | ||
| request("announcements") | ||
|
|
||
| client.request { | ||
| url(fullUrl) | ||
| } | ||
| suspend fun getLatestAppInfo(): APIResponse<ReVancedAsset> = | ||
| request("manager${prefs.useManagerPrereleases.prereleaseString()}") | ||
|
|
||
| } catch (e: Exception) { | ||
| Log.e("API", "Failed request: $fullUrl", e) | ||
| throw e | ||
| } | ||
| } | ||
| suspend fun getAppHistory(): APIResponse<List<ReVancedAssetHistory>> = | ||
| request("manager/history${prefs.useManagerPrereleases.prereleaseString()}") | ||
|
|
||
| private suspend inline fun <reified T> request(route: String, apiVersion: String = defaultApiVersion) = request<T>(apiUrl(), apiVersion, route) | ||
| suspend fun getPatchesUpdate(): APIResponse<ReVancedAsset> = | ||
| request("patches${prefs.usePatchesPrereleases.prereleaseString()}") | ||
|
|
||
| suspend fun getAnnouncements() = request<List<ReVancedAnnouncement>>("announcements") | ||
| suspend fun getPatchesHistory( | ||
| apiUrl: String, | ||
| prerelease: Boolean, | ||
| ): APIResponse<List<ReVancedAssetHistory>> = | ||
| requestForSource(apiUrl, "patches/history${prerelease.prereleaseString()}") | ||
|
|
||
| suspend fun getLatestAppInfo() = | ||
| request<ReVancedAsset>("manager${prefs.useManagerPrereleases.prereleaseString()}") | ||
| suspend fun getDownloaderUpdate(): APIResponse<ReVancedAsset> = | ||
| request("manager/downloaders${prefs.useDownloaderPrerelease.prereleaseString()}") | ||
|
|
||
| suspend fun getAppHistory() = request<List<ReVancedAssetHistory>>("manager/history${prefs.useManagerPrereleases.prereleaseString()}") | ||
| suspend fun getContributors(): APIResponse<List<ReVancedGitRepository>> = | ||
| request("contributors") | ||
|
|
||
| suspend fun getPatchesUpdate() = request<ReVancedAsset>("patches${prefs.usePatchesPrereleases.prereleaseString()}") | ||
| suspend fun getInfo(): APIResponse<ReVancedInfo> { | ||
| val (response, servedBy) = requestTracked<ReVancedInfo>("about") | ||
| if (response is APIResponse.Success && servedBy == EndpointState.ActiveEndpoint.PRIMARY) { | ||
| endpointState.updateFallbackFromAbout(response.data.api?.fallback) | ||
| } | ||
| return response | ||
| } | ||
|
|
||
| suspend fun getPatchesHistory(apiUrl: String, prerelease: Boolean) = | ||
| request<List<ReVancedAssetHistory>>(apiUrl, defaultApiVersion, "patches/history${prerelease.prereleaseString()}") | ||
| suspend fun probePrimary(): Boolean = withContext(Dispatchers.IO) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But is the fallback system supporting nested fallbacks? Primary -> Fallback A -> Fallback B If currently on B, it should probe A. if on A, it should probe Primary.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the system is intentionally just primary + one backup which is the scope we agreed on. right now /about only exposes a single fallback URL so supporting something like Primary => A => B would need API changes as well before we could even represent that chain. on the main concern though , restoring back to primary even if an intermediate fallback is down, that's already how it works. probePrimary() talks directly to the primary endpoint and recovery only depends on whether the primary is reachable. The backup's health isn't part of that decision. so if we're currently on the backup and the primary comes back online, we will show the restore prompt regardless of what's happening with the backup. he re-check happens on app launch (per the spec), not continuously during a session so we don't interrupt someone while they're using the app. if we ever want true N-level fallback chains the implementation already routes through an ordered endpoints() list, so extending it wouldn't be difficult. since that's outside the current scope and would also require /about changes, I'd rather treat it as a separate discussion than fold it into this PR. I hope this clears it for you.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Single backup is all we need in my opinion. The way to increase reliability would be sever side measures.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fallback can be down too, the functionality has already been discussed on our discord
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The N chain fallback was discussed as in scope of the PR for the same reason as mentioned in my previous comment. It's essential for highest availability. Otherwise if you just probe primary, backup A could be on, but primary and B off, and it would never recover
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Not to go against you but i don't remember that. Not to imply that we did not have that convo but i have put a ton of time into the website and forgot. I do however remember you telling me that:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See https://discord.com/channels/952946952348270622/1485653081038393415/1501176573100036167 for the explanation on the functionality |
||
| withTimeoutOrNull(PROBE_TIMEOUT_MS) { | ||
| val url = "${endpointState.primaryUrl()}/$defaultApiVersion/about" | ||
| http.request<ReVancedInfo> { url(url) } is APIResponse.Success | ||
| } ?: false | ||
| } | ||
|
|
||
| suspend fun getDownloaderUpdate() = request<ReVancedAsset>("manager/downloaders${prefs.useDownloaderPrerelease.prereleaseString()}") | ||
| private suspend inline fun <reified T> request(route: String): APIResponse<T> = | ||
| requestTracked<T>(route).first | ||
|
|
||
| suspend fun getContributors() = request<List<ReVancedGitRepository>>("contributors") | ||
| /** | ||
| * Routes a request for a patch-bundle source. Only the official source (the configured | ||
| * primary endpoint) participates in the primary -> fallback logic; a user-added custom | ||
| * remote source queries its own server directly and is never redirected to the backup. | ||
| */ | ||
| private suspend inline fun <reified T> requestForSource( | ||
| apiUrl: String, | ||
| route: String, | ||
| ): APIResponse<T> { | ||
| val normalized = apiUrl.trimEnd('/') | ||
|
mostafaNazari702 marked this conversation as resolved.
Outdated
|
||
| return if (normalized == endpointState.primaryUrl()) { | ||
| request(route) | ||
| } else { | ||
| directRequest(normalized, route) | ||
| } | ||
| } | ||
|
|
||
| suspend fun getInfo() = request<ReVancedInfo>("about") | ||
| private suspend inline fun <reified T> requestTracked( | ||
| route: String, | ||
| ): Pair<APIResponse<T>, EndpointState.ActiveEndpoint?> = withContext(Dispatchers.IO) { | ||
| var lastFailure: APIResponse<T>? = null | ||
| for ((endpoint, baseUrl) in endpointState.endpoints()) { | ||
|
mostafaNazari702 marked this conversation as resolved.
Outdated
|
||
| val response = directRequest<T>(baseUrl, route) | ||
| if (response is APIResponse.Success) { | ||
| if (endpoint == EndpointState.ActiveEndpoint.FALLBACK) { | ||
| endpointState.switchToFallback() | ||
| } | ||
| endpointState.markEndpointResponseSucceeded(endpoint) | ||
| return@withContext response to endpoint | ||
| } | ||
| lastFailure = response | ||
| } | ||
| (lastFailure ?: noAttemptsFailure<T>()) to null | ||
| } | ||
|
|
||
| private suspend inline fun <reified T> directRequest( | ||
| baseUrl: String, | ||
| route: String, | ||
| ): APIResponse<T> = http.request { | ||
| url("$baseUrl/$defaultApiVersion/$route") | ||
| retry { | ||
| maxRetries = MAX_RETRIES | ||
| retryOnServerErrors() | ||
| retryOnException(retryOnTimeout = true) | ||
| exponentialDelay(base = 2.0, baseDelayMs = BACKOFF_BASE_MS) | ||
| } | ||
| } | ||
|
|
||
| private fun <T> noAttemptsFailure(): APIResponse<T> = | ||
| APIResponse.Failure(APIFailure(IllegalStateException("No request attempts were made"), null)) | ||
|
|
||
| private companion object { | ||
| const val MAX_RETRIES = 3 | ||
| const val BACKOFF_BASE_MS = 250L | ||
| const val PROBE_TIMEOUT_MS = 5_000L | ||
|
|
||
| suspend fun Preference<Boolean>.prereleaseString() = if (get()) "/prerelease" else "" | ||
| fun Boolean.prereleaseString() = if (this) "/prerelease" else "" | ||
| } | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.