Keep one junk thumbnail from taking the page down with it

sxyprn's "latest" listing sends items whose thumb is the bare string
"https:". Resolved against the page -- which is what new URL(url, location)
does with anything that isn't absolute -- that becomes our own address, so
the card raced our own HTML as if it were a picture, pinned our origin to
the proxy for the rest of the session, and asked /api/image to fetch
"https:" (a 400, every time). An address that doesn't stand on its own is
no thumbnail at all, and is now treated as one: no src, no race, no
request, and the card keeps its placeholder.

While in here: a thumbnail that failed had exactly one more chance, the
proxy, and images already on the proxy route had none at all -- so one
refused connection left a card empty for as long as it stayed mounted. A
failure now walks a short ladder instead: the other route, then both again
after a pause. Bounded and backed off, and dropped the moment the card is
rebound, so a page of genuinely dead images costs a handful of requests
rather than a storm.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QBDkEXP4htyXTCZUwMLphd
This commit is contained in:
Simon
2026-09-18 19:31:05 +00:00
parent 191c81e55e
commit b478c51551
2 changed files with 287 additions and 41 deletions

View File

@@ -114,10 +114,36 @@ App.videos = App.videos || {};
return `${secs}`; return `${secs}`;
}; };
App.videos.buildImageProxyUrl = function(imageUrl) { // A thumbnail URL we can actually fetch, or '' if the provider sent
if (!imageUrl) return ''; // something we can't use.
//
// Providers do send junk: sxyprn's "latest" listing carries items whose
// `thumb` is the bare string "https:". Resolved against the page -- which
// is what `new URL(url, location)` does with anything that isn't absolute
// -- that becomes *our own* address, and the card then races our own HTML
// as if it were a picture, pins our origin to the proxy for the rest of
// the session, and asks /api/image to fetch "https:" (a 400, every time).
// So an address that doesn't stand on its own is treated as no thumbnail
// at all, which is what it is.
const usableThumbUrl = function(url) {
if (!url || typeof url !== 'string') return '';
let parsed;
try { try {
return `/api/image?url=${encodeURIComponent(imageUrl)}`; parsed = new URL(url); // no base: relative input throws
} catch (err) {
return '';
}
if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') return '';
// The provider's own spelling, not the parsed one: a signed URL is only
// valid as it was written, and normalising it can change the bytes.
return url;
};
App.videos.buildImageProxyUrl = function(imageUrl) {
const target = usableThumbUrl(imageUrl);
if (!target) return '';
try {
return `/api/image?url=${encodeURIComponent(target)}`;
} catch (err) { } catch (err) {
return ''; return '';
} }
@@ -177,10 +203,11 @@ App.videos = App.videos || {};
// provider while the host is still unknown -- the optimistic route, and the // provider while the host is still unknown -- the optimistic route, and the
// one a race starts on anyway. // one a race starts on anyway.
App.videos.thumbnailUrl = function(url) { App.videos.thumbnailUrl = function(url) {
if (!url) return ''; const target = usableThumbUrl(url);
return imageRoutes.get(imageHostOf(url)) === IMAGE_PROXY if (!target) return '';
? (App.videos.buildImageProxyUrl(url) || url) return imageRoutes.get(imageHostOf(target)) === IMAGE_PROXY
: url; ? (App.videos.buildImageProxyUrl(target) || target)
: target;
}; };
// A src-less <img> counts as "unavailable", and the browser paints its alt // A src-less <img> counts as "unavailable", and the browser paints its alt
@@ -203,29 +230,76 @@ App.videos = App.videos || {};
img.src = url; img.src = url;
}; };
// Last resort on a route that normally works: one expired or missing image // What a thumbnail that fails is given next, in order.
// shouldn't be left broken just because its host is fine in general. //
const attachProxyFallback = function(img, proxyUrl, token) { // A failure used to mean one more attempt -- the proxy -- and then an empty
if (!proxyUrl) return; // box for as long as the card stayed mounted. That is one blip away from a
// grid with holes in it: a refused connection, our own server busy for a
// moment, an origin that drops one request in twenty, and that card is
// simply blank until the reader happens to scroll it out of the window and
// back. So a failure now walks a short ladder instead: the other route
// first (most failures are one route's fault, not the picture's), then both
// again after a pause. Bounded and backed off, so a page of genuinely dead
// images costs a handful of requests rather than a storm, and abandoned the
// moment the card is rebound.
const RETRY_PAUSE_MS = 900;
const retryPlan = function(directUrl, proxyUrl, route) {
const own = route === IMAGE_PROXY ? proxyUrl : directUrl;
const other = route === IMAGE_PROXY ? directUrl : proxyUrl;
return [
{ url: other, delay: 0 },
{ url: own, delay: RETRY_PAUSE_MS },
{ url: other, delay: RETRY_PAUSE_MS * 3 }
].filter((step) => !!step.url);
};
// Arms `img` with the steps to take if what it is showing fails to load.
const attachRetry = function(img, plan, token) {
// Checked here too, not just in showThumbnail: this *replaces* whatever // Checked here too, not just in showThumbnail: this *replaces* whatever
// fallback the image currently has, so a call arriving for a generation // the image is currently armed with, so a call arriving for a generation
// the element has moved past would take away the live one and leave a // the element has moved past would take away the live plan and leave a
// dead one -- the recycled card's thumbnail would then have no fallback // dead one -- the recycled card's thumbnail would then have nothing
// at all if it failed. // behind it if it failed.
if (token !== undefined && img.dataset.thumbToken !== token) return; if (!img || (token !== undefined && img.dataset.thumbToken !== token)) return;
detachRetry(img);
if (!plan || !plan.length) return;
const steps = plan.slice();
// Held on the element so detachThumbnail can take it off again. On the // Held on the element so detachThumbnail can take it off again. On the
// happy path it never fires and `once` never collects it, so a pooled // happy path it never fires and `once` never collects it, so a pooled
// image would otherwise accumulate one closure per mount it has served. // image would otherwise accumulate one closure per mount it has served.
detachProxyFallback(img); const onError = function() {
const onError = () => { showThumbnail(img, proxyUrl, token); }; img._thumbRetry = null;
img._thumbFallback = onError; const step = steps.shift();
if (!step) return;
const go = function() {
img._thumbRetryTimer = null;
if (token !== undefined && img.dataset.thumbToken !== token) return;
// Re-arm before the src lands, so a step that fails immediately
// (a cached refusal) still hands on to the next one.
attachRetry(img, steps, token);
// The same URL assigned to the same element is not a new src,
// and a browser that sees no change starts no request -- so a
// retry of the route we're already on has to clear it first.
if (img.getAttribute('src') === step.url) img.removeAttribute('src');
showThumbnail(img, step.url, token);
};
if (step.delay > 0) img._thumbRetryTimer = setTimeout(go, step.delay);
else go();
};
img._thumbRetry = onError;
img.addEventListener('error', onError, { once: true }); img.addEventListener('error', onError, { once: true });
}; };
const detachProxyFallback = function(img) { const detachRetry = function(img) {
if (img && img._thumbFallback) { if (!img) return;
img.removeEventListener('error', img._thumbFallback); if (img._thumbRetry) {
img._thumbFallback = null; img.removeEventListener('error', img._thumbRetry);
img._thumbRetry = null;
}
if (img._thumbRetryTimer) {
clearTimeout(img._thumbRetryTimer);
img._thumbRetryTimer = null;
} }
}; };
@@ -238,12 +312,9 @@ App.videos = App.videos || {};
if (!waiting) return; if (!waiting) return;
imageWaiting.delete(host); imageWaiting.delete(host);
waiting.forEach((entry) => { waiting.forEach((entry) => {
if (route === IMAGE_PROXY) { attachRetry(entry.img, retryPlan(entry.directUrl, entry.proxyUrl, route), entry.token);
showThumbnail(entry.img, entry.proxyUrl, entry.token); showThumbnail(entry.img,
return; route === IMAGE_PROXY ? entry.proxyUrl : entry.directUrl, entry.token);
}
if (route !== IMAGE_DIRECT) attachProxyFallback(entry.img, entry.proxyUrl, entry.token);
showThumbnail(entry.img, entry.directUrl, entry.token);
}); });
}; };
@@ -303,12 +374,9 @@ App.videos = App.videos || {};
const giveUp = function() { const giveUp = function() {
if (shown) return; if (shown) return;
shown = true; shown = true;
if (imageRoutes.get(host) === IMAGE_PROXY) { const route = imageRoutes.get(host);
showThumbnail(img, proxyUrl, token); attachRetry(img, retryPlan(directUrl, proxyUrl, route), token);
return; showThumbnail(img, route === IMAGE_PROXY ? proxyUrl : directUrl, token);
}
attachProxyFallback(img, proxyUrl, token);
showThumbnail(img, directUrl, token);
}; };
const decide = function() { const decide = function() {
@@ -390,7 +458,9 @@ App.videos = App.videos || {};
// Points `img` at `url` by whichever route is known to work for its host, // Points `img` at `url` by whichever route is known to work for its host,
// racing the two the first time that host is seen. // racing the two the first time that host is seen.
App.videos.attachThumbnail = function(img, url) { App.videos.attachThumbnail = function(img, url) {
const directUrl = url || (img && img.dataset.thumb) || ''; // An address that isn't one is the same thing as no thumbnail: the card
// keeps its placeholder rather than chasing it. See usableThumbUrl.
const directUrl = usableThumbUrl(url || (img && img.dataset.thumb) || '');
if (!img) return; if (!img) return;
// Held back until there is an image to caption -- see showThumbnail. An // Held back until there is an image to caption -- see showThumbnail. An
// item with no thumbnail keeps an empty alt: the card's own title sits // item with no thumbnail keeps an empty alt: the card's own title sits
@@ -415,6 +485,7 @@ App.videos = App.videos || {};
img.dataset.thumbToken = token; img.dataset.thumbToken = token;
if (route === IMAGE_PROXY) { if (route === IMAGE_PROXY) {
attachRetry(img, retryPlan(directUrl, proxyUrl, IMAGE_PROXY), token);
showThumbnail(img, proxyUrl || directUrl, token); showThumbnail(img, proxyUrl || directUrl, token);
return; return;
} }
@@ -429,8 +500,8 @@ App.videos = App.videos || {};
} }
if (route === IMAGE_DIRECT || !host || !proxyUrl) { if (route === IMAGE_DIRECT || !host || !proxyUrl) {
// Known good, or nothing to race against: take the provider and keep // Known good, or nothing to race against: take the provider and keep
// the proxy as this image's own fallback. // the proxy behind it.
attachProxyFallback(img, proxyUrl, token); attachRetry(img, retryPlan(directUrl, proxyUrl, IMAGE_DIRECT), token);
showThumbnail(img, directUrl, token); showThumbnail(img, directUrl, token);
return; return;
} }
@@ -438,13 +509,13 @@ App.videos = App.videos || {};
}; };
// Voids whatever is still in flight for this element's thumbnail. Its // Voids whatever is still in flight for this element's thumbnail. Its
// generation moves on, so a race that settles later, or a proxy fallback // generation moves on, so a race that settles later, or a retry that fires
// that fires later, finds a token that no longer matches and does nothing. // later, finds a token that no longer matches and does nothing.
App.videos.detachThumbnail = function(img) { App.videos.detachThumbnail = function(img) {
if (!img) return; if (!img) return;
img.dataset.thumbToken = String(++thumbSeq); img.dataset.thumbToken = String(++thumbSeq);
delete img.dataset.alt; delete img.dataset.alt;
detachProxyFallback(img); detachRetry(img);
}; };
// Each channel in a group sends back a different number of videos per // Each channel in a group sends back a different number of videos per

175
tests/smoke_thumbnails.py Normal file
View File

@@ -0,0 +1,175 @@
#!/usr/bin/env python3
"""Thumbnail smoke tests: junk URLs, and images that fail once.
Run against a locally running backend:
backend/main.py &
.venv/bin/python tests/smoke_thumbnails.py
The listing and the image hosts are both served by this script rather than by a
provider, because what's under test is what the client does with awkward data:
* a `thumb` that isn't a URL at all -- sxyprn's "latest" listing sends items
whose thumb is the bare string "https:". Resolved against the page that is
*our own* address, so the card used to race our own HTML as if it were a
picture, pin our origin to the proxy for the rest of the session, and ask
/api/image to fetch "https:" (a 400, every time).
* a thumbnail whose first request fails. One blip used to mean an empty box
for as long as the card stayed mounted.
"""
import base64
import json
import sys
from playwright.sync_api import sync_playwright
BASE = "http://127.0.0.1:5000/"
SERVER = "https://hottubapp.io"
CHANNEL = "xvideos"
CDN = "https://cdn.example-thumbs.test"
# 1x1 transparent PNG.
PIXEL = base64.b64decode(
"iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAAC0lEQVR42mNkYPhfDwAChwGA60e6kgAAAABJRU5ErkJggg==")
SEED = """([server, channel]) => {
localStorage.setItem('config', JSON.stringify({ servers: [{ [server]: {} }] }));
localStorage.setItem('preferences', JSON.stringify({ [server]: { channelId: channel } }));
localStorage.removeItem('session');
localStorage.setItem('favorites', JSON.stringify([]));
}"""
STATE = """() => Array.from(document.querySelectorAll('#video-grid .video-card')).map((card) => {
const img = card.querySelector('img');
return {
id: card.dataset.videoId,
src: img ? img.getAttribute('src') || '' : '',
loaded: img ? img.naturalWidth > 0 : false,
};
})"""
class Checks:
def __init__(self):
self.failed = 0
def ok(self, label, condition, detail=""):
mark = "PASS" if condition else "FAIL"
if not condition:
self.failed += 1
print(f" [{mark}] {label}" + (f" -- {detail}" if detail and not condition else ""))
def listing():
"""Twelve ordinary items, one with the junk thumb, one that fails once."""
items = []
for i in range(12):
items.append({
"id": f"test:{i}",
"title": f"Video {i}",
"url": f"{CDN}/watch/{i}",
"channel": "test",
"duration": 60 + i,
"thumb": f"{CDN}/thumb/{i}.png",
"tags": [],
})
items[3]["thumb"] = "https:" # what sxyprn's "latest" actually sends
items[7]["thumb"] = f"{CDN}/flaky.png"
return {"items": items, "pageInfo": {"hasNextPage": False}}
def main():
c = Checks()
image_proxy_calls = []
flaky_hits = {"direct": 0, "proxy": 0}
with sync_playwright() as p:
browser = p.chromium.launch(args=[
"--no-sandbox", "--disable-dev-shm-usage", "--disable-gpu",
])
page = browser.new_page(viewport={"width": 1400, "height": 1000})
def serve_listing(route):
route.fulfill(status=200, content_type="application/json",
body=json.dumps(listing()))
def serve_proxy(route):
# Only the thumbnails matter here; record what the client asked us
# to fetch on its behalf, and hand back the picture.
image_proxy_calls.append(route.request.url)
if "flaky.png" in route.request.url:
flaky_hits["proxy"] += 1
# The flaky picture is refused on *both* routes the first time
# round, which is what used to leave the card empty for good.
if flaky_hits["proxy"] == 1:
route.fulfill(status=502, content_type="text/plain", body="nope")
return
route.fulfill(status=200, content_type="image/png", body=PIXEL)
def serve_cdn(route):
if route.request.url.endswith("/flaky.png"):
flaky_hits["direct"] += 1
if flaky_hits["direct"] == 1:
route.abort("connectionfailed")
return
route.fulfill(status=200, content_type="image/png", body=PIXEL)
page.route("**/api/videos", serve_listing)
page.route("**/api/image*", serve_proxy)
page.route(f"{CDN}/**", serve_cdn)
page.goto(BASE, wait_until="domcontentloaded")
page.evaluate(SEED, [SERVER, CHANNEL])
page.goto(BASE, wait_until="load")
page.wait_for_selector(".video-card", timeout=60000)
# Long enough for the host race (2.5s of patience) and the retry ladder
# (~900ms for its second step) to have run their course.
page.wait_for_timeout(8000)
cards = page.evaluate(STATE)
by_id = {card["id"]: card for card in cards}
print("\na thumb that isn't a URL")
junk = by_id.get("test:3")
c.ok("the card is mounted", junk is not None)
if junk:
c.ok("it asks for nothing at all", junk["src"] == "",
f"src={junk['src']!r}")
c.ok("and nothing is sent to the image proxy for it",
not [u for u in image_proxy_calls if "https%3A&" in u or u.endswith("url=https%3A")],
str([u for u in image_proxy_calls if "https%3A" in u][:2]))
print("\nthe rest of the page is unaffected by it")
others = [card for card in cards if card["id"] != "test:3"]
c.ok("every other card shows its picture",
all(card["loaded"] for card in others),
str([card["id"] for card in others if not card["loaded"]]))
# The junk URL used to resolve to our own origin, whose race then failed
# and pinned it to the proxy -- for everything, for the whole session.
c.ok("our own origin is not pinned to the proxy",
page.evaluate("() => App.videos.thumbnailUrl(location.origin + '/x.png')")
== page.evaluate("() => location.origin + '/x.png'"))
print("\na thumbnail refused on both routes, once")
flaky = by_id.get("test:7")
c.ok("the card is mounted", flaky is not None)
c.ok("both routes were refused once",
flaky_hits["direct"] >= 1 and flaky_hits["proxy"] >= 1,
f"direct={flaky_hits['direct']} proxy={flaky_hits['proxy']}")
c.ok("and it was asked for again after that",
flaky_hits["direct"] + flaky_hits["proxy"] > 2,
f"direct={flaky_hits['direct']} proxy={flaky_hits['proxy']}")
if flaky:
c.ok("so the card ends up showing a picture", flaky["loaded"],
f"src={flaky['src']!r}")
c.ok("and a dead thumbnail stops being asked for",
flaky_hits["direct"] + flaky_hits["proxy"] <= 4,
f"direct={flaky_hits['direct']} proxy={flaky_hits['proxy']}")
browser.close()
print(f"\n{'OK' if not c.failed else 'FAILED'}: {c.failed} check(s) failed")
return 1 if c.failed else 0
sys.exit(main())