From fad301e7a4771b56e2c816e89fc97d5690369b8f Mon Sep 17 00:00:00 2001 From: npeter83 Date: Sat, 19 Sep 2026 06:55:40 +0200 Subject: [PATCH] fix(youtube): a refused YouTube action answers a 422 that says what to do MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The two manual syncs (subscriptions import, playlist pull) had no YouTubeError handler, so a quota, token or transport failure was a raw 500 and the blocking "Server error" dialog (E9 F1). The other YouTube routes answered either the upstream's English or a playlist-scoped "youtube_failed". One helper, routes/_common.youtube_refusal: 422 {code: "youtube", reason} with the reason sorted by what the user can do — quota (wait for the reset), reconnect (the token step refused), unavailable (retry). Used by every route that surfaces a YouTube failure; localizeDetail translates it. "Reconnect" had no path: a grant Google revoked still read as granted, so Settings and the wizard offered no connect button while every call failed. An invalid_grant from the token endpoint now clears the stored grant. --- backend/app/routes/_common.py | 10 ++ backend/app/routes/channels.py | 10 +- backend/app/routes/feed.py | 4 +- backend/app/routes/playlists.py | 26 +++-- backend/app/routes/sync.py | 12 +- backend/app/youtube/client.py | 41 ++++++- backend/tests/test_log_pii.py | 4 +- backend/tests/test_youtube_refusals.py | 106 ++++++++++++++++++ .../usePlaylistActions.dom.test.tsx | 2 +- frontend/src/i18n/locales/en/errors.json | 5 + frontend/src/i18n/locales/en/playlists.json | 1 - frontend/src/i18n/locales/hu/errors.json | 5 + frontend/src/i18n/locales/hu/playlists.json | 1 - frontend/src/lib/api/core.test.ts | 17 +++ frontend/src/lib/api/core.ts | 4 + 15 files changed, 221 insertions(+), 27 deletions(-) create mode 100644 backend/tests/test_youtube_refusals.py diff --git a/backend/app/routes/_common.py b/backend/app/routes/_common.py index 7223ce93..9154ae4c 100644 --- a/backend/app/routes/_common.py +++ b/backend/app/routes/_common.py @@ -13,6 +13,7 @@ from sqlalchemy.orm import Session from app.db import Base from app.models import User +from app.youtube.client import YouTubeError, refusal_reason M = TypeVar("M", bound=Base) @@ -51,3 +52,12 @@ def owned_or_404( if getattr(obj, owner_attr) != user.id: raise not_found(what) return obj + + +def youtube_refusal(exc: YouTubeError) -> HTTPException: + """The answer to an action YouTube refused: a 422 whose reason the UI translates + (`errors.youtube.`). One definition, because the routes had grown three shapes of it — + a bare English string quoting the upstream message, a playlist-scoped reason, and nothing at + all (a raw 500 from the manual syncs). The upstream failure is already logged where it + happened (`youtube.api.error`).""" + return HTTPException(status_code=422, detail={"code": "youtube", "reason": refusal_reason(exc)}) diff --git a/backend/app/routes/channels.py b/backend/app/routes/channels.py index 604bfaf2..10495618 100644 --- a/backend/app/routes/channels.py +++ b/backend/app/routes/channels.py @@ -25,7 +25,7 @@ from app.models import ( User, Video, ) -from app.routes._common import get_or_404, not_found, owned_or_404 +from app.routes._common import get_or_404, not_found, owned_or_404, youtube_refusal from app.sync.explore import explore_ingest_page from app.sync.runner import backfill_channel_now, run_recent_backfill from app.sync.subscriptions import apply_channel_details @@ -424,7 +424,7 @@ def explore_channel( channel = db.get(Channel, channel_id) result = explore_ingest_page(db, yt, channel, page_token) except YouTubeError as exc: - raise HTTPException(status_code=422, detail=f"YouTube couldn't load this channel: {exc}") + raise youtube_refusal(exc) return { "channel_id": channel_id, "ingested": result["ingested"], @@ -583,9 +583,7 @@ def subscribe( if "subscriptionDuplicate" in exc.reason.split(","): yt_sub_id = "" else: - raise HTTPException( - status_code=422, detail=f"YouTube couldn't subscribe you: {exc}" - ) + raise youtube_refusal(exc) # Enrich a stub channel (title/thumbnail/subscriber count) so it shows up properly right # away, on BOTH the normal and the desync path — a resurfaced stub still needs metadata to # render. Keep the client open (we're still inside the `with`); a metadata failure here @@ -649,7 +647,7 @@ def unsubscribe( except YouTubeError as exc: # 422 (a real, explainable error → speaking modal), not 502 which the client treats as # a transient "connection lost" gateway blip. - raise HTTPException(status_code=422, detail=f"YouTube couldn't unsubscribe you: {exc}") + raise youtube_refusal(exc) db.delete(sub) db.commit() return {"unsubscribed": channel_id} diff --git a/backend/app/routes/feed.py b/backend/app/routes/feed.py index 9c2dac8d..b38bbdb1 100644 --- a/backend/app/routes/feed.py +++ b/backend/app/routes/feed.py @@ -29,7 +29,7 @@ from app.models import ( Video, VideoState, ) -from app.routes._common import get_or_404 +from app.routes._common import get_or_404, youtube_refusal from app.sync.videos import parse_iso8601_duration from app.utils import iso from app.youtube.client import YouTubeClient, YouTubeError @@ -411,7 +411,7 @@ def get_video_detail( ): items = yt.get_videos([video_id]) except YouTubeError as exc: - raise HTTPException(status_code=422, detail=f"YouTube lookup failed: {exc}") + raise youtube_refusal(exc) if not items: raise HTTPException(status_code=404, detail="Unknown video") diff --git a/backend/app/routes/playlists.py b/backend/app/routes/playlists.py index c7617c50..718de2a9 100644 --- a/backend/app/routes/playlists.py +++ b/backend/app/routes/playlists.py @@ -23,7 +23,7 @@ from app.sync.playlists import ( sync_user_playlists, ) from app.ratelimit import RateLimiter -from app.routes._common import get_or_404, owned_or_404 +from app.routes._common import get_or_404, owned_or_404, youtube_refusal from app.youtube.client import YouTubeClient, YouTubeError router = APIRouter(prefix="/api/playlists", tags=["playlists"]) @@ -322,8 +322,12 @@ def sync_youtube( if not has_read_scope(user): raise _refused(403, "youtube_read_scope") _check_pull_cooldown(user) - with quota.attribute(user.id, quota.QuotaAction.PLAYLISTS_PULL): - return sync_user_playlists(db, user) + try: + with quota.attribute(user.id, quota.QuotaAction.PLAYLISTS_PULL): + return sync_user_playlists(db, user) + except YouTubeError as exc: + db.rollback() + raise youtube_refusal(exc) @router.post("/{playlist_id}/revert-youtube") @@ -344,9 +348,9 @@ def revert_youtube( try: with quota.attribute(user.id, quota.QuotaAction.PLAYLISTS_PULL): return repull_playlist(db, user, pl) - except YouTubeError: + except YouTubeError as exc: db.rollback() - raise _refused(422, "youtube_failed") + raise youtube_refusal(exc) def _pushable(db: Session, user: User, playlist_id: int) -> Playlist: @@ -371,8 +375,8 @@ def push_plan( try: with quota.attribute(user.id, quota.QuotaAction.PLAYLISTS_PUSH): plan = plan_merge(db, user, pl) - except YouTubeError: - raise _refused(422, "youtube_failed") + except YouTubeError as exc: + raise youtube_refusal(exc) plan["remaining_today"] = quota.remaining_today(db) plan["affordable"] = plan["units_estimate"] <= plan["remaining_today"] return plan @@ -404,9 +408,9 @@ def push( if plan["units_estimate"] > quota.remaining_today(db): raise _refused(429, "quota_exceeded") return merge_playlist(db, user, pl, live=live) - except YouTubeError: + except YouTubeError as exc: db.rollback() - raise _refused(422, "youtube_failed") + raise youtube_refusal(exc) @router.get("/{playlist_id}") @@ -496,8 +500,8 @@ def delete_playlist( with quota.attribute(user.id, quota.QuotaAction.PLAYLISTS_DELETE): with YouTubeClient(db, user) as yt: yt.delete_playlist(pl.yt_playlist_id) - except YouTubeError: - raise _refused(422, "youtube_failed") + except YouTubeError as exc: + raise youtube_refusal(exc) db.delete(pl) # items cascade db.commit() return {"deleted": playlist_id} diff --git a/backend/app/routes/sync.py b/backend/app/routes/sync.py index 8b1e7462..a7a15d1e 100644 --- a/backend/app/routes/sync.py +++ b/backend/app/routes/sync.py @@ -7,10 +7,12 @@ from app.audit import AuditAction from app.auth import admin_user, current_user, has_read_scope, require_human from app.db import get_db from app.models import Channel, Subscription, User, Video +from app.routes._common import youtube_refusal from app.ratelimit import RateLimiter from app.sync.runner import estimate_deep_backfill from app.sync.subscriptions import import_subscriptions from app.scheduler import running_job_ids +from app.youtube.client import YouTubeError router = APIRouter(prefix="/api/sync", tags=["sync"]) @@ -32,8 +34,14 @@ def sync_subscriptions(user: User = Depends(require_human), db: Session = Depend ) if not _sync_limiter.allow(str(user.id)): raise HTTPException(status_code=429, detail={"code": "sync_cooldown"}) - with quota.measured(db, user.id, quota.QuotaAction.SUBSCRIPTIONS_PULL) as m: - result = import_subscriptions(db, user) + try: + with quota.measured(db, user.id, quota.QuotaAction.SUBSCRIPTIONS_PULL) as m: + result = import_subscriptions(db, user) + except YouTubeError as exc: + # The onboarding wizard's first call: an unguarded failure here was a raw 500 and the + # blocking "Server error" dialog, with nothing the user could act on (E9 F1). + db.rollback() + raise youtube_refusal(exc) result["quota_used_estimate"] = m.used result["quota_remaining_today"] = m.remaining return result diff --git a/backend/app/youtube/client.py b/backend/app/youtube/client.py index 381da7a9..0fa9679b 100644 --- a/backend/app/youtube/client.py +++ b/backend/app/youtube/client.py @@ -9,6 +9,7 @@ uses OAuth. import logging from collections.abc import Iterator from datetime import datetime, timedelta, timezone +from typing import Literal import httpx @@ -108,6 +109,22 @@ def is_quota_refusal(exc: YouTubeError) -> bool: return any(r in _QUOTA_REASONS for r in exc.reason.split(",")) +# What a refused YouTube action means to the person who asked for it — the three things they can +# act on differently: wait for tomorrow's budget, reconnect the account, or try again later. +YouTubeRefusal = Literal["quota", "reconnect", "unavailable"] + + +def refusal_reason(exc: YouTubeError) -> YouTubeRefusal: + """Sort a failure by what the user can do about it. The token step failing (Google refused the + refresh, or there is no usable token at all — raised before any endpoint answered, so with no + path and no transport reason) means the account must be reconnected; retrying cannot help.""" + if is_quota_refusal(exc): + return "quota" + if exc.path == "oauth2/token" or (not exc.path and exc.reason != "transport"): + return "reconnect" + return "unavailable" + + def _chunks(items: list, size: int) -> Iterator[list]: for i in range(0, len(items), size): yield items[i : i + size] @@ -181,7 +198,10 @@ class YouTubeClient: }, ) if resp.status_code != 200: - raise _upstream_failure("POST", "oauth2/token", resp) + exc = _upstream_failure("POST", "oauth2/token", resp) + if exc.reason == "invalid_grant": + self._drop_dead_grant(tok) + raise exc data = resp.json() access = data["access_token"] tok.access_token = encrypt(access) @@ -191,6 +211,25 @@ class YouTubeClient: log.info("Refreshed access token for user %s", self.user.id) return access + def _drop_dead_grant(self, tok) -> None: + """Google answered the refresh with `invalid_grant`: the grant was revoked or expired, and no + retry will revive it. Forget it, so the account reads as NOT connected — Settings and the + onboarding wizard then offer the connect button again (a fresh consent issues a new refresh + token). Kept as "granted", the UI showed a live connection with no way to renew it while + every call failed. Committed like the refresh above: the caller rolls back on the error.""" + tok.scopes = None + tok.access_token = None + tok.refresh_token_enc = None + tok.expiry = None + self.db.add(tok) + self.db.commit() + # INFO: the refusal itself is already the warning (`youtube.api.error`, invalid_grant). + log.info( + "cleared the dead YouTube grant of user %s", + self.user.id, + extra=logs.event("youtube.grant.cleared"), + ) + # --- core request --- def _get( self, diff --git a/backend/tests/test_log_pii.py b/backend/tests/test_log_pii.py index 92a52d73..fe523eb9 100644 --- a/backend/tests/test_log_pii.py +++ b/backend/tests/test_log_pii.py @@ -140,9 +140,9 @@ def test_a_refused_token_refresh_names_the_oauth_reason_and_not_its_prose(sink, request=httpx.Request("POST", "https://oauth2.googleapis.com/token"), ) client = yt.YouTubeClient.__new__(yt.YouTubeClient) # no HTTP client: `_send` is the stub - client.db = None + client.db = SimpleNamespace(add=lambda obj: None, commit=lambda: None) client.user = SimpleNamespace( - token=SimpleNamespace(access_token=None, expiry=None, refresh_token_enc=b"x") + id=1, token=SimpleNamespace(access_token=None, expiry=None, refresh_token_enc=b"x") ) monkeypatch.setattr(yt, "decrypt_optional", lambda value: None) monkeypatch.setattr(yt, "decrypt", lambda value: "a-refresh-token") diff --git a/backend/tests/test_youtube_refusals.py b/backend/tests/test_youtube_refusals.py new file mode 100644 index 00000000..724ef8e3 --- /dev/null +++ b/backend/tests/test_youtube_refusals.py @@ -0,0 +1,106 @@ +"""An action YouTube refuses answers a 422 the UI can translate — never a raw 500 — and says what the +user can do about it (`routes/_common.youtube_refusal`). E9 F1: the two manual syncs had no handler +at all, so a quota or transport failure opened the blocking "Server error" dialog.""" + +from types import SimpleNamespace + +import httpx +import pytest + +from app.auth import has_read_scope +from app.models import OAuthToken +from app.youtube import client as yt +from app.youtube.client import YouTubeError, refusal_reason + + +@pytest.mark.parametrize( + ("exc", "reason"), + [ + (YouTubeError("m", reason="quotaExceeded", path="playlists"), "quota"), + (YouTubeError("m", reason="backendError,dailyLimitExceeded", path="videos"), "quota"), + (YouTubeError("m", reason="invalid_grant", path="oauth2/token"), "reconnect"), + (YouTubeError("No refresh token; user must re-authenticate"), "reconnect"), + (YouTubeError("m", reason="transport"), "unavailable"), + (YouTubeError("m", reason="backendError", path="subscriptions"), "unavailable"), + ], +) +def test_a_failure_is_sorted_by_what_the_user_can_do(exc, reason): + assert refusal_reason(exc) == reason + + +@pytest.mark.parametrize( + ("route", "target"), + [ + ("/api/sync/subscriptions", "app.routes.sync.import_subscriptions"), + ("/api/playlists/sync-youtube", "app.routes.playlists.sync_user_playlists"), + ], +) +def test_a_manual_sync_youtube_refuses_is_a_422_not_a_500( + db, as_user, make_user, monkeypatch, route, target +): + user = make_user() + db.commit() + module, name = target.rsplit(".", 1) + + def refused(*a, **k): + raise YouTubeError("GET subscriptions -> 403: quotaExceeded", reason="quotaExceeded") + + monkeypatch.setattr(f"{module}.has_read_scope", lambda u: True) + monkeypatch.setattr(f"{module}.{name}", refused) + + r = as_user(user).post(route) + assert r.status_code == 422 + assert r.json()["detail"] == {"code": "youtube", "reason": "quota"} + + +def test_a_revoked_grant_is_forgotten_so_the_account_can_reconnect(db, make_user, monkeypatch): + """Google's `invalid_grant` is final. Kept, the grant read as live in Settings — with no connect + button — while every call failed; cleared, the connect button comes back.""" + user = make_user() + db.add( + OAuthToken( + user_id=user.id, + refresh_token_enc="x", + scopes="openid https://www.googleapis.com/auth/youtube.readonly", + ) + ) + db.commit() + db.refresh(user) + assert has_read_scope(user) + + resp = httpx.Response( + 400, + json={"error": "invalid_grant", "error_description": "Token has been expired or revoked."}, + request=httpx.Request("POST", yt.GOOGLE_TOKEN_URL), + ) + c = yt.YouTubeClient.__new__(yt.YouTubeClient) # no HTTP client: `_send` is the stub + c.db, c.user = db, user + monkeypatch.setattr(yt, "decrypt", lambda value: "a-refresh-token") + monkeypatch.setattr(yt.sysconfig, "get_str", lambda d, key: "") + monkeypatch.setattr(c, "_send", lambda *a, **k: resp) + + with pytest.raises(YouTubeError) as caught: + c._access_token() + assert refusal_reason(caught.value) == "reconnect" + + db.expire_all() + assert not has_read_scope(db.get(type(user), user.id)) + + +def test_any_other_token_refusal_keeps_the_grant(monkeypatch): + """Only `invalid_grant` is final: a 5xx from the token endpoint is Google having a bad minute.""" + resp = httpx.Response( + 503, json={"error": "backend_error"}, request=httpx.Request("POST", yt.GOOGLE_TOKEN_URL) + ) + tok = SimpleNamespace(access_token=None, expiry=None, refresh_token_enc="x", scopes="s") + c = yt.YouTubeClient.__new__(yt.YouTubeClient) + c.db = SimpleNamespace(add=lambda o: None, commit=lambda: None) + c.user = SimpleNamespace(id=1, token=tok) + monkeypatch.setattr(yt, "decrypt_optional", lambda value: None) + monkeypatch.setattr(yt, "decrypt", lambda value: "a-refresh-token") + monkeypatch.setattr(yt.sysconfig, "get_str", lambda d, key: "") + monkeypatch.setattr(c, "_send", lambda *a, **k: resp) + + with pytest.raises(YouTubeError): + c._access_token() + assert tok.scopes == "s" diff --git a/frontend/src/components/usePlaylistActions.dom.test.tsx b/frontend/src/components/usePlaylistActions.dom.test.tsx index 4b9569d6..7f8b5a50 100644 --- a/frontend/src/components/usePlaylistActions.dom.test.tsx +++ b/frontend/src/components/usePlaylistActions.dom.test.tsx @@ -49,7 +49,7 @@ const linked = { id: 4, name: "Mix", kind: "user", yt_playlist_id: "PL1" } as co describe("usePlaylistActions — deleting a list", () => { it("does not toast a failure the request layer already put on screen", async () => { vi.spyOn(globalThis, "fetch").mockResolvedValue( - new Response(JSON.stringify({ detail: { code: "playlist", reason: "youtube_failed" } }), { + new Response(JSON.stringify({ detail: { code: "youtube", reason: "unavailable" } }), { status: 422, headers: { "content-type": "application/json" }, }), diff --git a/frontend/src/i18n/locales/en/errors.json b/frontend/src/i18n/locales/en/errors.json index bf036a5a..6d4287f2 100644 --- a/frontend/src/i18n/locales/en/errors.json +++ b/frontend/src/i18n/locales/en/errors.json @@ -91,5 +91,10 @@ "reload": "Reload", "notifTitle": "Something broke", "notifMessage": "Unexpected error" + }, + "youtube": { + "quota": "Today's YouTube budget for this instance is spent. It resets at midnight Pacific time — try again then.", + "reconnect": "Google no longer accepts this account's YouTube connection. Sign out and sign in with Google again to reconnect it.", + "unavailable": "YouTube didn't accept that just now. Please try again." } } diff --git a/frontend/src/i18n/locales/en/playlists.json b/frontend/src/i18n/locales/en/playlists.json index 9a71c432..f1a6dc46 100644 --- a/frontend/src/i18n/locales/en/playlists.json +++ b/frontend/src/i18n/locales/en/playlists.json @@ -81,7 +81,6 @@ "name_required": "Give the playlist a name.", "youtube_read_scope": "Connect YouTube (read access) to sync your playlists.", "not_on_youtube": "This playlist isn't on YouTube.", - "youtube_failed": "YouTube didn't accept that just now. Please try again.", "watch_later_not_pushable": "The Watch later list can't be sent to YouTube.", "watch_later_undeletable": "The Watch later list can't be deleted.", "youtube_editing_off": "Enable YouTube editing in Settings first.", diff --git a/frontend/src/i18n/locales/hu/errors.json b/frontend/src/i18n/locales/hu/errors.json index b9c15734..13be5034 100644 --- a/frontend/src/i18n/locales/hu/errors.json +++ b/frontend/src/i18n/locales/hu/errors.json @@ -91,5 +91,10 @@ "reload": "Újratöltés", "notifTitle": "Valami elromlott", "notifMessage": "Váratlan hiba" + }, + "youtube": { + "quota": "Ennek a példánynak elfogyott a mai YouTube-kerete. Csendes-óceáni idő szerint éjfélkor újraindul — akkor próbáld újra.", + "reconnect": "A Google már nem fogadja el ennek a fióknak a YouTube-kapcsolatát. Jelentkezz ki, majd lépj be újra Google-lel az összekötéshez.", + "unavailable": "A YouTube most nem fogadta el. Próbáld újra." } } diff --git a/frontend/src/i18n/locales/hu/playlists.json b/frontend/src/i18n/locales/hu/playlists.json index 95d0f529..8bd0725a 100644 --- a/frontend/src/i18n/locales/hu/playlists.json +++ b/frontend/src/i18n/locales/hu/playlists.json @@ -81,7 +81,6 @@ "name_required": "Adj nevet a lejátszási listának.", "youtube_read_scope": "A listák szinkronizálásához kapcsold össze a YouTube-ot (olvasási hozzáférés).", "not_on_youtube": "Ez a lejátszási lista nincs fent a YouTube-on.", - "youtube_failed": "A YouTube most nem fogadta el. Próbáld újra.", "watch_later_not_pushable": "A „Megnézem később” listát nem lehet a YouTube-ra küldeni.", "watch_later_undeletable": "A „Megnézem később” lista nem törölhető.", "youtube_editing_off": "Előbb engedélyezd a YouTube-szerkesztést a Beállításokban.", diff --git a/frontend/src/lib/api/core.test.ts b/frontend/src/lib/api/core.test.ts index 29239630..57d591f6 100644 --- a/frontend/src/lib/api/core.test.ts +++ b/frontend/src/lib/api/core.test.ts @@ -167,6 +167,23 @@ describe("req — a structured refusal reads in the user's language", () => { detail: localized, }); }); + + it.each(["quota", "reconnect", "unavailable"])( + "says what to do about an action YouTube refused (%s)", + async (reason) => { + // Waiting for tomorrow's budget, reconnecting and retrying are different advice; the manual + // syncs used to answer a raw 500 here, and the other routes the upstream's English. + vi.spyOn(globalThis, "fetch").mockResolvedValue( + new Response(JSON.stringify({ detail: { code: "youtube", reason } }), { + status: 422, + headers: { "content-type": "application/json" }, + }), + ); + await expect(req("/api/sync/subscriptions", { method: "POST" })).rejects.toMatchObject({ + detail: `errors.youtube.${reason}`, + }); + }, + ); }); describe("req — what it has already put in front of the user", () => { diff --git a/frontend/src/lib/api/core.ts b/frontend/src/lib/api/core.ts index 3593f182..cacbf517 100644 --- a/frontend/src/lib/api/core.ts +++ b/frontend/src/lib/api/core.ts @@ -129,6 +129,10 @@ function localizeDetail(d: unknown): string { return o.reason === "name_taken" ? t("plex.collection.nameTaken") : t("plex.collection.editFailed"); + // An action YouTube refused (routes/_common.youtube_refusal): what the reader can do about it — + // wait for the budget, reconnect, or retry. An unknown reason reads as the retryable one. + if (o?.code === "youtube") + return t(`errors.youtube.${o.reason}`, t("errors.youtube.unavailable")); // A playlist action the server refused (Watch later can't be deleted, YouTube said no, …). if (o?.code === "playlist") return t(`playlists.errors.${o.reason}`, t("errors.generic")); // An admin action on an account or an invite that the server refused: activating an address that