From 323878c34ac1d7257a341172c66242645b88a77c Mon Sep 17 00:00:00 2001 From: pjay Date: Mon, 20 Apr 2026 16:55:10 +0700 Subject: [PATCH] fix(verifier): don't crash on throwing JS predicates (#21) * fix(verifier): don't crash on throwing JS predicates formulaThunk used to panic whenever goja returned an error from a predicate callable, and nothing on the LTL -> runner path recovered, so a malformed spec (e.g. a property whose body throws or touches an undefined field) would kill the verifier process. Latch the first error on formulaState, return false so LTL marks the property violated, and expose PredicateError(name) that walks the property's formula-spec tree and surfaces the latched cause. * fix(runner): log predicate errors alongside violations For each violated property, surface the verifier's latched predicate error via logger.Warn so operators can distinguish a genuine false verdict from a malformed spec. Add a runner-level test asserting that a throwing predicate no longer crashes the run and that the error message appears in the log. --- internal/runner/runner.go | 5 ++++ internal/runner/runner_test.go | 43 +++++++++++++++++++++++++++++- internal/verifier/bindings.go | 4 +++ internal/verifier/verifier_test.go | 27 +++++++++++++++++++ internal/verifier/worker.go | 36 ++++++++++++++++++++++++- 5 files changed, 113 insertions(+), 2 deletions(-) diff --git a/internal/runner/runner.go b/internal/runner/runner.go index b549e68..07136a5 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -108,6 +108,11 @@ func Run(ctx context.Context, options Options) (Summary, error) { stepIndex, screen, treeSize) verdicts := options.Verifier.EvaluateProperties() violations := violationNames(verdicts) + for _, name := range violations { + if predicateErr := options.Verifier.PredicateError(name); predicateErr != nil { + logger.Warn("predicate error", "step", stepIndex, "property", name, "err", predicateErr) + } + } nextAction, nextErr := options.Verifier.NextAction() var traceAction *trace.Action diff --git a/internal/runner/runner_test.go b/internal/runner/runner_test.go index bf00f16..6c4d643 100644 --- a/internal/runner/runner_test.go +++ b/internal/runner/runner_test.go @@ -42,6 +42,10 @@ type harness struct { } func newHarness(t *testing.T, snapshots []map[string]json.RawMessage) *harness { + return newHarnessWithSpec(t, snapshots, fixtureSpec) +} + +func newHarnessWithSpec(t *testing.T, snapshots []map[string]json.RawMessage, spec string) *harness { t.Helper() listener, err := net.Listen("tcp", "127.0.0.1:0") if err != nil { @@ -57,7 +61,7 @@ func newHarness(t *testing.T, snapshots []map[string]json.RawMessage) *harness { if err != nil { t.Fatal(err) } - if err := verifierInstance.Load(fixtureSpec); err != nil { + if err := verifierInstance.Load(spec); err != nil { t.Fatal(err) } state := &harness{ @@ -188,6 +192,43 @@ func TestRunner_ViolationSurfacesInSummary(t *testing.T) { } } +func TestRunner_ThrowingPredicateIsLoggedNotPanic(t *testing.T) { + const throwingSpec = ` +globalThis.properties = { + broken: __uatu__.always(() => { throw new Error("bad predicate"); }), +}; +globalThis.actions = __uatu__.actions(() => [__uatu__.tap({ on: "id:next" })]); +` + state := newHarnessWithSpec(t, []map[string]json.RawMessage{{}, {}}, throwingSpec) + state.startSDK(t) + state.acceptConnection(t) + + var buffer bytes.Buffer + logger := slog.New(slog.NewTextHandler(&buffer, &slog.HandlerOptions{Level: slog.LevelWarn})) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + summary, err := Run(ctx, Options{ + Duration: 100 * time.Millisecond, + SnapshotTimeout: 2 * time.Second, + IdleTimeout: 50 * time.Millisecond, + Connection: state.conn, + Driver: state.mock, + Verifier: state.verifier, + TraceWriter: state.writer, + Logger: logger, + }) + if err != nil { + t.Fatalf("Run: %v", err) + } + if !containsProperty(summary.Violations, "broken") { + t.Errorf("expected broken in violations: %v", summary.Violations) + } + if !strings.Contains(buffer.String(), "bad predicate") { + t.Errorf("expected predicate error in log, got %q", buffer.String()) + } +} + func TestRunner_RejectsMissingFields(t *testing.T) { _, err := Run(context.Background(), Options{Duration: time.Second}) if err == nil || !strings.Contains(err.Error(), "Connection") { diff --git a/internal/verifier/bindings.go b/internal/verifier/bindings.go index b1c3a9d..20fca55 100644 --- a/internal/verifier/bindings.go +++ b/internal/verifier/bindings.go @@ -41,6 +41,10 @@ type extractorState struct { type formulaState struct { predicate goja.Callable + // err latches the first goja error returned by predicate. The thunk + // returns false on error so the LTL evaluator marks the property + // violated; PredicateError surfaces the underlying cause. + err error } type specKind int diff --git a/internal/verifier/verifier_test.go b/internal/verifier/verifier_test.go index ca9d555..8c58f33 100644 --- a/internal/verifier/verifier_test.go +++ b/internal/verifier/verifier_test.go @@ -219,6 +219,33 @@ func TestLoad_PropagatesSyntaxError(t *testing.T) { } } +func TestEvaluateProperties_ThrowingPredicateDoesNotPanic(t *testing.T) { + const spec = ` +globalThis.properties = { + broken: __uatu__.always(() => { throw new Error("bad predicate"); }), +}; +` + verifier := newVerifier(t) + mustLoad(t, verifier, spec) + + if err := verifier.PushSnapshot(SnapshotInput{Snapshots: Snapshots{}}); err != nil { + t.Fatal(err) + } + + verdicts := verifier.EvaluateProperties() + if got := verdicts["broken"]; got != ltl.VerdictViolated { + t.Errorf("verdict: got %v, want %v", got, ltl.VerdictViolated) + } + + predicateErr := verifier.PredicateError("broken") + if predicateErr == nil { + t.Fatal("PredicateError: got nil, want non-nil") + } + if !strings.Contains(predicateErr.Error(), "bad predicate") { + t.Errorf("PredicateError message: got %q, want to contain %q", predicateErr.Error(), "bad predicate") + } +} + func TestLoad_AcceptsSpecWithoutPropertiesOrActions(t *testing.T) { verifier := newVerifier(t) if err := verifier.Load(`const noop = 1;`); err != nil { diff --git a/internal/verifier/worker.go b/internal/verifier/worker.go index 5b8b83f..9d54bc6 100644 --- a/internal/verifier/worker.go +++ b/internal/verifier/worker.go @@ -302,12 +302,46 @@ func (v *Verifier) formulaThunk(index int) func() bool { formula := v.formulas[index] result, err := formula.predicate(goja.Undefined()) if err != nil { - panic(fmt.Errorf("predicate panic: %w", err)) + if formula.err == nil { + formula.err = err + } + return false } return result.ToBoolean() } } +// PredicateError returns the first goja error raised by any thunk in the +// named property's formula tree, or nil if none fired. Callers typically +// consult this after EvaluateProperties reports a violation to distinguish +// a genuine predicate-false verdict from a malformed spec. +func (v *Verifier) PredicateError(name string) error { + rootIndex, ok := v.properties[name] + if !ok { + return nil + } + return v.firstThunkError(rootIndex) +} + +func (v *Verifier) firstThunkError(index int) error { + if index < 0 || index >= len(v.formulaSpecs) { + return nil + } + spec := v.formulaSpecs[index] + switch spec.kind { + case specKindThunk: + return v.formulas[spec.predicateIndex].err + case specKindImplies, specKindOr, specKindAnd: + if err := v.firstThunkError(spec.childA); err != nil { + return err + } + return v.firstThunkError(spec.childB) + case specKindNow, specKindNext, specKindEventually, specKindNot, specKindAlways: + return v.firstThunkError(spec.childA) + } + return nil +} + func (v *Verifier) resolveGenerator(generator goja.Value) (Action, error) { object := generator.ToObject(v.runtime) if object == nil {