From 0cd04926e68ea61dd752f477f5371ba02004e5be Mon Sep 17 00:00:00 2001 From: jordan Date: Tue, 29 Sep 2026 21:57:27 +0000 Subject: [PATCH 1/2] fix(ui): stop the book page refetching forever for a book that does not exist Opening /book/ for a book that no longer exists showed a spinner indefinitely instead of the existing "Sorry, that book cannot be found" screen. BookDetailsPageConnector fetches the book when it is not in the store (needsBookFetch = not fetching && no book). For a missing book the server answers 404 (GET /book?bookId=), the FETCH_BOOKS handler's failure branch sets isFetching=false, and needsBookFetch flips false -> true again; componentDidUpdate treats that transition as a reason to populate() and fetches again. That is an endless loop of failing requests with the spinner up most of the time, so the NotFound render is never reached. Remember the route key that was fetched and only fetch once per route (same for the author fetch, which has the same shape if an author is missing). Once that one fetch fails, isFetching is false, there is no book, and the existing NotFound screen renders. Any bookmark or list entry pointing at a pruned or deleted book hit this. --- .../Book/Details/BookDetailsPageConnector.js | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/frontend/src/Book/Details/BookDetailsPageConnector.js b/frontend/src/Book/Details/BookDetailsPageConnector.js index 770071d8..837b2ccd 100644 --- a/frontend/src/Book/Details/BookDetailsPageConnector.js +++ b/frontend/src/Book/Details/BookDetailsPageConnector.js @@ -104,6 +104,8 @@ class BookDetailsPageConnector extends Component { super(props); this.state = { hasMounted: false }; this._lastSiblingFetchKey = null; + this._lastBookFetchKey = null; + this._lastAuthorFetchKey = null; } // // Lifecycle @@ -141,7 +143,16 @@ class BookDetailsPageConnector extends Component { siblingCount } = this.props; - if (needsBookFetch && routeBookKey) { + // Fetch each book (and author) at most once per route. When the book does not exist the server + // answers 404, the fetch fails and `isFetching` drops back to false while the book is still + // missing, so `needsBookFetch` flips false -> true again and componentDidUpdate calls populate() + // again: an endless loop of failing requests with the spinner up most of the time, and the + // "cannot be found" screen below never gets to render. + const bookFetchKey = `${routeBookKey}|${scopedMediaType || ''}`; + + if (needsBookFetch && routeBookKey && this._lastBookFetchKey !== bookFetchKey) { + this._lastBookFetchKey = bookFetchKey; + // Fetch the specific book data. bookId may be either the local numeric id // or a Readarr-compatible titleSlug from an external service link. const fetchParams = { bookId: routeBookKey.toString() }; @@ -153,7 +164,9 @@ class BookDetailsPageConnector extends Component { this.props.fetchBooks(fetchParams); } - if (needsAuthorFetch && authorId) { + if (needsAuthorFetch && authorId && this._lastAuthorFetchKey !== authorId) { + this._lastAuthorFetchKey = authorId; + // Fetch the author data this.props.fetchAuthor({ id: authorId }); } From 4e83dd5199f867cb20e648bb6ed335f1380b6e25 Mon Sep 17 00:00:00 2001 From: jordan Date: Tue, 29 Sep 2026 21:59:46 +0000 Subject: [PATCH 2/2] Retry non-404 failures a few times instead of blocking every retry Adversarial review of #272: a once-per-route guard would leave 'cannot be found' up after a transient 500, a network error, or a request aborted by another fetchBooks, and also if the book is dropped from the store while the page is open. A definitive 404 is now final (that is the loop being fixed); any other outcome is retried up to 3 fetches per route (MAX_BOOK_FETCH_ATTEMPTS), so a persistent failure still ends on the NotFound screen. Adds booksError to the connector props and resets the counter when the route key changes. --- .../Book/Details/BookDetailsPageConnector.js | 33 +++++++++++++++---- 1 file changed, 27 insertions(+), 6 deletions(-) diff --git a/frontend/src/Book/Details/BookDetailsPageConnector.js b/frontend/src/Book/Details/BookDetailsPageConnector.js index 837b2ccd..2930ee97 100644 --- a/frontend/src/Book/Details/BookDetailsPageConnector.js +++ b/frontend/src/Book/Details/BookDetailsPageConnector.js @@ -47,6 +47,9 @@ function findBookByRouteSlug(items, routeBookKey, scopedMediaType) { return matches.sort((left, right) => left.id - right.id)[0]; } +// Max fetches of one book per route: the first try plus retries for non-404 failures. +const MAX_BOOK_FETCH_ATTEMPTS = 3; + function createMapStateToProps() { return createSelector( (state, { match }) => match, @@ -58,6 +61,7 @@ function createMapStateToProps() { const isNumericRoute = isNumericBookRoute(routeBookKey); const numericBookId = isNumericRoute ? parseInt(routeBookKey) : null; const isFetching = books.isFetching || author.isFetching; + const booksError = books.error; // Find the book if it exists const book = isNumericRoute ? @@ -86,6 +90,7 @@ function createMapStateToProps() { siblingCount, needsBookFetch, needsAuthorFetch, + booksError, isFetching, isPopulated: hasBook && !needsAuthorFetch }; @@ -105,6 +110,7 @@ class BookDetailsPageConnector extends Component { this.state = { hasMounted: false }; this._lastSiblingFetchKey = null; this._lastBookFetchKey = null; + this._bookFetchAttempts = 0; this._lastAuthorFetchKey = null; } // @@ -137,21 +143,35 @@ class BookDetailsPageConnector extends Component { scopedMediaType, needsBookFetch, needsAuthorFetch, + booksError, authorId, book, bookMediaType, siblingCount } = this.props; - // Fetch each book (and author) at most once per route. When the book does not exist the server - // answers 404, the fetch fails and `isFetching` drops back to false while the book is still - // missing, so `needsBookFetch` flips false -> true again and componentDidUpdate calls populate() - // again: an endless loop of failing requests with the spinner up most of the time, and the - // "cannot be found" screen below never gets to render. + // Do not refetch forever. When the book does not exist the server answers 404, the fetch fails + // and `isFetching` drops back to false while the book is still missing, so `needsBookFetch` flips + // false -> true again and componentDidUpdate calls populate() again: an endless loop of failing + // requests with the spinner up most of the time, and the "cannot be found" screen below never + // gets to render. + // + // A definitive 404 is final. Anything else (a transient 500, a network error, a request aborted by + // another fetchBooks, an empty 200) is retried, but only a few times so a persistent failure still + // ends on the "cannot be found" screen instead of looping. const bookFetchKey = `${routeBookKey}|${scopedMediaType || ''}`; + const isNewBookKey = this._lastBookFetchKey !== bookFetchKey; - if (needsBookFetch && routeBookKey && this._lastBookFetchKey !== bookFetchKey) { + if (isNewBookKey) { this._lastBookFetchKey = bookFetchKey; + this._bookFetchAttempts = 0; + } + + const lastFetchWasNotFound = !!booksError && booksError.status === 404; + const canRetryBookFetch = !isNewBookKey && !lastFetchWasNotFound && this._bookFetchAttempts < MAX_BOOK_FETCH_ATTEMPTS; + + if (needsBookFetch && routeBookKey && (isNewBookKey || canRetryBookFetch)) { + this._bookFetchAttempts++; // Fetch the specific book data. bookId may be either the local numeric id // or a Readarr-compatible titleSlug from an external service link. @@ -250,6 +270,7 @@ BookDetailsPageConnector.propTypes = { bookMediaType: PropTypes.string, siblingCount: PropTypes.number, needsBookFetch: PropTypes.bool, + booksError: PropTypes.object, needsAuthorFetch: PropTypes.bool, match: PropTypes.shape({ params: PropTypes.shape({ bookId: PropTypes.string.isRequired }).isRequired }).isRequired, fetchBooks: PropTypes.func.isRequired,