diff --git a/MyMistakes.md b/MyMistakes.md index 3c32435..ce1ad03 100644 --- a/MyMistakes.md +++ b/MyMistakes.md @@ -1399,3 +1399,42 @@ Corollary, and the part I keep skipping: **a repeated user correction is a docum not a user annoyance.** The second time the same question gets asked, the correct response is not a better answer — it is to go find why the written answer did not land, and fix *that*. Answering well the first time was never the problem; the map was not carrying the instruction. + +--- + +## Error messages: write for the maintenance dev, not for the toast (creator ruling 2026-10-04) + +The creator's standard, verbatim: *"my advice for error messages is to give some thought to the +poor maintenance dev who's got to slog through this code tracking down bugs... generic error +messages or, worse, error consolidation would ruin their day completely."* Extended on the ANSI +point: *"didn't matter how cool it looked if you didn't understand what it was trying to tell you."* + +**The rule: legibility outranks polish, and a cause must never be collapsed into a bucket.** + +1. **Never consolidate distinct causes.** The first bind-failure implementation mapped seven + documented reasons to seven messages and then funnelled everything unmapped into + `_ => $"YouTube rejected the stream binding ({status})."` — which is the trap wearing a + friendly face. YouTube adds an eighth reason, the generic branch swallows it, and the dev + greps for a string the API never emits. **An unmapped reason is passed through verbatim so it + stays greppable against the raw response.** A friendly sentence may be *added* to an unknown + cause; it may never *replace* it. +2. **Never let the toast be the only record.** The user-facing string may be reworded; the log + must keep status + reason + the whole response body verbatim. Toast = friendly, log = exact. +3. **"Re-authenticate" is not a universal answer.** It was my first instinct for a bind failure and + it is correct for exactly one of six documented reasons. Wrong for the other five it actively + misdirects — worst for `liveBroadcastBindingNotAllowed`, which means *our own ordering* is + broken. Sending the dev to re-authenticate hides a bug in our own code behind a plausible + excuse. **Before writing a fix-it suggestion, ask which of the documented reasons this text + would be wrong for.** +4. **A sentinel that means three things is not a sentinel.** The diagnostic emitted `?` for + "field absent", "field unparseable" and "endpoint returned something else" — an ANSI-bright + `?` that looked informative and told the reader nothing. Distinct states get distinct tokens + (`unset` vs `unread` vs a real value), and unreadable input is never reported as a finding. +5. **Check the docs' failure modes, not just the happy path** (this one already bit a week of + diagnosis — see the SOLVED box in `ai.md`). Every reason the API can refuse must have its own + handling and its own test, or the refusal is invisible. + +**Corollary, same shape as the decision-note lesson above:** the test suite is part of this +standard. A test that asserts a *disproven* behaviour doesn't just fail to help — it launders a +guess into a guarantee and makes the next reader stop looking. If a test's expectation is wrong, +the expectation is the bug; fix the belief, don't delete the assertion. diff --git a/Services/YouTubeStreamService.cs b/Services/YouTubeStreamService.cs index af7ac6a..7726826 100644 --- a/Services/YouTubeStreamService.cs +++ b/Services/YouTubeStreamService.cs @@ -29,7 +29,12 @@ public class YouTubeStreamService return true; } - public async Task CreateBroadcast(string title, string description, DateTime scheduledStartTime, string? streamId = null) + /// — note there is deliberately NO streamId + /// parameter (2026-10-04). It used to be here and was ignored: sending + /// `contentDetails.boundStreamId` does not bind the broadcast. A parameter that looks + /// meaningful but cannot affect the outcome is a defect — it is exactly what made the + /// week-long misdiagnosis possible — so the signature now says what it actually does. + public async Task CreateBroadcast(string title, string description, DateTime scheduledStartTime) { if (!await EnsureToken()) return null; @@ -50,9 +55,16 @@ public class YouTubeStreamService selfDeclaredMadeForKids = false }, // One-click go-live (TASK 5 design decision 1): auto start/stop with - // no monitor stream and low latency. A reusable stream, when given, - // binds here (boundStreamId) so no second bind round-trip is needed. - contentDetails = BuildContentDetails(streamId) + // no monitor stream and low latency. + // + // ⛔ `boundStreamId` is DELIBERATELY NOT SENT HERE (2026-10-04). It used to + // be, on the assumption that binding at insert saved a round-trip. It does + // not bind anything — proven: the insert was accepted, sibling fields in + // this same object took effect, and the broadcast still read `bound=NONE`, + // stayed in lifeCycleStatus `created`, and never went live. Binding is done + // by the explicit BindBroadcastAsync call instead. Leaving the field out + // stops the next reader trusting it; see that method for the full account. + contentDetails = BuildContentDetails() }; _http.DefaultRequestHeaders.Authorization = new("Bearer", _auth.CurrentChannel!.AccessToken); @@ -66,17 +78,17 @@ public class YouTubeStreamService return data.GetProperty("id").GetString(); } - private static Dictionary BuildContentDetails(string? streamId) + private static Dictionary BuildContentDetails() { - var details = new Dictionary + // No `boundStreamId` here on purpose — it does not bind (proven 2026-10-04). + // Binding is the explicit BindBroadcastAsync call; see CreateBroadcast. + return new Dictionary { ["enableAutoStart"] = true, ["enableAutoStop"] = true, ["enableMonitorStream"] = false, ["latencyPreference"] = "low", }; - if (streamId != null) details["boundStreamId"] = streamId; - return details; } /// @@ -288,6 +300,100 @@ public class YouTubeStreamService ? (value.ValueKind == JsonValueKind.True ? "true" : value.ValueKind == JsonValueKind.False ? "false" : value.ValueKind.ToString()) : "unset"; + /// Binds the broadcast to its stream — THE call that actually makes a broadcast + /// go live, and the reason it took a week to find (2026-10-04). + /// + /// `POST liveBroadcasts/bind?id=…&streamId=…&part=id`, no request body, `youtube.force-ssl`. + /// + /// **Sending `contentDetails.boundStreamId` on `liveBroadcasts.insert` does NOT bind the + /// broadcast.** Proven, not assumed: the insert carried `boundStreamId`, the API accepted the + /// request (sibling fields in the same `contentDetails` — `enableAutoStart`, `enableAutoStop` — + /// demonstrably took effect), and the created broadcast still came back + /// `bound=NONE`. YouTube's own migration guide says so outright: *"a liveBroadcast must be bound + /// to exactly one stream before the live broadcast can actually start. The broadcast is not bound + /// to any streams at the time that it's created."* + /// + /// The failure it produces is a **silent stall, not an error**, which is why it survived so long: + /// an unbound broadcast reports `lifeCycleStatus = created` ("incomplete settings, not ready to + /// transition to a live or testing status"), so `enableAutoStart` has no bound stream to fire + /// from, the broadcast never leaves `created`, and close-out then 403s with `invalidTransition` + /// because `created → complete` is not a legal edge. Meanwhile ffmpeg pushes ~99 MB happily into + /// the reusable stream, because a reusable stream accepts bytes for a broadcast that is not + /// watching. Every symptom pointed away from the binding. + /// + /// Returns null on success, otherwise a human-readable error naming the ACTUAL cause. + /// Never throws. + public async Task BindBroadcastAsync(string broadcastId, string streamId) + { + if (!await EnsureToken()) return "not signed in"; + + _http.DefaultRequestHeaders.Authorization = new("Bearer", _auth.CurrentChannel!.AccessToken); + + var response = await _http.PostAsync( + $"{ApiBase}/liveBroadcasts/bind?part=id&id={Uri.EscapeDataString(broadcastId)}&streamId={Uri.EscapeDataString(streamId)}", + content: null); + + if (response.IsSuccessStatusCode) + { + AppLog.Write($"Broadcast bound: {broadcastId} -> stream {streamId}"); + return null; + } + + var body = await response.Content.ReadAsStringAsync(); + var reason = ErrorReason(body); + AppLog.Write($"Broadcast bind failed ({(int)response.StatusCode}/{reason ?? "unknown"}) for {broadcastId} -> {streamId}: {body}"); + return BindFailureMessage((int)response.StatusCode, reason); + } + + /// Pulls `errors[0].reason` out of a Google API error body. Needed because + /// "rejected the binding" alone is not actionable — the reason is what tells the + /// creator whether to re-auth, wait, or suspect our own ordering. + internal static string? ErrorReason(string body) + { + try + { + using var doc = JsonDocument.Parse(body); + if (doc.RootElement.TryGetProperty("error", out var error) + && error.TryGetProperty("errors", out var errors) + && errors.GetArrayLength() > 0 + && errors[0].TryGetProperty("reason", out var reason)) + return reason.GetString(); + } + catch (JsonException) + { + // A non-JSON error body tells us nothing actionable; the status code is enough. + } + return null; + } + + /// Bind failures are NOT all "sign in again" (2026-10-04). Google documents six + /// distinct reasons and only one is an auth problem, so a single sign-in message + /// actively misdirects: `liveBroadcastBindingNotAllowed` in particular means OUR + /// ordering is wrong, and telling the creator to re-authenticate would hide a bug in + /// our own code behind a plausible excuse. Each documented reason gets its own text, + /// because the caller's only lever is the string it shows. + internal static string BindFailureMessage(int status, string? reason) => reason switch + { + "liveBroadcastBindingNotAllowed" => + "YouTube won't bind a broadcast in its current state — it may already be live. End the session and start again.", + "liveStreamNotFound" => + "YouTube couldn't find the reusable stream. Sign in again so it can be recreated.", + "liveBroadcastNotFound" => + "YouTube couldn't find the broadcast that was just created. Try again.", + "liveStreamingNotEnabled" => + "This channel isn't enabled for live streaming — check the channel's live settings in YouTube Studio.", + "insufficientLivePermissions" => + "This account doesn't have permission to stream live. Check the YouTube sign-in.", + "internalError" => + "YouTube hit an internal error binding the stream. Try again.", + "userRequestsExceededRateLimit" => + "YouTube is rate-limiting us. Wait a moment and try again.", + // Unknown or absent reason: keep it verbatim. Never collapse distinct causes into + // one bucket — an unmapped reason must stay greppable against the raw API response. + null => $"YouTube rejected the stream binding (HTTP {status}, no reason in the body — see startup.log).", + _ => $"YouTube rejected the stream binding (HTTP {status}, reason '{reason}' — see startup.log).", + }; + /// Returns the channel's reusable stream (TASK 5 design decision 2): /// lists existing streams first and reuses the one with cdn.isReusable=true, /// creating it with variable resolution/frame rate on first use. Binding to a diff --git a/ViewModels/MainViewModel.Streaming.Operations.cs b/ViewModels/MainViewModel.Streaming.Operations.cs index e47253e..b32a369 100644 --- a/ViewModels/MainViewModel.Streaming.Operations.cs +++ b/ViewModels/MainViewModel.Streaming.Operations.cs @@ -166,7 +166,7 @@ public partial class MainViewModel : ViewModelBase _layoutStore.SaveReusableStream(stream); _currentBroadcastId = await _youtubeStream.CreateBroadcast( - StreamTitle, StreamDescription, DateTime.UtcNow, stream.Id); + StreamTitle, StreamDescription, DateTime.UtcNow); _broadcastSeenLive = false; if (_currentBroadcastId == null) { @@ -178,6 +178,31 @@ public partial class MainViewModel : ViewModelBase AppLog.Write($"Broadcast created: {_currentBroadcastId}"); OnPropertyChanged(nameof(CurrentBroadcastId)); + // ⛔ THE BINDING — the call whose absence is why nothing ever went live + // (2026-10-04). Sending `contentDetails.boundStreamId` on the insert does NOT + // bind: the insert is accepted, its sibling fields take effect, and the + // broadcast still comes back `bound=NONE`, sits in lifeCycleStatus `created` + // forever ("incomplete settings"), so `enableAutoStart` has no bound stream to + // fire from and close-out 403s `invalidTransition`. Meanwhile ffmpeg happily + // pushes ~99MB into the reusable stream, because a reusable stream accepts + // bytes for a broadcast that is not watching — which is why every symptom + // pointed away from the binding. + // + // Failure here is fatal to the session, not cosmetic: an unbound broadcast can + // never go live, so continuing would present a live-looking UI over a broadcast + // that is quietly never going to be seen. + var bindError = await _youtubeStream.BindBroadcastAsync(_currentBroadcastId, stream.Id); + if (bindError != null) + { + AppLog.Write($"Stream binding failed: {bindError}"); + // Show the real reason. "Check the sign-in" would be wrong for five of the + // six documented bind failures, and would hide our own ordering bug behind + // a plausible excuse — see BindFailureMessage. + _notifications.Error("Couldn't go live", bindError); + StopStream(); // full rollback + return; + } + if (string.IsNullOrWhiteSpace(StreamTitle) && string.IsNullOrWhiteSpace(StreamDescription)) { BroadcastForm.IsDrawerOpen = true; diff --git a/ai.md b/ai.md index 57e561d..a0606c9 100644 --- a/ai.md +++ b/ai.md @@ -1677,6 +1677,57 @@ YouTube" button; going live is unreachable until an account is connected. ## YouTube Live API — design constraints (do not violate) +> ## ✅ SOLVED 2026-10-04 — the broadcasts never went live because they were never BOUND. +> +> **The bug, in one line: `contentDetails.boundStreamId` on `liveBroadcasts.insert` does not bind +> the broadcast.** Binding requires an explicit `POST liveBroadcasts/bind?id=…&streamId=…&part=id`. +> +> **How it was finally proven, after a week of misdiagnosis** — the diagnostic line, once its own +> JSON-nesting bug was fixed (`FirstItem`), read: +> ``` +> lifeCycle=created privacy=private autoStart=true bound=NONE boundMatches=NO-BINDING +> | streamStatus=active activeChat=none cdn=rtmp/variable@variable +> ``` +> Every field is real, and together they are conclusive: +> - `bound=NONE` **with** `autoStart=true`. Both live in the *same* `contentDetails` object on the +> insert, so the request was well-formed and accepted — the API took one field and silently ignored +> the other. That asymmetry is the proof; a malformed request would have failed outright. +> - `lifeCycle=created`, **not** `ready`. Google: `created` = *"incomplete settings, so it is not +> ready to transition to a live or testing status"*. An unbound broadcast is precisely incomplete. +> - `streamStatus=active` — the ingest half was never broken. YouTube had an active stream and a +> broadcast that was not attached to it. +> +> **The causal chain, complete:** no binding → `lifeCycleStatus` stuck at `created` → +> `enableAutoStart` has no bound stream to fire from → never live → close-out 403s +> `invalidTransition` because `created → complete` is not a legal edge (only `ready`/`live` are). +> +> **Why it survived a week, and the lesson worth keeping:** every observable pointed away from the +> binding. ffmpeg pushed ~99 MB cleanly, the reusable stream went `active`, close-out returned a +> *transition* error (which reads like a lifecycle-timing problem), and the insert returned a valid +> broadcast ID (which reads like success). **A reusable stream accepts bytes for a broadcast that is +> not watching it**, so a perfectly healthy RTMP session is exactly what an unbound broadcast looks +> like. Google says it outright in the migration guide: *"a liveBroadcast must be bound to exactly one +> stream before the live broadcast can actually start. The broadcast is not bound to any streams at +> the time that it's created."* That sentence was in the docs I read during the original design and +> was not applied. **Read the failure modes in the docs, not just the happy path.** +> +> **Shipped:** `YouTubeStreamService.BindBroadcastAsync`, called from `PrepareAndStartLiveAsync` +> immediately after `CreateBroadcast` and **before** the pump starts. Failure is fatal to the session +> (an unbound broadcast can never go live, so a live-looking UI over it is a lie). `boundStreamId` was +> **removed** from the insert body and the `streamId` parameter **removed from `CreateBroadcast`** — a +> parameter that looks meaningful but cannot affect the outcome is what made the misdiagnosis possible. +> The old test `CreateBroadcast_With_StreamId_Binds_Reusable_Stream_At_Insert`, which asserted the +> disproven behaviour and therefore *laundered a guess into a guarantee*, is replaced by +> `CreateBroadcast_Does_Not_Claim_To_Bind_And_Omits_BoundStreamId` plus +> `BindBroadcastAsync_Posts_To_Bind_Endpoint_With_Both_Ids_And_No_Body`. +> +> **Bind failures are not all "sign in again."** Google documents six reasons +> (`insufficientLivePermissions`, `liveStreamingNotEnabled`, `liveBroadcastBindingNotAllowed`, +> `liveStreamNotFound`, `liveBroadcastNotFound`, `internalError`, plus rate limiting) and only the +> first is an auth problem. `BindFailureMessage` maps each to its own text; `liveBroadcastBindingNotAllowed` +> matters most, because it means *our* ordering is wrong and a generic "check the sign-in" would hide +> a bug in our own code behind a plausible excuse. + These are the hard facts behind every decision. Full list in `TASKS.md`. - **One-click go-live** — never call `transition(live)`. Insert the broadcast with @@ -1845,15 +1896,20 @@ These are the hard facts behind every decision. Full list in `TASKS.md`. any left-click outside the rail closes whichever drawer is open — TEST included (fixed 2026-09-25, TASK 46; the code-behind outside-click close in `MainWindow` had skipped TestSession, so only Stream Settings + YPP dismissed on an outside click). - **liveChatId resolution (fixed 2026-09-25, TASK 44):** the id is `snippet.liveChatId` — - `contentDetails` has NO liveChatId property — and YouTube only populates it once the broadcast - is LIVE (the official GetLiveChatId.java sample lists broadcastStatus=active). So - `GetBroadcastLiveChatIdAsync(broadcastId, maxAttempts=10, delayMs=2000)` reads - part=snippet and polls with a bounded retry, and `PrepareAndStartLiveAsync` starts the frame - pump FIRST (the RTMP push is what flips ready→live via enableAutoStart) before resolving it. - Chrome-debugging note: a fetch at broadcast-insert time (lifecycleStatus ready) legitimately - returns no id — missing liveChatId is the ready state, not an auth failure, until ~20s post - push. + **liveChatId resolution (fixed 2026-09-25, TASK 44; ⚠️ CORRECTED 2026-10-04):** the id is + `snippet.liveChatId` — `contentDetails` has NO liveChatId property — and + `GetBroadcastLiveChatIdAsync(broadcastId, maxAttempts=10, delayMs=2000)` reads part=snippet and + polls with a bounded retry, and `PrepareAndStartLiveAsync` starts the frame pump FIRST before + resolving it. **The old claim that "YouTube only populates it once the broadcast is LIVE" is + WRONG, and it was inferred from a sample rather than from the API.** The 2026-10-04 diagnostic + caught `liveChatId=present` while `lifeCycleStatus=created` — not live, not even `ready`. The + official `GetLiveChatId.java` sample tests `broadcastStatus == active` before returning the id, and + that read as a statement about *when YouTube populates the field*; it is only a guard in the + sample. **Generalisation: a sample's defensive check is not a spec.** Also note the old note said + the missing id meant "the ready state" — the broadcasts were never `ready` at all, they were + `created` and unbound (see the SOLVED box above), so the terminology was wrong too. + The bounded retry is harmless and stays, but the id is available far earlier than assumed — worth + revisiting only if chat startup latency is ever reported as slow. Drawer tooling: **Mock Chat Input** = a REAL `liveChat/messages.insert` (`YouTubeStreamService.InsertChatMessageAsync`) that round-trips through the real ~2s poll and renders through the live overlay path; **simulated Subscriber / New Member / Super Chat** = `YouTubeChatService.InjectSimulatedMessage`, diff --git a/ytLive.Tests/YouTubeStreamServiceTests.cs b/ytLive.Tests/YouTubeStreamServiceTests.cs index 08afae2..f65baf2 100644 --- a/ytLive.Tests/YouTubeStreamServiceTests.cs +++ b/ytLive.Tests/YouTubeStreamServiceTests.cs @@ -79,25 +79,60 @@ public class YouTubeStreamServiceTests Assert.Null(handler.LastBody); // never reached the API } - // TASK 5: a reusable stream, when given, binds at broadcast insert via - // contentDetails.boundStreamId — no separate bind round-trip. One-click - // go-live also completes the v3 flags (no monitor stream, low latency). + // TASK 5: one-click go-live completes the v3 flags (no monitor stream, low latency). + // + // ⛔ REWRITTEN 2026-10-04. This used to be + // `CreateBroadcast_With_StreamId_Binds_Reusable_Stream_At_Insert`, and it asserted + // `"boundStreamId":"S456"` in the insert body — i.e. it encoded, as a passing test, + // the exact assumption that cost a week of misdiagnosis. The assumption is now + // DISPROVEN: the insert is accepted, its sibling fields take effect, and the created + // broadcast still comes back `bound=NONE`, stuck in lifeCycleStatus `created`, never + // live, with close-out 403ing `invalidTransition`. A test that pins disproven + // behaviour is worse than no test — it launders a guess into a guarantee. + // + // So the guarantee is inverted: assert the insert does NOT claim to bind, and assert + // the real binding call targets the documented bind endpoint with both ids. [Fact] - public async Task CreateBroadcast_With_StreamId_Binds_Reusable_Stream_At_Insert() + public async Task CreateBroadcast_Does_Not_Claim_To_Bind_And_Omits_BoundStreamId() { var handler = new RecordingHandler { ResponseBody = """{"id":"BC123","snippet":{"title":"t"}}""" }; var service = new YouTubeStreamService(CreateAuthed(), new HttpClient(handler)); - var id = await service.CreateBroadcast("Test Stream", "A description", DateTime.UtcNow, "S456"); + var id = await service.CreateBroadcast("Test Stream", "A description", DateTime.UtcNow); Assert.Equal("BC123", id); Assert.NotNull(handler.LastBody); - Assert.Contains("\"boundStreamId\":\"S456\"", handler.LastBody); + // The trap: boundStreamId reads as a binding and is not one. + Assert.DoesNotContain("boundStreamId", handler.LastBody); + // The flags that DO take effect must still be sent. Assert.Contains("\"enableMonitorStream\":false", handler.LastBody); Assert.Contains("\"latencyPreference\":\"low\"", handler.LastBody); Assert.Contains("\"privacyStatus\":\"private\"", handler.LastBody); } + /// The call that actually binds — the one whose absence is why nothing ever + /// went live. Google's documented shape is + /// `POST liveBroadcasts/bind?part=id&id=…&streamId=…` with NO request body (an empty + /// body would be rejected). The parameter is `id`, not `broadcastId` — a plausible + /// near-miss that returns 400 `idRequired`. + [Fact] + public async Task BindBroadcastAsync_Posts_To_Bind_Endpoint_With_Both_Ids_And_No_Body() + { + var handler = new RecordingHandler { ResponseBody = """{"id":"BC123"}""" }; + var service = new YouTubeStreamService(CreateAuthed(), new HttpClient(handler)); + + var error = await service.BindBroadcastAsync("BC123", "S456"); + + Assert.Null(error); + var request = Assert.Single(handler.Requests); + Assert.StartsWith("POST ", request); + Assert.Contains("/liveBroadcasts/bind", request); + Assert.Contains("part=id", request); + Assert.Contains("id=BC123", request); + Assert.Contains("streamId=S456", request); + Assert.True(string.IsNullOrEmpty(handler.LastBody), "bind takes no request body"); + } + // TASK 5 design decision 2: reuse the existing cdn.isReusable stream instead // of creating a new one per broadcast. [Fact]