diff --git a/frontend/js/videos.js b/frontend/js/videos.js index 7d4e2f0..5a91b93 100644 --- a/frontend/js/videos.js +++ b/frontend/js/videos.js @@ -114,10 +114,36 @@ App.videos = App.videos || {}; return `${secs}`; }; - App.videos.buildImageProxyUrl = function(imageUrl) { - if (!imageUrl) return ''; + // A thumbnail URL we can actually fetch, or '' if the provider sent + // 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 { - 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) { return ''; } @@ -177,10 +203,11 @@ App.videos = App.videos || {}; // provider while the host is still unknown -- the optimistic route, and the // one a race starts on anyway. App.videos.thumbnailUrl = function(url) { - if (!url) return ''; - return imageRoutes.get(imageHostOf(url)) === IMAGE_PROXY - ? (App.videos.buildImageProxyUrl(url) || url) - : url; + const target = usableThumbUrl(url); + if (!target) return ''; + return imageRoutes.get(imageHostOf(target)) === IMAGE_PROXY + ? (App.videos.buildImageProxyUrl(target) || target) + : target; }; // A src-less counts as "unavailable", and the browser paints its alt @@ -203,29 +230,76 @@ App.videos = App.videos || {}; img.src = url; }; - // Last resort on a route that normally works: one expired or missing image - // shouldn't be left broken just because its host is fine in general. - const attachProxyFallback = function(img, proxyUrl, token) { - if (!proxyUrl) return; + // What a thumbnail that fails is given next, in order. + // + // A failure used to mean one more attempt -- the proxy -- and then an empty + // 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 - // fallback the image currently has, so a call arriving for a generation - // the element has moved past would take away the live one and leave a - // dead one -- the recycled card's thumbnail would then have no fallback - // at all if it failed. - if (token !== undefined && img.dataset.thumbToken !== token) return; + // the image is currently armed with, so a call arriving for a generation + // the element has moved past would take away the live plan and leave a + // dead one -- the recycled card's thumbnail would then have nothing + // behind it if it failed. + 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 // happy path it never fires and `once` never collects it, so a pooled // image would otherwise accumulate one closure per mount it has served. - detachProxyFallback(img); - const onError = () => { showThumbnail(img, proxyUrl, token); }; - img._thumbFallback = onError; + const onError = function() { + img._thumbRetry = null; + 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 }); }; - const detachProxyFallback = function(img) { - if (img && img._thumbFallback) { - img.removeEventListener('error', img._thumbFallback); - img._thumbFallback = null; + const detachRetry = function(img) { + if (!img) return; + if (img._thumbRetry) { + 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; imageWaiting.delete(host); waiting.forEach((entry) => { - if (route === IMAGE_PROXY) { - showThumbnail(entry.img, entry.proxyUrl, entry.token); - return; - } - if (route !== IMAGE_DIRECT) attachProxyFallback(entry.img, entry.proxyUrl, entry.token); - showThumbnail(entry.img, entry.directUrl, entry.token); + attachRetry(entry.img, retryPlan(entry.directUrl, entry.proxyUrl, route), entry.token); + showThumbnail(entry.img, + route === IMAGE_PROXY ? entry.proxyUrl : entry.directUrl, entry.token); }); }; @@ -303,12 +374,9 @@ App.videos = App.videos || {}; const giveUp = function() { if (shown) return; shown = true; - if (imageRoutes.get(host) === IMAGE_PROXY) { - showThumbnail(img, proxyUrl, token); - return; - } - attachProxyFallback(img, proxyUrl, token); - showThumbnail(img, directUrl, token); + const route = imageRoutes.get(host); + attachRetry(img, retryPlan(directUrl, proxyUrl, route), token); + showThumbnail(img, route === IMAGE_PROXY ? proxyUrl : directUrl, token); }; 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, // racing the two the first time that host is seen. 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; // 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 @@ -415,6 +485,7 @@ App.videos = App.videos || {}; img.dataset.thumbToken = token; if (route === IMAGE_PROXY) { + attachRetry(img, retryPlan(directUrl, proxyUrl, IMAGE_PROXY), token); showThumbnail(img, proxyUrl || directUrl, token); return; } @@ -429,8 +500,8 @@ App.videos = App.videos || {}; } if (route === IMAGE_DIRECT || !host || !proxyUrl) { // Known good, or nothing to race against: take the provider and keep - // the proxy as this image's own fallback. - attachProxyFallback(img, proxyUrl, token); + // the proxy behind it. + attachRetry(img, retryPlan(directUrl, proxyUrl, IMAGE_DIRECT), token); showThumbnail(img, directUrl, token); return; } @@ -438,13 +509,13 @@ App.videos = App.videos || {}; }; // 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 - // that fires later, finds a token that no longer matches and does nothing. + // generation moves on, so a race that settles later, or a retry that fires + // later, finds a token that no longer matches and does nothing. App.videos.detachThumbnail = function(img) { if (!img) return; img.dataset.thumbToken = String(++thumbSeq); delete img.dataset.alt; - detachProxyFallback(img); + detachRetry(img); }; // Each channel in a group sends back a different number of videos per diff --git a/tests/smoke_thumbnails.py b/tests/smoke_thumbnails.py new file mode 100644 index 0000000..a779519 --- /dev/null +++ b/tests/smoke_thumbnails.py @@ -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())