feat: graceful degradation for torrent details and preferences when qBittorrent is slow - #2064
feat: graceful degradation for torrent details and preferences when qBittorrent is slow#2064s0up4200 wants to merge 8 commits into
Conversation
Preferences and alternative-speed-limits now fall back to the last-known-good cached value with an X-Qui-Cached-At header when the live call fails, instead of returning an error. The fallback uses the offline client accessor so an unreachable instance cannot trigger a blocking health-check on the fallback path.
…Bittorrent is slow Detail tabs render their last cached response with a staleness badge, or a retry action on a cold start, instead of spinning when a live qBittorrent call fails. The preferences dialog surfaces a cached-settings note when the backend served stale data, refreshing on open so the note clears once qBittorrent recovers.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR adds cached-response handling for instance preferences and alternative speed limits on the backend, exposes ChangesCached preference and freshness flow
Sequence Diagram(s)sequenceDiagram
participant InstancePreferencesDialog
participant useInstancePreferences
participant ApiClient
participant PreferencesHandler
InstancePreferencesDialog->>useInstancePreferences: open -> refetch()
useInstancePreferences->>ApiClient: getInstancePreferencesWithMeta(instanceId)
ApiClient->>PreferencesHandler: GET /instances/{id}/preferences
PreferencesHandler-->>ApiClient: preferences JSON + X-Qui-Cached-At
ApiClient-->>useInstancePreferences: preferences + cachedAt
useInstancePreferences-->>InstancePreferencesDialog: cachedAt notice
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
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 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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 `@internal/qbittorrent/preferences.go`:
- Around line 75-82: The cached preferences body and fetched-at timestamp are
being read separately, which can produce a mismatched snapshot if the cache
updates between calls. Add a single snapshot getter on Client in the preferences
cache code that returns both the cached app preferences and preferencesFetchedAt
under the same preferencesMu lock, then update the preferences handlers to use
that combined accessor instead of calling the body and time getters
independently.
In `@web/src/components/torrents/details/CachedDataBadge.tsx`:
- Around line 30-31: The cached-age message in CachedDataBadge is split between
t("detailCache.showingCached") and the relativeAge JSX token, which prevents
proper localization and word-order control. Update the component to build the
full sentence through the translation helper in CachedDataBadge, using
interpolation for relativeAge, and then add the matching placeholder to each
detailCache.showingCached locale entry so translators can reorder the phrase as
needed.
In `@web/src/components/torrents/TorrentDetailsPanel.tsx`:
- Around line 1589-1601: The cold-error fallback in TorrentDetailsPanel is
unreachable on first-load failures because the current tab visibility/redirect
logic still treats missing webseeds data as absent and navigates away before the
TabErrorBlock can render. Update the webseeds handling in TorrentDetailsPanel so
the tab remains visible while the getTorrentWebSeeds query is loading or in
"cold-error", or defer the redirect until after a confirmed empty successful
response; keep the retry UI wired to refetchWebseeds so first-load failures can
be recovered in place.
In `@web/src/hooks/useInstancePreferences.ts`:
- Around line 55-61: The query in useInstancePreferences should not
short-circuit to cachedMetadata.preferences when refetch() runs, because that
bypasses the live qBittorrent check and can leave stale data with a cleared
cachedAt state. Keep cachedMetadata only as initialData, and update the query
function so every execution of the hook’s fetch path calls
api.getInstancePreferencesWithMeta(instanceId) and then stores cachedAt in the
cachedAtQueryKey, while preserving the cached metadata for initialization only.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: eee39942-5fc8-4143-af4a-9e1cd0da72e4
📒 Files selected for processing (26)
internal/api/handlers/preferences.gointernal/api/handlers/preferences_test.gointernal/qbittorrent/client.gointernal/qbittorrent/preferences.gointernal/qbittorrent/sync_manager.goweb/src/components/instances/preferences/InstancePreferencesDialog.tsxweb/src/components/torrents/TorrentDetailsPanel.tsxweb/src/components/torrents/details/CachedDataBadge.tsxweb/src/components/torrents/details/index.tsweb/src/hooks/useInstancePreferences.test.tsxweb/src/hooks/useInstancePreferences.tsweb/src/i18n/locales/de/instances.jsonweb/src/i18n/locales/de/torrents.jsonweb/src/i18n/locales/en/instances.jsonweb/src/i18n/locales/en/torrents.jsonweb/src/i18n/locales/fr/instances.jsonweb/src/i18n/locales/fr/torrents.jsonweb/src/i18n/locales/it/instances.jsonweb/src/i18n/locales/it/torrents.jsonweb/src/i18n/locales/ko/instances.jsonweb/src/i18n/locales/ko/torrents.jsonweb/src/i18n/locales/zh-CN/instances.jsonweb/src/i18n/locales/zh-CN/torrents.jsonweb/src/lib/api.tsweb/src/lib/stale-query.test.tsweb/src/lib/stale-query.ts
…r one lock Reading the cached preferences (and alternative-speed-limits mode) body separately from its fetched-at timestamp could return a mismatched pair if the cache refreshed between the two calls. Add combined snapshot accessors and use them on the fallback path.
Build the cached-data badge message through i18n interpolation so the relative age is localizable, keep the web seeds tab reachable on a cold-load error so its retry stays usable instead of being hidden as "no web seeds", and always refetch preferences live (metadata seeds the initial render only) so an explicit refetch re-checks qBittorrent instead of returning cached data with a cleared staleness marker.
f6fd5e8 to
6f0c579
Compare
The Ukrainian locale (merged via #2032) lacked the detailCache and dialog.cachedNotice keys this branch adds, failing pnpm check:i18n.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/src/components/torrents/TorrentDetailsPanel.tsx (1)
524-531: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the fallback tab during render instead of in an Effect.
This Effect derives
activeTabfrom query state. Use aneffectiveActiveTabvalue for<Tabs>so the invalid webseeds tab is hidden without an extra state update/render.♻️ Proposed refactor
const hasWebseeds = (webseedsData?.length ?? 0) > 0 + const effectiveActiveTab = + activeTab === "webseeds" && !hasWebseeds && !loadingWebseeds && !webseedsError + ? "general" + : activeTab - // Redirect away from the webseeds tab only on a confirmed empty successful load - // (e.g. switching to a torrent without web seeds). On a cold-error we keep the tab - // so its retry UI stays reachable instead of silently hiding the failure. - useEffect(() => { - if (activeTab === "webseeds" && !hasWebseeds && !loadingWebseeds && !webseedsError) { - setActiveTab("general") - } - }, [activeTab, hasWebseeds, loadingWebseeds, webseedsError, setActiveTab])Apply this where the tabs are rendered:
- <Tabs value={activeTab} onValueChange={handleTabChange} className="flex-1 flex flex-col overflow-hidden"> + <Tabs value={effectiveActiveTab} onValueChange={handleTabChange} className="flex-1 flex flex-col overflow-hidden">As per coding guidelines, “Use
useEffectonly to sync with external systems” and “Avoid derived state in Effects; calculate during render or useuseMemofor expensive computation.”🤖 Prompt for 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. In `@web/src/components/torrents/TorrentDetailsPanel.tsx` around lines 524 - 531, Derive the fallback tab during render instead of using the current useEffect in TorrentDetailsPanel. Replace the state-sync logic around activeTab/hasWebseeds/loadingWebseeds/webseedsError with an effectiveActiveTab value computed during render and pass that to the Tabs render path, so the webseeds tab is hidden immediately without an extra state update/render. Keep the existing tab selection behavior for confirmed empty successful loads, but move the fallback decision out of the Effect and into the render logic near the Tabs component and activeTab handling.Source: Coding guidelines
🤖 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 `@web/src/i18n/locales/ko/torrents.json`:
- Around line 1132-1139: The Korean torrents locale is missing required `_one`
plural entries and has several placeholder mismatches versus the English source.
Update `ko/torrents.json` to add every missing `_one` key alongside its existing
plural counterparts, and fix the affected strings with the correct placeholder
usage (for example in `actionToasts`, `statusBar.torrentCount`, and
`crossSeedDialog.indexersFiltered`) so the locale matches the source schema
exactly.
---
Nitpick comments:
In `@web/src/components/torrents/TorrentDetailsPanel.tsx`:
- Around line 524-531: Derive the fallback tab during render instead of using
the current useEffect in TorrentDetailsPanel. Replace the state-sync logic
around activeTab/hasWebseeds/loadingWebseeds/webseedsError with an
effectiveActiveTab value computed during render and pass that to the Tabs render
path, so the webseeds tab is hidden immediately without an extra state
update/render. Keep the existing tab selection behavior for confirmed empty
successful loads, but move the fallback decision out of the Effect and into the
render logic near the Tabs component and activeTab handling.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 97a16fc4-6a2b-42e8-8608-f19d0405ea45
📒 Files selected for processing (8)
internal/qbittorrent/preferences.goweb/src/components/torrents/TorrentDetailsPanel.tsxweb/src/i18n/locales/de/torrents.jsonweb/src/i18n/locales/en/torrents.jsonweb/src/i18n/locales/fr/torrents.jsonweb/src/i18n/locales/it/torrents.jsonweb/src/i18n/locales/ko/torrents.jsonweb/src/i18n/locales/zh-CN/torrents.json
✅ Files skipped from review due to trivial changes (5)
- web/src/i18n/locales/it/torrents.json
- web/src/i18n/locales/en/torrents.json
- web/src/i18n/locales/de/torrents.json
- web/src/i18n/locales/fr/torrents.json
- web/src/i18n/locales/zh-CN/torrents.json
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/qbittorrent/preferences.go
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🧹 Nitpick comments (1)
web/src/components/torrents/TorrentDetailsPanel.tsx (1)
524-531: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDerive the fallback tab during render instead of in an Effect.
This Effect derives
activeTabfrom query state. Use aneffectiveActiveTabvalue for<Tabs>so the invalid webseeds tab is hidden without an extra state update/render.♻️ Proposed refactor
const hasWebseeds = (webseedsData?.length ?? 0) > 0 + const effectiveActiveTab = + activeTab === "webseeds" && !hasWebseeds && !loadingWebseeds && !webseedsError + ? "general" + : activeTab - // Redirect away from the webseeds tab only on a confirmed empty successful load - // (e.g. switching to a torrent without web seeds). On a cold-error we keep the tab - // so its retry UI stays reachable instead of silently hiding the failure. - useEffect(() => { - if (activeTab === "webseeds" && !hasWebseeds && !loadingWebseeds && !webseedsError) { - setActiveTab("general") - } - }, [activeTab, hasWebseeds, loadingWebseeds, webseedsError, setActiveTab])Apply this where the tabs are rendered:
- <Tabs value={activeTab} onValueChange={handleTabChange} className="flex-1 flex flex-col overflow-hidden"> + <Tabs value={effectiveActiveTab} onValueChange={handleTabChange} className="flex-1 flex flex-col overflow-hidden">As per coding guidelines, “Use
useEffectonly to sync with external systems” and “Avoid derived state in Effects; calculate during render or useuseMemofor expensive computation.”🤖 Prompt for 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. In `@web/src/components/torrents/TorrentDetailsPanel.tsx` around lines 524 - 531, Derive the fallback tab during render instead of using the current useEffect in TorrentDetailsPanel. Replace the state-sync logic around activeTab/hasWebseeds/loadingWebseeds/webseedsError with an effectiveActiveTab value computed during render and pass that to the Tabs render path, so the webseeds tab is hidden immediately without an extra state update/render. Keep the existing tab selection behavior for confirmed empty successful loads, but move the fallback decision out of the Effect and into the render logic near the Tabs component and activeTab handling.Source: Coding guidelines
🤖 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 `@web/src/i18n/locales/ko/torrents.json`:
- Around line 1132-1139: The Korean torrents locale is missing required `_one`
plural entries and has several placeholder mismatches versus the English source.
Update `ko/torrents.json` to add every missing `_one` key alongside its existing
plural counterparts, and fix the affected strings with the correct placeholder
usage (for example in `actionToasts`, `statusBar.torrentCount`, and
`crossSeedDialog.indexersFiltered`) so the locale matches the source schema
exactly.
---
Nitpick comments:
In `@web/src/components/torrents/TorrentDetailsPanel.tsx`:
- Around line 524-531: Derive the fallback tab during render instead of using
the current useEffect in TorrentDetailsPanel. Replace the state-sync logic
around activeTab/hasWebseeds/loadingWebseeds/webseedsError with an
effectiveActiveTab value computed during render and pass that to the Tabs render
path, so the webseeds tab is hidden immediately without an extra state
update/render. Keep the existing tab selection behavior for confirmed empty
successful loads, but move the fallback decision out of the Effect and into the
render logic near the Tabs component and activeTab handling.
🪄 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: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 97a16fc4-6a2b-42e8-8608-f19d0405ea45
📒 Files selected for processing (8)
internal/qbittorrent/preferences.goweb/src/components/torrents/TorrentDetailsPanel.tsxweb/src/i18n/locales/de/torrents.jsonweb/src/i18n/locales/en/torrents.jsonweb/src/i18n/locales/fr/torrents.jsonweb/src/i18n/locales/it/torrents.jsonweb/src/i18n/locales/ko/torrents.jsonweb/src/i18n/locales/zh-CN/torrents.json
✅ Files skipped from review due to trivial changes (5)
- web/src/i18n/locales/it/torrents.json
- web/src/i18n/locales/en/torrents.json
- web/src/i18n/locales/de/torrents.json
- web/src/i18n/locales/fr/torrents.json
- web/src/i18n/locales/zh-CN/torrents.json
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/qbittorrent/preferences.go
🛑 Comments failed to post (1)
web/src/i18n/locales/ko/torrents.json (1)
1132-1139: 📐 Maintainability & Code Quality | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Description: Compare every locale's torrents.json against English for key parity, # placeholder parity, HTML tag parity, and empty translated strings. python - <<'PY' import json import pathlib import re import sys root = pathlib.Path("web/src/i18n/locales") base_path = root / "en" / "torrents.json" placeholder_re = re.compile(r"{{\s*([^{}\s]+)\s*}}") tag_re = re.compile(r"</?([A-Za-z][A-Za-z0-9-]*)(?:\s[^>]*)?>") def load(path): with path.open("r", encoding="utf-8") as f: return json.load(f) def flatten(obj, prefix=""): out = {} if isinstance(obj, dict): for key, value in obj.items(): next_prefix = f"{prefix}.{key}" if prefix else key out.update(flatten(value, next_prefix)) else: out[prefix] = obj return out base = flatten(load(base_path)) failed = False for path in sorted(root.glob("*/torrents.json")): if path == base_path: continue locale = path.parent.name current = flatten(load(path)) missing = sorted(set(base) - set(current)) extra = sorted(set(current) - set(base)) if missing: failed = True print(f"[{locale}] missing keys:") for key in missing: print(f" - {key}") if extra: failed = True print(f"[{locale}] extra keys:") for key in extra: print(f" - {key}") for key in sorted(set(base) & set(current)): base_value = base[key] current_value = current[key] if isinstance(current_value, str) and current_value == "": failed = True print(f"[{locale}] empty string: {key}") if isinstance(base_value, str) and isinstance(current_value, str): base_placeholders = sorted(placeholder_re.findall(base_value)) current_placeholders = sorted(placeholder_re.findall(current_value)) if base_placeholders != current_placeholders: failed = True print(f"[{locale}] placeholder mismatch at {key}: {current_placeholders} != {base_placeholders}") base_tags = sorted(tag_re.findall(base_value)) current_tags = sorted(tag_re.findall(current_value)) if base_tags != current_tags: failed = True print(f"[{locale}] HTML tag mismatch at {key}: {current_tags} != {base_tags}") if failed: sys.exit(1) print("Locale parity check passed.") PYRepository: autobrr/qui
Length of output: 15139
Add missing
_oneplural form keys toko/torrents.json.The Korean
torrents.jsonis missing 47 keys ending in_one(e.g.,actionToasts.failed.addTags_one,statusBar.torrentCount_one). According to the project's localization guidelines, all locales must include plural forms to match the English source.Additionally,
ko/torrents.jsonhas 13 placeholder mismatches wherepluralis expected but missing (e.g.,crossSeedDialog.indexersFiltered).Please add the missing keys and correct the placeholder usage to ensure full parity with the English source.
🤖 Prompt for 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. In `@web/src/i18n/locales/ko/torrents.json` around lines 1132 - 1139, The Korean torrents locale is missing required `_one` plural entries and has several placeholder mismatches versus the English source. Update `ko/torrents.json` to add every missing `_one` key alongside its existing plural counterparts, and fix the affected strings with the correct placeholder usage (for example in `actionToasts`, `statusBar.torrentCount`, and `crossSeedDialog.indexersFiltered`) so the locale matches the source schema exactly.Source: Coding guidelines
Replace the activeTab state-sync useEffect with an effectiveActiveTab value computed during render, so the webseeds tab is hidden immediately without an extra render and the persisted tab preference is left intact. Loading/error states still keep the tab so its retry UI stays reachable.
PR #2057 kept the torrent list on screen with a staleness badge when qBittorrent is slow to serialize maindata, but the rest of the UI still hard-failed in that case: the detail tabs spun or silently emptied, and the preferences dialog errored out. This extends the same graceful degradation to the torrent detail tabs (general/properties, trackers, peers, content, web seeds) and to instance preferences. The detail tabs now keep rendering the last response already held in the client cache with an amber "showing cached data" badge, falling back to a retry action only on a cold start with no data to show.
On the backend, the preferences and alternative-speed-limits endpoints fall back to their last-known-good cached value (signalled with an
X-Qui-Cached-Atheader) instead of returning an error, and the preferences dialog renders a "cached settings" note that refreshes on open so it clears once qBittorrent recovers. The cache lookup uses the offline client accessor so an unreachable instance cannot trigger a blocking health-check on the fallback path. Detail-tab resilience is handled entirely client-side, since those responses are already retained by the query cache, so no new server-side per-torrent cache was introduced.Summary by CodeRabbit
X-Qui-Cached-Atsupport for preferences and exposed preference metadata viagetInstancePreferencesWithMeta.cachedAtlifecycle, alternative speed limits degradation, and query freshness/retry states.