fix: tie the feed cache to its feed and expire it, clamp cache_interval

README.md

@@ -54,6 +54,8 @@ Every deploy waits for approval in its GitHub environment.
 1. **Stage:** merge to `main`. The push deploys the main app and every feed app with a stage id to stage.
 2. **Production:** after checking stage, tag that commit on `main` with the next CalVer version, `vYY.M.PATCH` (for example `v26.11.0`, then `v26.11.1`; check `git tag --sort=-v:refname | head -1` for the latest), and push the tag. This deploys the main app and every feed app with a production id to production. Tags not on `main` are refused, and tags that aren't `v` followed by a digit (such as `vrc1`) are ignored.
 
+   > **Any** tag that starts with `v` and a digit deploys to production, including pre-release tags such as `v26.11.0-rc.1`. Don't push one to test; use stage. (This comes from the shared edge-apps-actions tag filter, so it applies to every Edge App.)
+
 ```bash
 git checkout main && git pull
 git tag vYY.M.PATCH
@@ -98,21 +100,22 @@ The initialize summary also says to set a repo variable. Ignore that for feed ap
 
 - **Its name, description, icon, header title or feed URL:** edit its entry in `deployed-apps.yml` and release as usual. Merging deploys the change to stage, and the next tag deploys it to production. (Changing `app_name` after the app exists doesn't rename it in Screenly; it is only used when the app is created.)
 - **Code or other settings:** change the code or `screenly.yml`. Every feed app picks it up on the next release, with no change to `deployed-apps.yml`.
-- **A one-off redeploy or test:** run **Actions → Update RSS Reader Apps**, from `main` for stage or from a release tag for production. Leave the app key empty for every app, or enter one such as `bbc-news`. The optional overrides (description, header title, icon URL, feed URL) apply to that run only; to keep a change, put it in `deployed-apps.yml`.
+- **A one-off redeploy or test:** run **Actions → Update RSS Reader Apps**, from `main` for stage or from a release tag for production. Leave the app key empty for every app, or enter one such as `bbc-news`. The optional overrides (description, header title, icon URL, feed URL) apply to that run only, so set an app key when you use one; with the key empty, an override changes every feed app. To keep a change, put it in `deployed-apps.yml`.
+- **Redeploying the main RSS Reader app:** a manual run of **Update Edge App** from `main` redeploys stage. For production, re-run the workflow run of the latest release tag; a manual run from a tag does nothing for the main app.
 - **Removing an app from deploys:** clear its id for that environment in `deployed-apps.yml`. This stops deploying to it but doesn't delete the app in Screenly.
 
 ## Configuration
 
-| Setting             | Description                                                                                                  | Type     | Default                                |
-| ------------------- | ------------------------------------------------------------------------------------------------------------ | -------- | -------------------------------------- |
-| `bypass_cors`       | Route the RSS feed URL through the CORS proxy                                                                | optional | `true`                                 |
-| `cache_interval`    | How often (in seconds) to refresh the feed                                                                   | required | `1800`                                 |
-| `display_errors`    | Show errors on screen for debugging purposes                                                                 | optional | `false`                                |
-| `mode`              | Layout: `headlines` (lead story with image, plus a list) or `list` (headlines only, for maximum readability) | optional | `headlines`                            |
-| `override_locale`   | Override the locale used for date formatting (e.g. `en`, `fr`, `de`)                                         | optional | `en`                                   |
-| `override_timezone` | Override the timezone for date display (e.g. `Europe/London`)                                                | optional | system timezone                        |
-| `rss_title`         | Source name shown in the header                                                                              | required | `BBC News`                             |
-| `rss_url`           | URL of the RSS feed to display                                                                               | required | `http://feeds.bbci.co.uk/news/rss.xml` |
+| Setting             | Description                                                                                                                                                                                              | Type     | Default                                |
+| ------------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -------- | -------------------------------------- |
+| `bypass_cors`       | Route the RSS feed URL through the CORS proxy                                                                                                                                                            | optional | `true`                                 |
+| `cache_interval`    | How often (in seconds) to refresh the feed, from 60 to 86400; values outside that range are clamped. If a refresh fails or the feed is empty, the last stories from this feed stay up for up to 24 hours | required | `1800`                                 |
+| `display_errors`    | Show errors on screen for debugging purposes                                                                                                                                                             | optional | `false`                                |
+| `mode`              | Layout: `headlines` (lead story with image, plus a list) or `list` (headlines only, for maximum readability)                                                                                             | optional | `headlines`                            |
+| `override_locale`   | Override the locale used for date formatting (e.g. `en`, `fr`, `de`)                                                                                                                                     | optional | `en`                                   |
+| `override_timezone` | Override the timezone for date display (e.g. `Europe/London`)                                                                                                                                            | optional | system timezone                        |
+| `rss_title`         | Source name shown in the header                                                                                                                                                                          | required | `BBC News`                             |
+| `rss_url`           | URL of the RSS feed to display                                                                                                                                                                           | required | `http://feeds.bbci.co.uk/news/rss.xml` |
 
 ## Example Feeds
 

screenly.yml

@@ -21,7 +21,9 @@ settings:
     title: Cache Interval
     optional: false
     help_text: |
-      Specify the interval (in seconds) used to periodically cache feeds.
+      How often, in seconds, to refresh the feed (60 to 86400; values
+      outside that range are clamped). If a refresh fails, the last stories
+      from this feed stay on screen for up to 24 hours.
   display_errors:
     type: string
     default_value: 'false'

src/main.ts

@@ -18,6 +18,7 @@ import {
 import { getNewsMode, type NewsMode } from './mode'
 import {
   getTitleSizeClass,
+  getRefreshIntervalMs,
   loadCache,
   resizeImageUrl,
   saveCache,
@@ -196,23 +197,26 @@ async function loadAndRender(
 ) {
   try {
     const entries = await fetchFeed(config)
-    if (entries.length === 0) {
-      showError(els)
+    if (entries.length > 0) {
+      saveCache(config.rssUrl, entries)
+      render(els, mode, entries)
+      showBody(els)
       return
     }
-    saveCache(entries)
-    render(els, mode, entries)
-    showBody(els)
+    // An empty feed is usually a hiccup at the source; keep showing the last
+    // stories rather than blanking the screen
+    console.warn('RSS feed returned no entries')
   } catch (err) {
     console.error('RSS fetch failed:', err)
-    const cached = loadCache()
-    if (cached.length === 0) {
-      showError(els)
-      return
-    }
-    render(els, mode, cached)
-    showBody(els)
   }
+
+  const cached = loadCache(config.rssUrl)
+  if (cached.length === 0) {
+    showError(els)
+    return
+  }
+  render(els, mode, cached)
+  showBody(els)
 }
 
 function updateDate(els: NewsElements, config: FeedConfig) {
@@ -257,8 +261,9 @@ document.addEventListener('DOMContentLoaded', async () => {
     locale: await getLocale(),
     timezone: await getTimeZone(),
   }
-  const cacheInterval =
-    getSettingWithDefault<number>('cache_interval', 1800) * 1000
+  const cacheInterval = getRefreshIntervalMs(
+    getSettingWithDefault<number>('cache_interval', 1800),
+  )
 
   updateDate(els, config)
   setInterval(() => updateDate(els, config), DATE_UPDATE_INTERVAL_MS)

src/utils.test.ts

@@ -1,5 +1,7 @@
 import { describe, test, expect, beforeEach } from 'bun:test'
 import {
+  CACHE_MAX_AGE_MS,
+  getRefreshIntervalMs,
   getTitleSizeClass,
   loadCache,
   resizeImageUrl,
@@ -10,14 +12,30 @@ import type { RssEntry } from './utils'
 
 const localStorageMock = (() => {
   let store: Record<string, string> = {}
+  let quota = Infinity
   return {
     getItem: (key: string) => store[key] ?? null,
     setItem: (key: string, value: string) => {
+      if (!(key in store) && Object.keys(store).length >= quota) {
+        throw new DOMException('quota exceeded', 'QuotaExceededError')
+      }
       store[key] = value
     },
+    setQuota: (keys: number) => {
+      quota = keys
+    },
+    removeItem: (key: string) => {
+      delete store[key]
+    },
+    key: (index: number) => Object.keys(store)[index] ?? null,
+    get length() {
+      return Object.keys(store).length
+    },
     clear: () => {
       store = {}
+      quota = Infinity
     },
+    keys: () => Object.keys(store),
   }
 })()
 
@@ -58,29 +76,142 @@ describe('stripHtml', () => {
   })
 })
 
+const BBC = 'http://feeds.bbci.co.uk/news/rss.xml'
+const NPR = 'https://feeds.npr.org/1004/rss.xml'
+const NOW = 1_800_000_000_000
+
 describe('loadCache / saveCache', () => {
   beforeEach(() => {
     localStorageMock.clear()
   })
 
   test('returns empty array when cache is empty', () => {
-    expect(loadCache()).toEqual([])
+    expect(loadCache(BBC, NOW)).toEqual([])
+  })
+
+  test('returns saved entries for the same feed', () => {
+    saveCache(BBC, ENTRIES, NOW)
+    expect(loadCache(BBC, NOW)).toEqual(ENTRIES)
+  })
+
+  test('overwrites previous cache for the same feed', () => {
+    saveCache(BBC, ENTRIES, NOW)
+    saveCache(BBC, [ENTRIES[0]], NOW)
+    expect(loadCache(BBC, NOW)).toEqual([ENTRIES[0]])
+  })
+
+  test("never returns another feed's stories", () => {
+    // e.g. rss_url changed and the first fetch of the new feed failed
+    saveCache(BBC, ENTRIES, NOW)
+    expect(loadCache(NPR, NOW)).toEqual([])
+  })
+
+  test('keeps separate caches per feed', () => {
+    saveCache(BBC, ENTRIES, NOW)
+    saveCache(NPR, [ENTRIES[1]], NOW)
+    expect(loadCache(BBC, NOW)).toEqual(ENTRIES)
+    expect(loadCache(NPR, NOW)).toEqual([ENTRIES[1]])
   })
 
-  test('returns saved entries', () => {
-    saveCache(ENTRIES)
-    expect(loadCache()).toEqual(ENTRIES)
+  test('expires after CACHE_MAX_AGE_MS', () => {
+    saveCache(BBC, ENTRIES, NOW)
+    expect(loadCache(BBC, NOW + CACHE_MAX_AGE_MS)).toEqual(ENTRIES)
+    expect(loadCache(BBC, NOW + CACHE_MAX_AGE_MS + 1)).toEqual([])
   })
 
-  test('overwrites previous cache', () => {
-    saveCache(ENTRIES)
-    saveCache([ENTRIES[0]])
-    expect(loadCache()).toEqual([ENTRIES[0]])
+  test('ignores the legacy shared cache and removes it on save', () => {
+    localStorage.setItem(
+      'rssStore',
+      JSON.stringify({ entries: ENTRIES, timestamp: NOW }),
+    )
+    expect(loadCache(BBC, NOW)).toEqual([])
+    saveCache(BBC, [ENTRIES[0]], NOW)
+    expect(localStorageMock.keys()).not.toContain('rssStore')
+  })
+
+  test("prunes other feeds' expired caches on save, keeps fresh ones", () => {
+    saveCache(NPR, ENTRIES, NOW)
+    saveCache(BBC, ENTRIES, NOW + 1000)
+    expect(loadCache(NPR, NOW + 1000)).toEqual(ENTRIES)
+
+    saveCache(BBC, ENTRIES, NOW + CACHE_MAX_AGE_MS + 1)
+    expect(localStorageMock.keys()).toEqual([`rssStore:${BBC}`])
+  })
+})
+
+describe('cache recovery and validation', () => {
+  beforeEach(() => {
+    localStorageMock.clear()
+  })
+
+  test('recovers when storage is full of expired caches', () => {
+    saveCache(NPR, ENTRIES, NOW)
+    localStorageMock.setQuota(1)
+    saveCache(BBC, ENTRIES, NOW + CACHE_MAX_AGE_MS + 1)
+    expect(loadCache(BBC, NOW + CACHE_MAX_AGE_MS + 1)).toEqual(ENTRIES)
+  })
+
+  test('treats a cache saved in the future as stale', () => {
+    // e.g. saved while the player's clock was ahead, before it synced
+    saveCache(BBC, ENTRIES, NOW + 60_000)
+    expect(loadCache(BBC, NOW)).toEqual([])
+  })
+
+  test('rejects a cache with any malformed entry', () => {
+    const store = (entries: unknown) =>
+      localStorage.setItem(
+        `rssStore:${BBC}`,
+        JSON.stringify({ feedUrl: BBC, timestamp: NOW, entries }),
+      )
+    store([null])
+    expect(loadCache(BBC, NOW)).toEqual([])
+    store([ENTRIES[0], { content: 'no title' }])
+    expect(loadCache(BBC, NOW)).toEqual([])
+    store(['just a string'])
+    expect(loadCache(BBC, NOW)).toEqual([])
+  })
+
+  test('fills in missing optional fields of a cached entry', () => {
+    localStorage.setItem(
+      `rssStore:${BBC}`,
+      JSON.stringify({
+        feedUrl: BBC,
+        timestamp: NOW,
+        entries: [{ title: 'T' }],
+      }),
+    )
+    expect(loadCache(BBC, NOW)).toEqual([
+      { title: 'T', content: '', formattedDate: '', imageUrl: '', source: '' },
+    ])
   })
 
   test('returns empty array for malformed cache', () => {
-    localStorage.setItem('rssStore', 'not-json')
-    expect(loadCache()).toEqual([])
+    localStorage.setItem(`rssStore:${BBC}`, 'not-json')
+    expect(loadCache(BBC, NOW)).toEqual([])
+    localStorage.setItem(`rssStore:${BBC}`, JSON.stringify({ feedUrl: BBC }))
+    expect(loadCache(BBC, NOW)).toEqual([])
+    localStorage.setItem(`rssStore:${BBC}`, 'null')
+    expect(loadCache(BBC, NOW)).toEqual([])
+  })
+})
+
+describe('getRefreshIntervalMs', () => {
+  test('uses a normal value as is', () => {
+    expect(getRefreshIntervalMs(1800)).toBe(1_800_000)
+    expect(getRefreshIntervalMs(60)).toBe(60_000)
+  })
+
+  test('raises values that would refetch in a tight loop', () => {
+    expect(getRefreshIntervalMs(0)).toBe(60_000)
+    expect(getRefreshIntervalMs(-5)).toBe(60_000)
+    expect(getRefreshIntervalMs(1)).toBe(60_000)
+    expect(getRefreshIntervalMs(NaN)).toBe(60_000)
+  })
+
+  test('caps values that would overflow setInterval', () => {
+    // setInterval fires immediately for delays above 2^31 - 1 ms (~24.8 days)
+    expect(getRefreshIntervalMs(Infinity)).toBe(86_400_000)
+    expect(getRefreshIntervalMs(10_000_000)).toBe(86_400_000)
   })
 })