diff --git a/internal/observability/fx.go b/internal/observability/fx.go index 21e9759a..4c165282 100644 --- a/internal/observability/fx.go +++ b/internal/observability/fx.go @@ -20,13 +20,28 @@ func (fxLogger) LogEvent(event fxevent.Event) { } func describeFXEventError(event fxevent.Event) (string, map[string]any, error) { + if eventName, fields, ok, err := describeFXBuildEventError(event); ok { + return eventName, fields, err + } + + return describeFXLifecycleEventError(event) +} + +func describeFXBuildEventError(event fxevent.Event) (string, map[string]any, bool, error) { switch e := event.(type) { case *fxevent.Provided: - return "fx_provide_failed", map[string]any{"constructor": e.ConstructorName}, e.Err + return "fx_provide_failed", map[string]any{"constructor": e.ConstructorName}, true, e.Err case *fxevent.Invoked: - return "fx_invoke_failed", map[string]any{"function": e.FunctionName}, e.Err + return "fx_invoke_failed", map[string]any{"function": e.FunctionName}, true, e.Err case *fxevent.Run: - return "fx_run_failed", map[string]any{"function": e.Name, "kind": e.Kind}, e.Err + return "fx_run_failed", map[string]any{"function": e.Name, "kind": e.Kind}, true, e.Err + default: + return "", nil, false, nil + } +} + +func describeFXLifecycleEventError(event fxevent.Event) (string, map[string]any, error) { + switch e := event.(type) { case *fxevent.OnStartExecuted: return "fx_on_start_failed", map[string]any{"caller": e.CallerName, "function": e.FunctionName, "runtime": e.Runtime}, e.Err case *fxevent.OnStopExecuted: diff --git a/internal/observability/fx_test.go b/internal/observability/fx_test.go new file mode 100644 index 00000000..efb55eeb --- /dev/null +++ b/internal/observability/fx_test.go @@ -0,0 +1,119 @@ +package observability + +import ( + "errors" + "reflect" + "testing" + "time" + + "go.uber.org/fx/fxevent" +) + +func TestDescribeFXEventErrorDescribesFailedBuildEvents(t *testing.T) { + err := errors.New("fx failed") + + tests := map[string]struct { + event fxevent.Event + wantName string + wantFields map[string]any + }{ + "provided": { + event: &fxevent.Provided{ConstructorName: "newServer", Err: err}, + wantName: "fx_provide_failed", + wantFields: map[string]any{"constructor": "newServer"}, + }, + "invoked": { + event: &fxevent.Invoked{FunctionName: "startServer", Err: err}, + wantName: "fx_invoke_failed", + wantFields: map[string]any{"function": "startServer"}, + }, + "run": { + event: &fxevent.Run{Name: "newRepo", Kind: "provide", Err: err}, + wantName: "fx_run_failed", + wantFields: map[string]any{"function": "newRepo", "kind": "provide"}, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + assertFXEventError(t, tt.event, tt.wantName, tt.wantFields, err) + }) + } +} + +func TestDescribeFXEventErrorDescribesFailedLifecycleEvents(t *testing.T) { + err := errors.New("fx failed") + runtime := 12 * time.Millisecond + + tests := map[string]struct { + event fxevent.Event + wantName string + wantFields map[string]any + }{ + "on start executed": { + event: &fxevent.OnStartExecuted{CallerName: "server", FunctionName: "listen", Runtime: runtime, Err: err}, + wantName: "fx_on_start_failed", + wantFields: map[string]any{"caller": "server", "function": "listen", "runtime": runtime}, + }, + "on stop executed": { + event: &fxevent.OnStopExecuted{CallerName: "server", FunctionName: "close", Runtime: runtime, Err: err}, + wantName: "fx_on_stop_failed", + wantFields: map[string]any{"caller": "server", "function": "close", "runtime": runtime}, + }, + "started": { + event: &fxevent.Started{Err: err}, + wantName: "fx_start_failed", + }, + "stopped": { + event: &fxevent.Stopped{Err: err}, + wantName: "fx_stop_failed", + }, + "rolling back": { + event: &fxevent.RollingBack{StartErr: err}, + wantName: "fx_rollback_start", + }, + "rolled back": { + event: &fxevent.RolledBack{Err: err}, + wantName: "fx_rollback_failed", + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + assertFXEventError(t, tt.event, tt.wantName, tt.wantFields, err) + }) + } +} + +func assertFXEventError(t *testing.T, event fxevent.Event, wantName string, wantFields map[string]any, wantErr error) { + t.Helper() + + gotName, gotFields, gotErr := describeFXEventError(event) + + if gotName != wantName { + t.Fatalf("event name = %q, want %q", gotName, wantName) + } + if !reflect.DeepEqual(gotFields, wantFields) { + t.Fatalf("fields = %#v, want %#v", gotFields, wantFields) + } + if !errors.Is(gotErr, wantErr) { + t.Fatalf("error = %v, want %v", gotErr, wantErr) + } +} + +func TestDescribeFXEventErrorIgnoresEventsWithoutErrors(t *testing.T) { + tests := map[string]fxevent.Event{ + "provided": &fxevent.Provided{ConstructorName: "newServer"}, + "unknown": &fxevent.Supplied{}, + } + + for name, event := range tests { + t.Run(name, func(t *testing.T) { + _, _, gotErr := describeFXEventError(event) + + if gotErr != nil { + t.Fatalf("error = %v, want nil", gotErr) + } + }) + } +} diff --git a/plans/001-fix-broken-go-lint-baseline.md b/plans/001-fix-broken-go-lint-baseline.md deleted file mode 100644 index c7d395fe..00000000 --- a/plans/001-fix-broken-go-lint-baseline.md +++ /dev/null @@ -1,32 +0,0 @@ ---- -finding: "Fix broken Go lint baseline" -catalog: "DX / Tooling" -impact: "The documented full verification gate cannot pass because Go lint currently fails." -base_commit: "0d31d46" ---- - -## Effort - -S - one focused refactor plus verification. - -## Risk - -LOW - the change should preserve behavior and only reduce function complexity. - -## Confidence - -HIGH - `bun run lint:go` fails reproducibly on one function. - -## Evidence - -- `internal/observability/fx.go:22` defines `describeFXEventError` with a large type switch. -- `bun run lint:go` reports `cyclop`: calculated cyclomatic complexity is 11, max is 10. -- `justfile:40` defines `check` as `lint test typecheck build`, so this failure breaks `just check`. - -## Resolution Approach - -Refactor `describeFXEventError` without changing the log events it emits. Keep the same public behavior: `LogEvent` should call `describeFXEventError`, ignore events with no error, and log failed Fx lifecycle events with the same event names and field maps. - -A small, safe approach is to split the switch into narrower helpers, for example one helper for constructor/invoke/run failures and one helper for lifecycle/start/stop/rollback failures. Another acceptable approach is to use small typed helper functions for repeated return shapes. Do not suppress the linter unless a refactor proves impossible. - -Add or update tests if existing tests cover Fx event descriptions. Verify with `bun run lint:go`, then run `go test ./...`. After this lands, `just check` should be able to proceed past Go lint. diff --git a/plans/002-stop-marking-json-attributes-trusted-html.md b/plans/002-stop-marking-json-attributes-trusted-html.md deleted file mode 100644 index d8ec8673..00000000 --- a/plans/002-stop-marking-json-attributes-trusted-html.md +++ /dev/null @@ -1,35 +0,0 @@ ---- -finding: "Stop marking JSON attributes as trusted HTML" -catalog: "Security" -impact: "Provider-controlled strings can break quoted data attributes on the watch page, creating an HTML injection risk." -base_commit: "0d31d46" ---- - -## Effort - -S - localized template helper and tests. - -## Risk - -LOW - the intended behavior is still to serialize data for client-side parsing, but escaping must be handled by `html/template`. - -## Confidence - -HIGH - the current helper explicitly bypasses contextual escaping. - -## Evidence - -- `templates/funcs.go:37-42` marshals JSON and returns `template.HTMLAttr`. -- `templates/renderer.go:46-50` registers the helper as `json`. -- `templates/components/video_player.gohtml:10-12` embeds JSON inside single-quoted `data-*` attributes. -- `internal/playback/watch_data.go:181-188` copies provider subtitle labels into that serialized payload. - -## Resolution Approach - -Change the JSON serialization path so marshaled JSON is not marked as trusted `template.HTMLAttr`. Let `html/template` perform contextual escaping, or move larger JSON payloads into `