diff --git a/vue/src/composables/__tests__/useFeeds.spec.js b/vue/src/composables/__tests__/useFeeds.spec.js index 61b9b0e..bf7b215 100644 --- a/vue/src/composables/__tests__/useFeeds.spec.js +++ b/vue/src/composables/__tests__/useFeeds.spec.js @@ -1,5 +1,4 @@ -import { describe, it, expect, vi, beforeEach } from 'vitest' -import { flushPromises } from '@vue/test-utils' +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' import axios from 'axios' import { useFeeds } from '../useFeeds' @@ -13,7 +12,7 @@ class FakeIntersectionObserver { vi.stubGlobal('IntersectionObserver', FakeIntersectionObserver) describe('useFeeds', () => { - const { feeds, allItems, feedFilter, feedTitles, feedUnreadCounts, unreadCount, displayedFeedUnreadCounts, refreshUnreadDisplay, setFeedFilter, showMessage, message, showModal, fetchData, sync, getReadable, setInitialLoad, handleIntersection, markAllRead, setupIntersectionObserver } = useFeeds() + const { feeds, allItems, feedFilter, feedTitles, feedUnreadCounts, unreadCount, displayedFeedUnreadCounts, refreshUnreadDisplay, setFeedFilter, showMessage, message, showModal, fetchData, sync, getReadable, setInitialLoad, handleIntersection, markAllRead, markRead, setupIntersectionObserver } = useFeeds() beforeEach(() => { localStorage.setItem('user-token', 'test-token') @@ -178,6 +177,193 @@ describe('useFeeds', () => { setInitialLoad(false) }) + describe('markRead resilience', () => { + // Mirrors useFeeds.js's MARK_READ_RETRY_DELAYS_MS (not exported). Kept in + // sync via this comment: if that backoff changes, update this too — too + // small a value just makes the affected test's `await` hang until it + // times out, rather than pass incorrectly. + const RETRY_BACKOFF_SUM_MS = 300 + 1000 + + // A failed assertion before a test's own vi.useRealTimers() call would + // otherwise leak fake timers into every later test in this file. + afterEach(() => { + vi.useRealTimers() + }) + + it('retries a failed mark-read PUT before giving up, with a request timeout set', async () => { + vi.useFakeTimers() + axios.put + .mockRejectedValueOnce(new Error('network')) + .mockRejectedValueOnce(new Error('network')) + .mockResolvedValueOnce({ status: 200 }) + + const done = markRead(601) + await vi.runAllTimersAsync() + + expect(await done).toBe(true) + expect(axios.put).toHaveBeenCalledTimes(3) + expect(axios.put).toHaveBeenCalledWith( + '/api/v1/article/read/601', + null, + expect.objectContaining({ timeout: expect.any(Number) }), + ) + expect(showMessage.value).toBe(false) + }) + + it('reports failure once retries are exhausted, and stops withholding the item from a refetch', async () => { + vi.useFakeTimers() + axios.put.mockRejectedValue(new Error('network')) + + const done = markRead(602) + // Advance past just the two retry backoffs — not runAllTimersAsync(), + // which would also fire showMessageForXSeconds's own 5s auto-hide timer + // and reset showMessage before the assertion below. + await vi.advanceTimersByTimeAsync(RETRY_BACKOFF_SUM_MS) + + expect(await done).toBe(false) + expect(axios.put).toHaveBeenCalledTimes(3) + expect(showMessage.value).toBe(true) + expect(message.value).toMatch(/could not mark/i) + + // The mark never committed, so the item genuinely is still unread + // server-side — a refetch must be free to show it again, not hide it + // forever as "pending". + axios.get.mockResolvedValueOnce({ + data: { + feeds: [{ + title: 'Feed A', + items: [{ id: 602, title: 'A1', content: '', url: 'https://example.test/a1', timestamp: '2026-01-01 10:00:00' }], + }], + }, + }) + await fetchData() + expect(feeds.value.map(f => f.id)).toEqual([602]) + }) + + it('does not retry a non-retryable 4xx mark-read failure', async () => { + const error = new Error('Not Found') + error.response = { status: 404 } + axios.put.mockRejectedValueOnce(error) + + const result = await markRead(605) + + expect(result).toBe(false) + expect(axios.put).toHaveBeenCalledTimes(1) + expect(showMessage.value).toBe(true) + }) + + it('keeps a still-unconfirmed read article out of a concurrent background refetch', async () => { + let resolvePut + axios.put.mockReturnValueOnce(new Promise(resolve => { resolvePut = resolve })) + + const done = markRead(603) // simulates a page-turn's fire-and-forget mark-read, still in flight + + // A background sync's trailing fetchData() resolves while that PUT is + // still pending — the server hasn't committed the read yet, so it's + // still in the response. + axios.get.mockResolvedValueOnce({ + data: { + feeds: [{ + title: 'Feed A', + items: [{ id: 603, title: 'A1', content: '', url: 'https://example.test/a1', timestamp: '2026-01-01 10:00:00' }], + }], + }, + }) + await fetchData() + + expect(feeds.value).toEqual([]) + + resolvePut({ status: 200 }) + await done + }) + + it('excludes an item whose mark-read PUT settles between a refetch\'s request and its response', async () => { + // The PUT commits (and clears the pending id) while the GET below is + // still in flight — the GET's response nonetheless reflects the DB as + // of before that commit, still reporting the item unread. A snapshot + // taken only once the GET resolves would miss this; one taken before + // it's dispatched (unioned with the live state) catches it too. + axios.put.mockResolvedValueOnce({ status: 200 }) + const markDone = markRead(604) + + let resolveGet + axios.get.mockReturnValueOnce(new Promise(resolve => { resolveGet = resolve })) + const fetchPromise = fetchData() + + await markDone // the PUT (and pending-id cleanup) settles first... + + resolveGet({ // ...then the already-in-flight GET resolves, still unread. + data: { + feeds: [{ + title: 'Feed A', + items: [{ id: 604, title: 'A1', content: '', url: 'https://example.test/a1', timestamp: '2026-01-01 10:00:00' }], + }], + }, + }) + await fetchPromise + + expect(feeds.value).toEqual([]) + }) + + it('keeps an item withheld until every overlapping markRead() call for it settles', async () => { + // nextArticle()/prevArticle() can both re-mark the same article (page + // forward, back, forward again) — two overlapping calls for one id. + let resolveFirst + let resolveSecond + axios.put + .mockReturnValueOnce(new Promise(resolve => { resolveFirst = resolve })) + .mockReturnValueOnce(new Promise(resolve => { resolveSecond = resolve })) + + const first = markRead(606) + const second = markRead(606) + + resolveSecond({ status: 200 }) // the second (later) call settles first + await second + + // The first call is still in flight — a refetch must still withhold it. + axios.get.mockResolvedValueOnce({ + data: { + feeds: [{ + title: 'Feed A', + items: [{ id: 606, title: 'A1', content: '', url: 'https://example.test/a1', timestamp: '2026-01-01 10:00:00' }], + }], + }, + }) + await fetchData() + expect(feeds.value).toEqual([]) + + resolveFirst({ status: 200 }) + await first + }) + }) + + describe('markAllRead resilience', () => { + const RETRY_BACKOFF_SUM_MS = 300 + 1000 + + afterEach(() => { + vi.useRealTimers() + }) + + it('reports a partial failure instead of claiming full success', async () => { + feeds.value = [ + { id: 701, title: 'First' }, + { id: 702, title: 'Second' }, + ] + vi.spyOn(window, 'confirm').mockReturnValue(true) + axios.put + .mockResolvedValueOnce({ status: 200 }) // 701 succeeds + .mockRejectedValue(new Error('network')) // 702 fails every attempt + + vi.useFakeTimers() + const done = markAllRead() + await vi.advanceTimersByTimeAsync(RETRY_BACKOFF_SUM_MS) + await done + + expect(message.value).toContain('1 of 2') + expect(message.value).not.toBe('All articles marked as read.') + }) + }) + describe('feed filter', () => { const twoFeedsResponse = { data: { diff --git a/vue/src/composables/useFeeds.js b/vue/src/composables/useFeeds.js index e77615e..49ffd2d 100644 --- a/vue/src/composables/useFeeds.js +++ b/vue/src/composables/useFeeds.js @@ -228,12 +228,74 @@ async function getReadable(feed, index) { } } +// Ids with a mark-read PUT in flight (or being retried) — see markRead() and +// fetchData() below. A refcount rather than a plain Set: nextArticle()/ +// prevArticle() can both re-mark the same article (page forward, back, +// forward again), so two overlapping markRead() calls for one id must both +// finish before fetchData() is allowed to trust the server's word on it — +// otherwise the first call to settle would clear the id out from under the +// second, still-in-flight one. +const pendingReadCounts = new Map() + +function addPendingRead(id) { + pendingReadCounts.set(id, (pendingReadCounts.get(id) ?? 0) + 1) +} + +function removePendingRead(id) { + const count = pendingReadCounts.get(id) ?? 0 + if (count <= 1) { + pendingReadCounts.delete(id) + } else { + pendingReadCounts.set(id, count - 1) + } +} + +function delay(ms) { + return new Promise(resolve => setTimeout(resolve, ms)) +} + +// Short backoff between retries of a failed mark-read PUT — each entry is the +// wait before that retry attempt. +const MARK_READ_RETRY_DELAYS_MS = [300, 1000] +// Bounds how long a hung request (dead connection, captive portal — axios has +// no default timeout) can keep an id "pending": without this, a request that +// never settles would never leave pendingReadCounts, silently withholding +// that article from every fetchData() for the rest of the session. +const MARK_READ_TIMEOUT_MS = 10000 + +// 4xx responses (expired token, item not found/not owned — see +// src/reader/mark_read.rs) mean the request itself is wrong and won't +// succeed on retry; only a missing response (network error, timeout) or a +// server-side/rate-limit status is worth retrying. +function isRetryableMarkReadError(error) { + const status = error.response?.status + return status === undefined || status >= 500 || status === 429 +} + +// Resolves to true once the article is confirmed read server-side, or false +// once retries (if any) are exhausted / the error isn't retryable. async function markRead(id) { + addPendingRead(id) try { - const response = await axios.put("/api/v1/article/read/" + id, null, authHeaders()) - console.log(response.status) - } catch (error) { - console.log(error) + for (let attempt = 0; ; attempt++) { + try { + await axios.put("/api/v1/article/read/" + id, null, { ...authHeaders(), timeout: MARK_READ_TIMEOUT_MS }) + return true + } catch (error) { + console.error('Error marking article read:', error) + if (attempt >= MARK_READ_RETRY_DELAYS_MS.length || !isRetryableMarkReadError(error)) { + // Out of retries, or a retry can't help — this mark never committed + // server-side, so the item genuinely is still unread there. Surface + // it rather than letting it silently vanish from the local list now + // only to mysteriously reappear as unread later. + showMessageForXSeconds('Could not mark an article as read. It may reappear as unread.', 5) + return false + } + await delay(MARK_READ_RETRY_DELAYS_MS[attempt]) + } + } + } finally { + removePendingRead(id) } } @@ -265,15 +327,28 @@ async function setFeedFilter(title) { const fetchData = async () => { const user_id = localStorage.getItem("user-id") try { + // Snapshot ids pending *before* the GET goes out, not just when its + // response comes back. A markRead() PUT that commits while this GET is + // in flight is invisible to the check below (its id has already been + // cleared by the time the response arrives) even though the server read + // the DB before that commit and so still reported the item unread — this + // snapshot catches that case too. + const pendingBeforeFetch = new Set(pendingReadCounts.keys()) const response = await axios.get("/api/v1/article/get/" + user_id, authHeaders()); const items = []; response.data.feeds.forEach(feed => { feed.items.forEach(item => items.push({ ...item, feedTitle: feed.title })); }); + // An item pending at either end of this request may not have committed + // server-side yet, so the server can still report it unread here. Drop it + // from this snapshot too — otherwise a concurrent background sync's + // refetch (see sync()) would resurrect it as unread out from under the + // user while markRead() is still confirming or retrying it. + const freshItems = items.filter(item => !pendingBeforeFetch.has(item.id) && !pendingReadCounts.has(item.id)); // timestamps are zero-padded "YYYY-MM-DD HH:MM:SS" strings, so a plain // lexicographic comparison sorts them chronologically. - items.sort((a, b) => b.timestamp.localeCompare(a.timestamp)); - allItems.value = items; + freshItems.sort((a, b) => b.timestamp.localeCompare(a.timestamp)); + allItems.value = freshItems; applyFilter(); refreshUnreadDisplay(); await nextTick(); @@ -411,9 +486,18 @@ async function markAllRead() { allItems.value = allItems.value.filter(feed => !readIds.has(feed.id)) currentIndex.value = 0 refreshUnreadDisplay() - // markRead swallows its own errors, so Promise.all can't reject here. - await Promise.all(ids.map(id => markRead(id))) - showMessageForXSeconds('All articles marked as read.', 5) + // markRead() resolves to true/false rather than rejecting, so Promise.all + // can't reject here — but a false means that article is still unread + // server-side, which the message below must not contradict. + const results = await Promise.all(ids.map(id => markRead(id))) + const failedCount = results.filter(ok => !ok).length + if (failedCount === 0) { + showMessageForXSeconds('All articles marked as read.', 5) + } else { + // markRead() already surfaced each individual failure — this summarizes + // the batch outcome instead of claiming full success over it. + showMessageForXSeconds(`Marked ${ids.length - failedCount} of ${ids.length} as read; ${failedCount} failed and may reappear.`, 5) + } } function markCurrentArticleRead() {