feat: Implement a fallback system to the ReVanced Website#393
feat: Implement a fallback system to the ReVanced Website#393mostafaNazari702 wants to merge 23 commits into
Conversation
- drop cache: 'no-store' from SSR fetches and client reads so HTTP cache headers can take effect once the cache middleware lands. - correct `announcement/latest` => `announcements/latest` in server.ts to match the real API and the existing client call.
Introduces the `RV_API_URL_FALLBACK` env var and a `fallback_api_url` field on the `/about` response (validated via `z.url()` per the discussed approach ("SR-7478" thread, ReVanced Discord Server), schema does the URL check, no custom validator).
consumption of added values lands in a later commit.
Added getApiUrl() returning the env primary, it introduces the indirection point coming commit's failover layer will hook into. Added getFallbackApiUrl() returning the resolved fallback chain: stored /about value overrides the RV_API_URL_FALLBACK env value. Persist /about's fallback_api_url to localStorage under "revanced_api_url_fallback", always overwrite, clear on null/empty. Fix existing "only write if empty" bug in populateDynamicSettings for status URL and email, now always synced with /about. Route buildUrl through getApiUrl() , so both fetchJson and mutateJson in client.ts pick up the resolved URL transitively.
wraps SSR fetches in a cloudflare workers cache API middleware , "caches.defaul"
oSumAtrIX
left a comment
There was a problem hiding this comment.
One consideration about the fallback system is if we want to have a capture feature where a fallback can prevent the site recovering to the original URL, this would prevent a scenario when the main domain is taken over and brought back live. The fallback API field can be:
fallback: { url: "", recover: true }
If recover is set to false, it won't attempt to recover until the recover variable is set to true again.
oSumAtrIX
left a comment
There was a problem hiding this comment.
The fallback logic isnt implemented yet
a945434 to
9ad5854
Compare
|
I don't think ssr/worker sees localstorage so the "recover: false" capture is on the browser only i think. it continues following RV_API_URL => RV_API_URL_FALLBACK (env only). full server-side capture needs persisting the dynamic value in a DO, it will be a follow-up. Probably after this. |
|
revanced-website-storage is separate cloudflare worker for the Durable Object storage for /about.fallback. After the first deploy uncomment the Durable Object binding in the main wrangler.toml and redeploy the Pages app. |
|
|
||
| for (const announcement of latestAnnouncements) { | ||
| byId.set(announcement.id, announcement); | ||
| async function poll(): Promise<void> { |
There was a problem hiding this comment.
Why did you add polling and client side fetching? The website is intentionally server side rendered, so this shouldnt be needed
| // interface Platform {} | ||
| interface Platform { | ||
| env?: { | ||
| FALLBACK_STORAGE?: { |
There was a problem hiding this comment.
This should simply be called storage. Why is it placed in Platform.env?
| # the Storage interface in src/lib/api/storage.ts keeps the impl swappable if we ever migrate to DO. | ||
| # Create the namespace with "wrangler kv namespace create FALLBACK_STORAGE", paste the returned id below, then uncomment. | ||
| # until then the SSR path falls through to env-only behavior. | ||
| # [[kv_namespaces]] |
There was a problem hiding this comment.
So weird that sveltekit supports this, but not durable objects
| # server-side persistence for the fallback URL from the /about endpoint. | ||
| # We need this for the "recover: false" capture mechanism to also bite at the SSR layer (without it capture is browser-only) | ||
| # Pages cant host Durable Object classes and we agreed not to deploy a separate worker just for this so KV is the persistence layer. | ||
| # the Storage interface in src/lib/api/storage.ts keeps the impl swappable if we ever migrate to DO. |
There was a problem hiding this comment.
| # the Storage interface in src/lib/api/storage.ts keeps the impl swappable if we ever migrate to DO. | |
| # the Storage interface in src/lib/api/storage.ts keeps the implementation swappable once DO is supported. |
|
|
||
| # server-side persistence for the fallback URL from the /about endpoint. | ||
| # We need this for the "recover: false" capture mechanism to also bite at the SSR layer (without it capture is browser-only) | ||
| # Pages cant host Durable Object classes and we agreed not to deploy a separate worker just for this so KV is the persistence layer. |
There was a problem hiding this comment.
Pages can host DO, what cant is sveltekit
| # Pages cant host Durable Object classes and we agreed not to deploy a separate worker just for this so KV is the persistence layer. | |
| # SvelteKit does not yet support Durable Objects and we agreed not to deploy a separate worker just for this so KV is the persistence layer. |
| pages_build_output_dir = ".svelte-kit/cloudflare" | ||
| compatibility_flags = ["nodejs_compat"] | ||
|
|
||
| # server-side persistence for the fallback URL from the /about endpoint. |
There was a problem hiding this comment.
| # server-side persistence for the fallback URL from the /about endpoint. | |
| # Server-side persistence for the fallback URL from the /about endpoint. |
| compatibility_flags = ["nodejs_compat"] | ||
|
|
||
| # server-side persistence for the fallback URL from the /about endpoint. | ||
| # We need this for the "recover: false" capture mechanism to also bite at the SSR layer (without it capture is browser-only) |
There was a problem hiding this comment.
Try to reformulate comments. Instead of saying "we" say something else like. ""recover: false" is needed...
| # until then the SSR path falls through to env-only behavior. | ||
| # [[kv_namespaces]] | ||
| # binding = "FALLBACK_STORAGE" | ||
| # id = "" |
There was a problem hiding this comment.
Please also adjust CI so at deployment it injects the id. The action should be able to, if not let me know
| export function getApiUrl(): string { | ||
| return getActiveUrls().primary; |
There was a problem hiding this comment.
This is inefficient. Every call to getApiUrl is going to make a read to the persistence layer. The worker does multiple fetches. Please use constants wherever possible and reduce unnecessary calls
|
|
||
| export function getContactEmail(): string { | ||
| if (browser) { | ||
| const cached = localStorage.getItem(EMAIL_KEY); |
There was a problem hiding this comment.
Why was the email being cached in local storage?
|
|
||
| export function buildUrl(endpoint: string): string { | ||
| export function composeApiUrl(base: string, endpoint: string): string { | ||
| endpoint = endpoint.replace(/^\/+/, ''); |
| if (endpoint === API_VERSION || endpoint.startsWith(API_VERSION + '/')) { | ||
| endpoint = endpoint.slice(API_VERSION.length).replace(/^\/+/, ''); | ||
| } | ||
| return `${base}/${API_VERSION}/${endpoint}`; |
There was a problem hiding this comment.
Defensive code like this isnt needed. Also the implementations of the server/worker side code differ from this.
|
|
||
| type StoredFallback = { url: string | null; recover: boolean }; | ||
|
|
||
| function readStoredFallback(): StoredFallback | null { |
There was a problem hiding this comment.
It looks like the worker/server share similar code. Is it possible to have a shared code file where you can inject implementations of the persistence layer (e.g. the frontend would call the shared code with localstorage, server with cloudflare kv)
| function syncLocal(key: string, value: string | null | undefined): void { | ||
| if (!browser) return; | ||
| if (value) { | ||
| localStorage.setItem(key, value); | ||
| } else { | ||
| localStorage.removeItem(key); | ||
| } | ||
| } |
There was a problem hiding this comment.
Weird name, simply call it save, or setStorage
| - name: Inject KV namespace id | ||
| env: | ||
| KV_NAMESPACE_ID: ${{ secrets.KV_NAMESPACE_ID }} | ||
| run: sed -i "s/__KV_NAMESPACE_ID__/${KV_NAMESPACE_ID}/g" wrangler.toml |
There was a problem hiding this comment.
There was a problem hiding this comment.
On the broader setup (template file + gitignored wrangler.toml) i can switch to that if you . i just went with the simpler in-place envsubst for now since it matched the "use envsubst instead" and we only need to inject a single env value anyway.
Do you want it split into a template setup?
| const fresh = await fetchFn(url); | ||
| if (!fresh.ok) return fresh; | ||
|
|
||
| const cacheControl = fresh.headers.get('Cache-Control')?.toLowerCase() ?? ''; | ||
| if (/\bno-store\b|\bno-cache\b|\bprivate\b/.test(cacheControl)) return fresh; | ||
|
|
||
| if (/\bmax-age\b/.test(cacheControl)) { | ||
| await this.cache.put(url, fresh.clone()); | ||
| } else { | ||
| const body = await fresh.clone().arrayBuffer(); |
There was a problem hiding this comment.
Please use the cloudflare native cache as explained in my previous comments with overrriding the header if none
| if (storage) { | ||
| try { | ||
| if (about.fallback === null) { | ||
| await storage.delete(STORAGE_KEY); |
There was a problem hiding this comment.
if you have .set, this should be .unset or simply do set(null)
| fetchFn?: typeof fetch, | ||
| platform?: App.Platform | ||
| ): Promise<Contributable[]> { | ||
| const storage = createStorageFromPlatform(platform); |
There was a problem hiding this comment.
Why do you create an instance of storage for every fetch instead of creating it once and passing it down?
…t storage creation


This pull request implements a fallback system for the ReVanced Website should the primary and fallback API not be available, an offline/cache-state will be implemented to keep the website operational even if:
The process will be iterating rather than one massive PR. The maintainer(s) can suggest or push back on anything they see and their opinions matter on each PUSH, "continue" is required for me to continue with the next part. A maximum of 2 commits will be pushed a push.