Repository navigation
fix(seerr): stop the requests list from reloading forever on large instances - #185
Conversation
Bhavyashah94
left a comment
Bhavyashah94
left a comment
There was a problem hiding this comment.
Testing & Verification Summary
Tested branch pr-185 across both automated test suites and end-to-end on an Android emulator (Pixel 7, API 35 / Android 15) against a live Seerr backend instance.
1. Automated Tests & Static Analysis
- Unit & Provider Tests: Ran
flutter testinservices/service_seerr— 21/21 passed.- Verified parallel page retrieval maintains proper ordering across multi-page responses.
- Verified graceful fallback to sequential pagination if
/request/countfails. - Verified the 50-page safety cap enforcement.
- Verified polling timer interval only begins counting after the entire fetch finishes.
- Verified
seerrMediaDetailsProvidercache retention and 5-minute keep-alive auto-dispose.
- Static Analysis:
dart analyze services/service_seerrreported 0 errors, 0 warnings, 0 lints.
2. Android Emulator Verification
- Installed debug build of the app on an Android emulator and connected to a live Seerr instance.
- Requests View: Navigated to the Seerr Requests screen; requests loaded cleanly and quickly without enters into infinite reload loops or UI thrashing.
- Scrolling & Posters: Scrolled through the requests list; poster images loaded reactively through the media details provider without hiccups or re-triggering parent fetches.
- Pull-to-Refresh: Performed pull-to-refresh; settled cleanly and waited the configured polling interval before any background refresh.
- Logcat Monitoring: Observed zero unhandled exceptions, zero UI thread starvation, and clean Impeller frame rendering during navigation and refresh cycles.
3. Scope & Coverage
- Covered: Parallel page fetching, sequential fallback, safety cap, chained polling interval (
ref.polled), poster caching auto-dispose, live backend requests retrieval, Android emulator UI rendering and pull-to-refresh. - Not Covered: Very large instances (>1,000 requests requiring 10+ pages concurrently) on live hardware (verified via mocked unit tests), and live request mutation actions (approve/decline/delete) as this PR strictly touches fetching and polling logic.
Everything tested functions as intended and cleanly resolves the reload loop issue. Leaving as a review comment for other testers to verify before merge.
|
Thanks for the fix — moving the refresh logic from an uncoordinated Building on the verification above, a few edge-case and failure-mode observations to consider:
Overall great improvements — happy to approve once the concurrency fan-out and error retry behavior are tuned! |
…detail cache The requests provider armed a 10s invalidation timer at fetch start, so a multi-page drain slower than 10s restarted from page 0 forever - endless loading on large instances (~1500 requests = 15 serial round-trips). The refresh now goes through ref.polled (armed after the fetch settles), the unfiltered drain reads /request/count and fetches all pages concurrently, and per-title detail lookups are autoDispose with a 5-minute keep-alive instead of cached forever.
…cannot be read A failed /request/count fell back to the 2000-item safety cap, so every refresh fired 20 concurrent page requests regardless of instance size. Without a total the page count is unknown, so take the serial short-page loop instead. Add tests for the concurrent paging, the cap, and the fallback.
SeerrCounts.total defaults to 0, so a /request/count reply without a
total (an older or variant server, a proxy answering {}) planned zero
pages and the Requests list came back empty with no error. Treat 0 like
an unknown count: the serial loop costs one request on a truly empty
server and still reads every page on one that sent no total.
…the server Every failure of /request/count fell back to serial paging, so a server that does not answer burned the full timeout on the count and then again on page 0 - twice as long before the error shows as before the count was read. Rethrow timeouts, unreachable, TLS and cancelled errors; keep the fallback for a server that answered (404 on older builds, 5xx, an odd body, or a refusal, which may be a permission the list does not need).
… kept in page order The fake answered every page at once and in turn, so the order check passed even for pages gathered as they arrive, and a serial loop asks the same three pages for 250 requests - nothing showed the pages were fetched together. Hold each page until the test lets it go, check all are asked before any answers, and let them answer last first. Add a page that fails mid-way (an error, never part of the list), and say what the fake actually mimics instead of "the way the server does".
…arted The branch's headline fix - refresh after the read settles instead of on a blind 10s timer - had no test. A read that takes 15s now finishes, and the next starts 10s after it; the old timer started a second read at 10s (checked: this test fails on it, 2 reads instead of 1).
…erver throws The getAllRequests doc still promised the serial loop whenever the count could not be read; since timeouts and unreachable servers rethrow, only a count the server answered without a usable total falls back.
… count at once Only a timeout drove the transport rethrow in _requestTotal; a server that is down or refuses the connection - the commoner case - could drop out of the list and the suite stayed green while the count and page 0 each failed in turn. Checked: removing it from the list fails this test.
…d at once The getAllRequests doc said serial paging was dropped because refresh cycles could lap it, but the requests provider now arms its next cycle only after a read settles, so nothing can lap it either way. What the concurrent pages still buy is a shorter wait on every load and refresh.
_requestTotal's doc named only an unreachable server as the reason to throw, though a bad certificate and a cancel throw too, and left the refusal fallback out of its list. The test fake claimed to send only the fields the client reads but also sent pageInfo, which nothing parses, and called its requests "newest first" while numbering them upwards; drop the pageInfo and say the ids are list places.
The doc said an untrusted certificate throws, but dio reports a failed handshake as an unknown error, which falls back to paging and fails on the first page instead; "a refusal" also read like a refused connection, which does throw. Name the timeout and no-connection cases that the tests pin, and the 401/403 that falls back.
b4f4fb2 to
4a1812b
Compare
|
Tested against a live Seerr instance. The concurrent paging and settled-refresh polling work cleanly, and the request list loads fast and reliably. LGTM! |
4 participants
Issue
The request provider started a 10-second refresh timer when a fetch began, rather than when it ended.
/requestreturns at most 100 rows per call, so the full list is read page by page. With ~1500 requests that's 15 calls in a row. If those take longer than 10s, the timer fires mid-read and everything starts over from page 0. On big instances the list never finishes loading.Changes
ref.polled, so the next refresh is only scheduled after the current read is done./request/countfirst, then fetch all pages in parallel (page order is kept).seerrMediaDetailsProvideris now autoDispose with a 5 min keep-alive. Before, it kept one cached response per title ever scrolled past, for as long as the app ran.Size
Testing
flutter analyzeis clean and all 21 tests inservice_seerrpass.LLM Disclosure
Developed with assistance from Claude Code (Opus 5.5). The LLM helped me identify the related files, code parts and proposed possible code modifications and fixes.
I've reviewed, cleaned, refactored and hardened the code the best I could, but my Dart capabilities are not that good.