From e709bc94e26c26e67094437ee3443c683b825b7b Mon Sep 17 00:00:00 2001 From: AAA <35992542+TNTcraftHIM@users.noreply.github.com> Date: Thu, 20 Aug 2026 17:43:15 +0800 Subject: [PATCH] fix(native): classify local bridge read failures (#73) --- .../0006-fixed-high-native-sender-canary.md | 25 +- docs/project-memory.md | 2 +- docs/research/native-shared-encode-sender.md | 39 ++- docs/status.md | 19 +- native/sender/internal/app/server.go | 15 ++ native/sender/internal/app/server_test.go | 236 ++++++++++++++++++ 6 files changed, 305 insertions(+), 31 deletions(-) diff --git a/docs/adr/0006-fixed-high-native-sender-canary.md b/docs/adr/0006-fixed-high-native-sender-canary.md index 9a56254..0c95b4e 100644 --- a/docs/adr/0006-fixed-high-native-sender-canary.md +++ b/docs/adr/0006-fixed-high-native-sender-canary.md @@ -100,14 +100,23 @@ This remains `no-go-unclassified`. The frozen probe counted the first encoded chunk before invoking the product output callback and did not record whether the generation-bound local bridge attempted, returned from, or threw during its binary send. Static inspection finds matching 17-byte big-endian frame contracts -on the Web and Go sides, but cannot convert that into runtime evidence. The proven -break interval is therefore after WebCodecs output callback entry and before Go -frame/source-RTP accounting, not a proven product bridge defect. The current -unrun gate now records only bounded attempt/synchronous-return/synchronous-throw -counters for the first local bridge generation and rechecks that identity at -Viewer signal, Viewer media, and final success. Pure tests reject late -replacement. A further run requires separate authorization. There is still no -Viewer, two-viewer, FIFO, or physical/hardware-encoder proof. +on the Web and Go sides. A focused integration now configures the real `/media` +handler, sends one same-contract binary frame, and observes positive Go frame and +source-RTP accounting. That proves the current envelope can enter the current Go +fanout in a controlled test, not that the retained browser executed its send. + +The handler no longer returns silently for every post-config bridge read error: +while the app context is active, any close other than normal or going-away emits +one `fatal` with the fixed sanitized message `local media bridge read failed`. +Focused tests prove the abnormal classification without its underlying close +reason and prove normal close and app shutdown do not misreport. This is a real +P1 bridge-read failure fix, but it does not classify the retained zero counters +or prove a framing defect. The current unrun gate still records only bounded +attempt/synchronous-return/synchronous-throw counters for the first local bridge +generation and rechecks that identity at Viewer signal, Viewer media, and final +success. Pure tests reject late replacement. No Chrome rerun occurred; another +run requires separate authorization. There is still no Viewer, two-viewer, FIFO, +or physical/hardware-encoder proof. ## Staged Revalidation diff --git a/docs/project-memory.md b/docs/project-memory.md index 3475ff5..66e6edd 100644 --- a/docs/project-memory.md +++ b/docs/project-memory.md @@ -23,7 +23,7 @@ Last updated: 2026-08-20 - Use direct host P2P for one or two viewers. Keep ADR-0004 off broadly; use an isolated exact-room candidate until resource, quality, recovery, SFU/UDP, bounded-failure, and browser/mobile gates pass. - Browser relays resend remote `MediaStreamTrack` values and re-encode at each hop; WebRTC does not guarantee a shared encoder across peer connections, so measure the cost. - Keep experiments bounded: the standard representation sequence is below; native RTP relay, encoded-object striping, and FEC remain separate. -- ADR-0006 stays no-go: the second product run reached first WebCodecs output, then zero Go frame/RTP and no Viewer; cleanup passed 5/5. A separate offline Media Foundation fixture proved one RTX 4070 SUPER H.264 hardware path over 360/360 frames, but its valid Constrained Baseline SPS `42c01f` is outside Pion's default exact fmtp set; Pion/Viewer interop remains unproven. Native v2 stays memory-only and leaves SQLite/Web unchanged. +- ADR-0006 stays no-go: retained Chrome reached WebCodecs output, then Go frame/RTP stayed zero and no Viewer was created. A real `/media` integration now accepts the current 17-byte binary frame into Go frame/RTP; unexpected post-config reads emit a fixed sanitized fatal. No Chrome rerun occurred, so the retained product-send/framing cause remains unproved. A separate MF fixture proved RTX H.264 hardware over 360/360 frames, but `42c01f` misses Pion's default fmtp; Viewer interop remains unproven. Native v2 stays memory-only and leaves SQLite/Web unchanged. - Mobile Web Host is unsupported; feature-detect and fail clearly. Mobile Viewer stays leaf-only. After Windows native, gate Android 14+; iOS waits for stable iOS 27 ScreenCaptureKit. - ADR-0005 accepts direct/peer UDP, bounded SFU roots, then optional selected-edge TURN. The built-in participant-wide candidate failed its real direct acceptance before forced relay and rolled back completely; it is rejected as the product path. Production advertises STUN-only ICE. - Keep the controller exact-room only: room `1` is the STUN/SFU smoke. Preserve sticky progressing P2P, mobile leaves and break-before-make. Recovery spends one attempt per layer (ICE restart, same-parent rebuild, alternate peer, then SFU), never three identical retries; active SFU gets one fresh grant before Peer failback. diff --git a/docs/research/native-shared-encode-sender.md b/docs/research/native-shared-encode-sender.md index d406a86..242e584 100644 --- a/docs/research/native-shared-encode-sender.md +++ b/docs/research/native-shared-encode-sender.md @@ -280,18 +280,33 @@ encoded-output observation before it calls the product output callback, and that run did not retain whether the current local media WebSocket attempted, returned from, or threw during the binary frame send. Static source comparison shows a matching binary message type, 17-byte big-endian header, one-MiB payload limit, -and VP8 frame decode on both sides. It also confirms that Pion's unbound static RTP track -returns no error. Those facts do not prove which runtime operation followed the -callback. The retained first break interval is after WebCodecs output callback -entry and before Go frame/source-RTP accounting. There is no uniquely proven -product bug. The current unrun probe now binds the first actual local media -WebSocket as gate-local generation 1 and records only bounded binary-send -attempt, synchronous-return, and synchronous-throw counters. A second bridge -generation saturates at 2, stops accumulating the first generation's counters, -and fails the Viewer signal, Viewer media, and final-success checks even after -Sender-start passed. A pure in-memory test covers success, throw, and late -replacement without retaining a URL, payload, token, SDP, IP address, or raw -error. No subsequent browser/media run is authorized or has occurred. +and VP8 frame decode on both sides. A focused test now drives the real local +`/media` WebSocket through ready, fixed config acceptance, one same-contract +binary frame, decode, fanout, and positive `framesWritten` plus source-RTP packet +accounting. Pion's unbound static RTP track also returns no error. This proves +the current envelope is consumable by the current Go handler; it does not prove +that the retained browser called or returned from the product send. + +The concrete P1 defect at this boundary was silent handling of every unexpected +post-config WebSocket read error. The handler now emits one `fatal` with fixed +message `local media bridge read failed` only while the app context is active +and the close status is neither normal nor going-away. It never includes the +underlying error or close reason. Focused real-WebSocket tests cover an abnormal +post-config close, normal close, and app shutdown. All Go tests, vet, Windows +amd64 no-CGO build, TypeScript typecheck, and the relevant bridge probe/ledger +tests pass. + +The retained first break interval is still after WebCodecs output callback entry +and before Go frame/source-RTP accounting. This fix does not prove the retained +zero counters came from a read error or framing defect. The current unrun probe +still binds the first actual local media WebSocket as gate-local generation 1 +and records only bounded binary-send attempt, synchronous-return, and +synchronous-throw counters. A second bridge generation saturates at 2, stops +accumulating the first generation's counters, and fails the Viewer signal, +Viewer media, and final-success checks even after Sender-start passed. A pure +in-memory test covers success, throw, and late replacement without retaining a +URL, payload, token, SDP, IP address, or raw error. No subsequent Chrome run was +authorized or occurred. Chromium documents the tab-capture auto-selection switch as a test-only aid, and Chrome requires a non-default user-data directory for remote debugging from diff --git a/docs/status.md b/docs/status.md index ceb67df..f0e71e4 100644 --- a/docs/status.md +++ b/docs/status.md @@ -35,7 +35,7 @@ and is not part of production. - Host A+B and authenticated P2P Viewer C are sanitized, generation-bound, read-only, and fail closed for stale, ambiguous, or SFU-fed evidence; no raw media metadata is retained. - SVC is `no-go-web-svc-cross-path-hardware-contract`: no direct/peer selection, cross-PC shared encode, or portable hardware proof; pinned screen share is `L1T3`. No browser run was warranted. - The `a11a73d` built-in TURN canary is rejected: local allocation passed, but direct Host/Pion Viewer failed before forced relay. Full rollback restored STUN-only client ICE and zero allocations. -- Native stays no-go at WebCodecs -> Go RTP. MF proved RTX H.264 hardware (360/360, p95 11.575 ms); `42c01f` misses Pion's default fmtp, so Viewer interop is unproven. +- Native stays no-go: retained Chrome reached WebCodecs output but Go stayed zero. A real `/media` test accepts the 17-byte frame into Go frame/RTP; unexpected post-config reads emit a fixed sanitized fatal. No Chrome rerun occurred, so retained send/framing cause and MF H.264-to-Pion/Viewer interop remain unproven. - Access protocol/config/HTTP/storage/SQLite/signaling focused tests pass, including commit-first teardown and v1 rollback. ## Unverified Boundaries @@ -45,7 +45,7 @@ and is not part of production. - Silent partitions can wait 30 to 60 seconds for heartbeat detection before the default 5-second grace; this remains unverified. - Real audio and heterogeneous clients remain unverified. Production reports poor film audio and voice-call self-echo under system capture; there is no app audio ceiling or Web process isolation. Diagnose A/B/C and sync, then gate Windows 11 game-process audio; Windows 10 stays unresolved without system fallback. - The former release reportedly sustained bandwidth-limited blur on a capable LAN; Host refresh recovered while Viewer refresh did not. The new deployment has not yet reproduced or cleared it. -- ADR-0006 stays no-go at WebCodecs output -> Go frame/RTP. Viewer and two-edge/FIFO remain unverified. +- ADR-0006 still has no Viewer, two-edge, or FIFO proof. - Browser fanout is host two/viewer one; any accepted endpoint relay stays capped at two downstream edges. - PR #49/#50 BWE is unverified/default-off: test zero-child leaves, then root impact. No local per-leaf UDP shaper; resume with Linux `tc` or public canary, never CDP. Web P2P/SVC shortcuts remain no-go. - The corrected standby smoke is localhost/headless/video-only; public DNS/TLS reuse, transport, audio, shaping, load, mobile, endurance, and sub-25 ms overlap remain open. @@ -60,15 +60,14 @@ The recovery target uses one ICE restart, one same-parent rebuild, one alternate SFU; it never runs three identical retries or abandons progressing P2P early. Next gate exactly-two/Dynacast-off BWE on a zero-child SFU leaf, then root -evacuation. Native needs separate authorization; its next run gates Go RTP on -the first bridge send. +evacuation. Native needs separate authorization; its next Chrome run must retain +the first product binary-send outcome before Go RTP. Audio A/B/C may proceed without displacing P0. -Ordinary Peer ICE stays STUN-only. Participant-wide TURN code and wire are -removed; stale `PEER_ICE_TURN_*` keys fail startup even when blank. Selected-edge -TURN remains unimplemented and undeployed. After SFU/UDP fails, the next -independent slice must bind one selected parent/child rebuild to current -generations before one forced-relay canary. Do not repeat the built-in direct -canary. +Ordinary Peer ICE stays STUN-only. Participant-wide TURN source and wire are +removed; stale `PEER_ICE_TURN_*` keys fail startup even blank. Selected-edge TURN +is unimplemented and undeployed. After SFU/UDP, bind one selected parent/child +rebuild to current generations before one forced-relay canary; do not repeat the +built-in direct canary. ADR-0004 still requires a full-resolution 30-minute `1/3/5/8` network, resource, quality, latency, recovery, and browser/mobile-leaf matrix. A separate diff --git a/native/sender/internal/app/server.go b/native/sender/internal/app/server.go index 9f80d99..68ee1a9 100644 --- a/native/sender/internal/app/server.go +++ b/native/sender/internal/app/server.go @@ -332,6 +332,9 @@ func (app *App) handleMedia(response http.ResponseWriter, request *http.Request) for { messageType, payload, err = connection.Read(app.ctx) if err != nil { + if failure := postConfigMediaReadFailure(app.ctx, err); failure != nil { + session.Fail(failure) + } return } if messageType != websocket.MessageBinary { @@ -351,6 +354,18 @@ func (app *App) handleMedia(response http.ResponseWriter, request *http.Request) } } +func postConfigMediaReadFailure(appContext context.Context, err error) error { + if err == nil || appContext.Err() != nil { + return nil + } + switch websocket.CloseStatus(err) { + case websocket.StatusNormalClosure, websocket.StatusGoingAway: + return nil + default: + return errors.New("local media bridge read failed") + } +} + func (app *App) authorizeAPI(response http.ResponseWriter, request *http.Request, method string) bool { if !exactRequest(request, app.expectedHost, app.origin) || !tokenMatches(request.Header.Get(processTokenHeader), app.token) { diff --git a/native/sender/internal/app/server_test.go b/native/sender/internal/app/server_test.go index 3e722ec..6cc8768 100644 --- a/native/sender/internal/app/server_test.go +++ b/native/sender/internal/app/server_test.go @@ -3,13 +3,17 @@ package app import ( "bytes" "context" + "encoding/binary" "encoding/json" + "errors" "net/http" "net/http/httptest" "strconv" + "strings" "testing" "time" + "github.com/TNTcraftHIM/Screener/native/sender/internal/media" "github.com/TNTcraftHIM/Screener/native/sender/internal/remote" "github.com/coder/websocket" ) @@ -172,6 +176,109 @@ func TestAcceptedMediaWithoutConfigFailsWithinTheHandshakeDeadline(t *testing.T) assertAbandonRoom(t, remoteDone) } +func TestConfiguredMediaBinaryFrameReachesFanout(t *testing.T) { + application, events, terminal := startObservedTestApplication(t) + connection := openConfiguredMedia(t, application) + + payload := make([]byte, media.FrameHeaderBytes+4) + payload[0] = 1 + binary.BigEndian.PutUint64(payload[1:9], 1_000_000) + binary.BigEndian.PutUint64(payload[9:17], 33_333) + copy(payload[media.FrameHeaderBytes:], []byte{0x10, 0x00, 0x00, 0x00}) + writeContext, cancelWrite := context.WithTimeout(context.Background(), time.Second) + err := connection.Write(writeContext, websocket.MessageBinary, payload) + cancelWrite() + if err != nil { + t.Fatalf("write encoded frame: %v", err) + } + + diagnostics := awaitRemoteEvent(t, events, "diagnostics", 5*time.Second) + if diagnostics.Media == nil || diagnostics.Media.FramesWritten == 0 || + diagnostics.Media.SourceRTPPacketsWritten == 0 { + t.Fatalf("binary frame did not reach fanout: %+v", diagnostics.Media) + } + if err = connection.Close(websocket.StatusNormalClosure, "test complete"); err != nil { + t.Fatalf("close configured media: %v", err) + } + waitForSessionStop(t, application) + assertAbandonRoom(t, terminal) + assertNoFatalEvents(t, events) +} + +func TestAbnormalPostConfigMediaReadUsesFixedFailure(t *testing.T) { + application, events, terminal := startObservedTestApplication(t) + connection := openConfiguredMedia(t, application) + const privateReason = "transport detail that must not escape" + + if err := connection.Close(websocket.StatusPolicyViolation, privateReason); err != nil { + t.Fatalf("close configured media abnormally: %v", err) + } + fatal := awaitRemoteEvent(t, events, "fatal", 2*time.Second) + if fatal.Message != "local media bridge read failed" { + t.Fatalf("fatal message = %q", fatal.Message) + } + if strings.Contains(fatal.Message, privateReason) { + t.Fatalf("fatal message exposed the WebSocket close reason: %q", fatal.Message) + } + waitForSessionStop(t, application) + assertAbandonRoom(t, terminal) +} + +func TestApplicationShutdownDoesNotReportMediaReadFailure(t *testing.T) { + application, events, terminal := startObservedTestApplication(t) + connection := openConfiguredMedia(t, application) + + closed := make(chan struct{}) + go func() { + application.Close() + close(closed) + }() + readContext, cancelRead := context.WithTimeout(context.Background(), time.Second) + _, _, err := connection.Read(readContext) + cancelRead() + if websocket.CloseStatus(err) != websocket.StatusNormalClosure { + t.Fatalf("application close status = %v, err = %v", websocket.CloseStatus(err), err) + } + select { + case <-closed: + case <-time.After(2 * time.Second): + t.Fatal("application shutdown did not complete") + } + assertAbandonRoom(t, terminal) + assertNoFatalEvents(t, events) +} + +func TestPostConfigMediaReadFailureClassification(t *testing.T) { + canceledContext, cancel := context.WithCancel(context.Background()) + cancel() + tests := []struct { + name string + ctx context.Context + err error + wantErr bool + }{ + {name: "transport-error", ctx: context.Background(), err: errors.New("private transport detail"), wantErr: true}, + {name: "policy-close", ctx: context.Background(), err: websocket.CloseError{Code: websocket.StatusPolicyViolation, Reason: "private close detail"}, wantErr: true}, + {name: "normal-close", ctx: context.Background(), err: websocket.CloseError{Code: websocket.StatusNormalClosure}}, + {name: "going-away", ctx: context.Background(), err: websocket.CloseError{Code: websocket.StatusGoingAway}}, + {name: "application-shutdown", ctx: canceledContext, err: errors.New("private shutdown detail")}, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + failure := postConfigMediaReadFailure(test.ctx, test.err) + if test.wantErr { + if failure == nil || failure.Error() != "local media bridge read failed" { + t.Fatalf("failure = %v", failure) + } + return + } + if failure != nil { + t.Fatalf("failure = %v", failure) + } + }) + } +} + func TestStopDuringDelayedStartCannotResurrectTheSession(t *testing.T) { authReceived := make(chan struct{}) releaseAuthentication := make(chan struct{}) @@ -243,6 +350,135 @@ func startTestApplication(t *testing.T, remoteURL string, attachTimeout, configT return application } +func startObservedTestApplication(t *testing.T) (*App, <-chan remote.Event, <-chan []byte) { + t.Helper() + terminal := make(chan []byte, 1) + remoteServer := newAppRemoteServer(t, terminal) + t.Cleanup(remoteServer.Close) + + application, err := New() + if err != nil { + t.Fatal(err) + } + if _, err = application.Start(); err != nil { + t.Fatal(err) + } + t.Cleanup(application.Close) + + baseURL, err := normalizeRemoteBase(remoteServer.URL) + if err != nil { + t.Fatal(err) + } + sessionContext, sessionCancel := context.WithCancel(application.ctx) + events := make(chan remote.Event, 32) + session, room, err := remote.Start(sessionContext, remote.StartOptions{ + BaseURL: baseURL, + OnEvent: func(event remote.Event) { + events <- event + }, + }) + if err != nil { + sessionCancel() + t.Fatal(err) + } + application.mu.Lock() + application.generation++ + application.session = session + application.sessionCancel = sessionCancel + application.room = room + application.mu.Unlock() + return application, events, terminal +} + +func openConfiguredMedia(t *testing.T, application *App) *websocket.Conn { + t.Helper() + connection, _, err := websocket.Dial(context.Background(), application.origin+"/media", &websocket.DialOptions{ + HTTPHeader: http.Header{"Origin": []string{application.origin}}, + Subprotocols: []string{"screener.token." + application.token}, + }) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = connection.CloseNow() }) + readLocalKind(t, connection, "ready") + config := []byte(`{"kind":"config","codec":"vp8","width":1280,"height":720,"fps":30,"bitrate":3000000,"encoderInstances":1}`) + writeContext, cancelWrite := context.WithTimeout(context.Background(), time.Second) + err = connection.Write(writeContext, websocket.MessageText, config) + cancelWrite() + if err != nil { + t.Fatalf("write encoder config: %v", err) + } + readLocalKind(t, connection, "config-accepted") + return connection +} + +func readLocalKind(t *testing.T, connection *websocket.Conn, want string) { + t.Helper() + readContext, cancelRead := context.WithTimeout(context.Background(), time.Second) + defer cancelRead() + messageType, payload, err := connection.Read(readContext) + if err != nil { + t.Fatalf("read local %s event: %v", want, err) + } + if messageType != websocket.MessageText { + t.Fatalf("local %s event was not text", want) + } + var event struct { + Kind string `json:"kind"` + } + if err = json.Unmarshal(payload, &event); err != nil { + t.Fatalf("decode local %s event: %v", want, err) + } + if event.Kind != want { + t.Fatalf("local event kind = %q, want %q", event.Kind, want) + } +} + +func awaitRemoteEvent(t *testing.T, events <-chan remote.Event, kind string, timeout time.Duration) remote.Event { + t.Helper() + timer := time.NewTimer(timeout) + defer timer.Stop() + for { + select { + case event := <-events: + if event.Kind == kind { + return event + } + case <-timer.C: + t.Fatalf("remote %s event was not emitted", kind) + } + } +} + +func waitForSessionStop(t *testing.T, application *App) { + t.Helper() + deadline := time.Now().Add(2 * time.Second) + for time.Now().Before(deadline) { + application.mu.Lock() + stopped := application.session == nil + application.mu.Unlock() + if stopped { + return + } + time.Sleep(time.Millisecond) + } + t.Fatal("media session did not stop") +} + +func assertNoFatalEvents(t *testing.T, events <-chan remote.Event) { + t.Helper() + for { + select { + case event := <-events: + if event.Kind == "fatal" { + t.Fatalf("unexpected fatal event: %q", event.Message) + } + default: + return + } + } +} + func stopTestApplication(t *testing.T, application *App) { t.Helper() if status := postLocal(t, application, "/api/stop", nil); status != http.StatusNoContent {