diff --git a/docs/roadmap/implementation.md b/docs/roadmap/implementation.md index 789ff08..13db00c 100644 --- a/docs/roadmap/implementation.md +++ b/docs/roadmap/implementation.md @@ -184,6 +184,9 @@ Focused check: `go test ./internal/engine -run 'Projection|Inline|Resolution'`. Finding: **B01**. Owners: `internal/engine/resolution.go`, `validation.go`, callback tests. +Status: **complete** (B01). Prior stage commits: `2736ddf`, `deb9aa5`, +`ae3408a`, `ea27f17`. Validation passed: `GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -run 'Callback|Resolution|Validation|Version' -count=1` and `GOTOOLCHAIN=local GOPROXY=off GOTELEMETRY=off go test ./internal/engine -count=1`. + Wrap each application callback invocation in a small function with deferred context deactivation. The defer must run when that callback returns or panics, not at the end of subsequent projection/auditing work. Apply this independently diff --git a/internal/engine/callback_panic_test.go b/internal/engine/callback_panic_test.go new file mode 100644 index 0000000..e3bb5ee --- /dev/null +++ b/internal/engine/callback_panic_test.go @@ -0,0 +1,73 @@ +package engine + +import ( + "errors" + "testing" +) + +func capturePanic(call func()) (recovered any) { + defer func() { recovered = recover() }() + call() + return nil +} + +func requirePanicValue(t *testing.T, want any, call func()) { + t.Helper() + if got := capturePanic(call); got != want { + t.Fatalf("panic = %#v, want %#v", got, want) + } +} + +func TestCallbackContextsCloseWhenCallbacksPanic(t *testing.T) { + type config struct { + Value string `yaml:"value"` + Version string `yaml:"version"` + } + + t.Run("resolution", func(t *testing.T) { + sentinel := errors.New("resolution panic sentinel") + var retained *Resolution + got := capturePanic(func() { + _, _ = testLoad[config](NoSource(), Options[config]{Definition: Definition[config]{Resolve: func(_ *config, resolution *Resolution) error { + retained = resolution + panic(sentinel) + }}}) + }) + if got != sentinel { + t.Fatalf("callback panic = %#v, want original sentinel", got) + } + requirePanicValue(t, closedCallbackContextMessage, func() { retained.IsExplicit(Keys("value")) }) + }) + + t.Run("validation", func(t *testing.T) { + sentinel := errors.New("validation panic sentinel") + var retained *Validation + got := capturePanic(func() { + _, _ = testLoad[config](NoSource(), Options[config]{Definition: Definition[config]{Validate: func(_ *config, validation *Validation) { + retained = validation + panic(sentinel) + }}}) + }) + if got != sentinel { + t.Fatalf("callback panic = %#v, want original sentinel", got) + } + requirePanicValue(t, closedCallbackContextMessage, func() { retained.Error(Keys("value"), "too late") }) + }) + + t.Run("version", func(t *testing.T) { + sentinel := errors.New("version panic sentinel") + var retained *Validation + got := capturePanic(func() { + _, _ = testLoad[config](Bytes("version.yml", []byte("version: v1\n")), Options[config]{Definition: Definition[config]{ + Version: &VersionRule{Path: Keys("version"), Check: func(_ Version, validation *Validation) error { + retained = validation + panic(sentinel) + }}, + }}) + }) + if got != sentinel { + t.Fatalf("callback panic = %#v, want original sentinel", got) + } + requirePanicValue(t, closedCallbackContextMessage, func() { retained.Warning(Keys("version"), "too late") }) + }) +} diff --git a/internal/engine/resolution.go b/internal/engine/resolution.go index e00546d..1b56977 100644 --- a/internal/engine/resolution.go +++ b/internal/engine/resolution.go @@ -287,8 +287,11 @@ func runResolution[T any](configuration *T, resolver func(*T, *Resolution) error records: make(map[string]*resolutionRecord), } context := &Resolution{state: state} - err := resolver(configuration, context) - state.active = false + var err error + func() { + defer func() { state.active = false }() + err = resolver(configuration, context) + }() if err != nil && !hasErrors(state.diagnostics) { state.addDiagnostic(CodeResolveHookFailed, Path{}, "application resolution failed") } diff --git a/internal/engine/validation.go b/internal/engine/validation.go index 740d1f6..77837ff 100644 --- a/internal/engine/validation.go +++ b/internal/engine/validation.go @@ -102,8 +102,10 @@ func runValidation[T any](configuration *T, validator func(*T, *Validation), sch invalidPath: CodeValidateInvalidPath, schema: schema, document: input, } context := &Validation{state: state} - validator(configuration, context) - state.active = false + func() { + defer func() { state.active = false }() + validator(configuration, context) + }() after, issue := projectConfiguration(schema, reflect.ValueOf(*configuration), defaultProjectionLimits()) if issue != nil { state.diagnostics = append(state.diagnostics, validationMutationDiagnostic(issue.path)) @@ -253,8 +255,11 @@ func reportVersion(rule *VersionRule, version Version, root *document.Node) []Di invalidPath: CodeVersionError, document: root, } validation := &Validation{state: state} - err := rule.Check(version, validation) - state.active = false + var err error + func() { + defer func() { state.active = false }() + err = rule.Check(version, validation) + }() if err != nil && !hasErrors(state.diagnostics) { location, _ := nearestDocumentLocation(root, rule.Path) state.diagnostics = append(state.diagnostics, Diagnostic{