fix: stop the screen client serving stale or blank content after a degraded pull

CHANGELOG.md

@@ -4,6 +4,19 @@ All notable changes to this project will be documented in this file.
 
 ## [Unreleased]
 
+- Fixed a region that changes type — between the default and touch-button renderers under the same
+  region id — going blank until the next pull delivered different content, because the outgoing
+  component's cleanup dropped the region's scheduled slides before the incoming one asked for them.
+- Fixed absent slides counting as content, which suppressed the fallback image and left the screen
+  black on a region that held nothing else.
+- Fixed the screen client caching an error response as its configuration, which left `apiEndpoint`
+  undefined for the whole config interval and so failed every request made in it.
+- Removed the screen client's template requests. Templates are bundled into the client, and the only
+  thing it took from `/v2/templates/{id}` was the id already carried by the slide, so the request
+  cost one round trip per template per pull and emptied a region whenever it was throttled (#507).
+- Fixed a slide whose template the client cannot resolve taking down its whole region: the error was
+  thrown before the slide's own error boundary mounted, so the region's boundary caught it instead
+  and, having no way to reset, showed the error fallback until the client was reloaded.
 - Fixed the screen client treating a collection cut short by the 100-page backstop as a complete one,
   which cached the truncated data behind the server's checksum until an editor changed the content
   (#507). The collection is now given up, the same as when a page request fails.
@@ -34,6 +47,28 @@ All notable changes to this project will be documented in this file.
   `NGINX_RATE_LIMIT_BURST` and answers `429` instead of `503`.
 - Made the screen client retry throttled and temporarily failing API requests with backoff, and
   bounded how many requests one pull may have in flight (#507).
+- Fixed the screen client caching a partially failed pull as if it were complete, which left a
+  region, layout or media item stale until someone edited the content in the admin (#507).
+- Made the screen client wait for a pull to finish before starting the next one, so a slow pull can
+  no longer have a second one stacked on top of it (#507).
+- Added a timeout to the client config request, so a request that never answers can no longer stall
+  the screen client's polling (#507).
+- Fixed the screen client showing stale playlist content until the layout happened to change, where a
+  pull that served the layout from cache handed the regions an unchanged object, so they never asked
+  for the slides the pull had just fetched (#507).
+- Removed a tenant config request and a colour-scheme rebuild that the screen client repeated on
+  every pull (#507).
+- Fixed a failed layout request blanking the screen client, where every region was unmounted and
+  playback restarted from the first slide on the next successful pull (#507). The last known good
+  layout is kept instead, unless it belongs to a campaign or to a layout the screen has since been
+  moved away from.
+- Fixed the screen client suppressing the fallback image while showing nothing, where slides the
+  regions had dropped were still counted as content, so a screen with no renderable slides went
+  black instead (#507).
+- Fixed a failed slides request emptying a playlist whose region had loaded fine, which lost that
+  playlist's content for a whole pull (#507). The slides the previous pull loaded are kept instead.
+- Fixed the screen client pairing slides and playlists with the previous pull by position rather
+  than by id, so reordering a playlist in the admin could pair a slide with another slide's media.
 
 ## [3.0.0-rc8] - 2026-08-24
 

README.md

@@ -753,7 +753,7 @@ compose `environment:` with defaults in `infrastructure/nginx/Dockerfile`:
 | `NGINX_RATE_LIMIT_BURST` | Requests allowed to exceed the rate before rejection | `500` |
 
 Size these for the screen client, not for a browser. One pull from a screen on a multi-region layout is
-a burst of one request per region, playlist, slide, template, media and feed — easily a few hundred
+a burst of one request per region, playlist, slide, media and feed — easily a few hundred
 requests within a couple of seconds. Setting the limit too low makes regions and images randomly fail to
 render.
 

assets/client/app.jsx

@@ -276,12 +276,17 @@ function App({ preview, previewId }) {
     };
   }, []);
 
+  // Keyed on the id, not the screen object: the screen is emitted on every pull,
+  // and loadTenantConfig is an api request - one per pull is the fan-out #507 is
+  // about.
+  const screenId = screen?.["@id"];
+
   useEffect(() => {
-    if (screen && screen["@id"]) {
-      releaseService.setScreenIdInUrl(screen["@id"]);
+    if (screenId) {
+      releaseService.setScreenIdInUrl(screenId);
       tenantService.loadTenantConfig();
     }
-  }, [screen]);
+  }, [screenId]);
 
   return (
     <div className="app" style={appStyle}>

assets/client/components/region.jsx

@@ -3,6 +3,7 @@ import { createGridArea } from "../../shared/grid-generator/grid-generator";
 import { TransitionGroup, CSSTransition } from "react-transition-group";
 import ErrorBoundary from "./error-boundary.jsx";
 import idFromPath from "../util/id-from-path";
+import isRenderableSlide from "../util/is-renderable-slide";
 import logger from "../logger/logger";
 import Slide, { MIN_SLIDE_DWELL_MS } from "./slide.jsx";
 import nextRunId from "../../shared/slide-utils/next-run-id.js";
@@ -105,8 +106,9 @@ function Region({ region }) {
   function regionContentListener(event) {
     const receivedSlides = [...event.detail.slides];
 
-    // Filter out invalid slides.
-    setNewSlides(receivedSlides.filter((slide) => !slide.invalid));
+    // Filter out invalid slides. Shared with ScheduleService.checkForEmptyContent
+    // so the two cannot disagree about what counts as content.
+    setNewSlides(receivedSlides.filter(isRenderableSlide));
   }
 
   // Setup event listener for region content.
@@ -137,15 +139,18 @@ function Region({ region }) {
     };
   }, []);
 
-  // Notify that region is ready.
+  // Notify that region is ready. Mount only: content is pushed by ScheduleService
+  // from here on, and asking again whenever the region prop changes identity was
+  // both redundant and unreliable - a pull that served the layout from cache
+  // hands back the same object, so the effect never ran (#507).
   useEffect(() => {
     const event = new CustomEvent("regionReady", {
       detail: {
         id: regionId,
       },
     });
     document.dispatchEvent(event);
-  }, [region]);
+  }, []);
 
   // Start the progress if no slide is currently playing.
   useEffect(() => {

assets/client/components/screen.jsx

@@ -68,8 +68,13 @@ function Screen({ screen }) {
     });
   };
 
+  // Keyed on the flag, not the screen object: the screen is emitted on every pull,
+  // and re-running this strips color-scheme-* off the html element before an async
+  // config load puts it back - a theme flash whenever that load is not cached.
+  const enableColorSchemeChange = screen?.enableColorSchemeChange;
+
   useEffect(() => {
-    if (screen?.enableColorSchemeChange) {
+    if (enableColorSchemeChange) {
       logger.info("Enabling color scheme change.");
       refreshColorScheme();
       // Refresh color scheme every 5 minutes.
@@ -90,7 +95,7 @@ function Screen({ screen }) {
         "color-scheme-dark",
       );
     };
-  }, [screen]);
+  }, [enableColorSchemeChange]);
 
   return (
     <div className="screen" style={rootStyle} id={screen["@id"]}>

assets/client/components/slide.jsx

@@ -13,6 +13,27 @@ import "./slide.scss";
 // the next advance -- keep region.scss's opacity transition in step.
 export const MIN_SLIDE_DWELL_MS = 1000;
 
+/**
+ * Render the slide's template.
+ *
+ * A component rather than a call in Slide's own render: renderSlide throws when
+ * the slide names a template this build does not bundle, and an argument is
+ * evaluated before ErrorBoundary mounts. The throw escaped the boundary meant
+ * to contain it and hit the region's instead, which has no handler and never
+ * resets - so one unrenderable slide replaced its whole region with the error
+ * fallback until the client was reloaded. Thrown from inside the boundary's
+ * subtree it costs that one slide, which is then moved on from.
+ *
+ * @param {object} props - Props.
+ * @param {object} props.slide - The slide data.
+ * @param {number} props.run - Run id. Changes each time the slide should run.
+ * @param {Function} props.slideDone - The function to call when the slide is done running.
+ * @returns {object} - The rendered template.
+ */
+function SlideTemplate({ slide, run, slideDone }) {
+  return renderSlide(slide, run, slideDone);
+}
+
 /**
  * Slide component.
  *
@@ -137,7 +158,11 @@ function Slide({ slide, id, run, slideDone, slideError, forwardRef }) {
       data-execution-id={slide.executionId}
     >
       <ErrorBoundary errorHandler={handleError}>
-        {renderSlide(slide, run, slideDoneAfterMinimumDwell)}
+        <SlideTemplate
+          slide={slide}
+          run={run}
+          slideDone={slideDoneAfterMinimumDwell}
+        />
       </ErrorBoundary>
     </div>
   );

assets/client/components/touch-region.jsx

@@ -97,15 +97,15 @@ function TouchRegion({ region }) {
     };
   }, []);
 
-  // Notify that region is ready.
+  // Notify that region is ready. Mount only, see Region.
   useEffect(() => {
     const event = new CustomEvent("regionReady", {
       detail: {
         id: regionId,
       },
     });
     document.dispatchEvent(event);
-  }, [region]);
+  }, []);
 
   // Make sure current slide is set.
   useEffect(() => {

assets/client/data-sync/api-helper.js

@@ -1,5 +1,8 @@
 import logger from "../logger/logger";
 import appStorage from "../util/app-storage";
+import fetchWithTimeout, {
+  REQUEST_TIMEOUT,
+} from "../util/fetch-with-timeout.js";
 
 // Statuses that mean "try again later" rather than "this failed".
 // 429 is the rate limit response from the reverse proxy, 502/503/504 are
@@ -20,9 +23,11 @@ const RETRY_BASE_DELAY = 500;
 // for an hour inside a poll that runs every few minutes.
 const MAX_RETRY_DELAY = 30000;
 
-// Give up on a single request after this long. A socket that never answers
-// neither fails nor retries, which is the one case backoff cannot help.
-const REQUEST_TIMEOUT = 15000;
+// Spread applied on top of a Retry-After the server sent. Every client rejected
+// in the same second is handed the same value, so honouring it verbatim would
+// re-synchronise the burst the header exists to spread. Added rather than
+// subtracted: RFC 9110 makes Retry-After a minimum, not a target.
+const RETRY_AFTER_JITTER = 250;
 
 // Backstop so a misbehaving collection cannot page indefinitely. Matches the
 // limit used by the admin's get-all-pages helper.
@@ -68,7 +73,9 @@ class ApiHelper {
     );
 
     if (!Number.isNaN(retryAfter) && retryAfter > 0) {
-      return Math.min(retryAfter * 1000, MAX_RETRY_DELAY);
+      const asked = Math.min(retryAfter * 1000, MAX_RETRY_DELAY);
+
+      return asked + Math.floor(Math.random() * RETRY_AFTER_JITTER);
     }
 
     // Full jitter: anywhere in [0, capped]. Spreads a burst that was rejected
@@ -150,17 +157,12 @@ class ApiHelper {
 
     logger.log("info", `Fetching: ${this.endpoint + path}`);
 
-    // A request that never answers would otherwise sit in Promise.allSettled
-    // forever, holding a worker and stalling the pull.
-    const controller = new AbortController();
-    const timeout = setTimeout(() => controller.abort(), REQUEST_TIMEOUT);
     let response;
 
     try {
-      response = await fetch(this.endpoint + path, {
-        headers,
-        signal: controller.signal,
-      });
+      // A request that never answers would otherwise sit in the fan-out
+      // forever, holding a worker and stalling the pull.
+      response = await fetchWithTimeout(this.endpoint + path, { headers });
     } catch (err) {
       const timedOut = err.name === "AbortError";
 
@@ -172,8 +174,6 @@ class ApiHelper {
 
       // A transport error and a timeout are both worth another attempt.
       return { data: null, retry: true, status: null, response: null };
-    } finally {
-      clearTimeout(timeout);
     }
 
     if (response.ok === false) {