From d12935f7f6c91ccfb24464280386cf71cc6e073a Mon Sep 17 00:00:00 2001 From: PJ Date: Sat, 6 Jun 2026 11:23:37 +0530 Subject: [PATCH] fix(runner): absorb one-off apply errors; only an unbroken streak aborts --- internal/runner/runner.go | 52 +++++++------------------ internal/runner/runner_test.go | 71 +++++++++++++++++++--------------- 2 files changed, 54 insertions(+), 69 deletions(-) diff --git a/internal/runner/runner.go b/internal/runner/runner.go index ce27850..ae5d640 100644 --- a/internal/runner/runner.go +++ b/internal/runner/runner.go @@ -14,8 +14,6 @@ import ( "time" "golang.org/x/sync/errgroup" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" "github.com/priyanshujain/sanderling/internal/driver" "github.com/priyanshujain/sanderling/internal/hierarchy" @@ -232,18 +230,23 @@ func Run(ctx context.Context, options Options) (Summary, error) { if isWDADrop(err) { return summary, fmt.Errorf("step %d: the iOS XCTest runner could not be restarted - re-run the test: %w", stepIndex, err) } - if isTransientApplyError(ctx, err) { - consecutiveApplyFailures++ - if consecutiveApplyFailures >= maxConsecutiveApplyFailures { - return summary, fmt.Errorf("step %d apply: %d consecutive transient failures; the device is not recovering: %w", stepIndex, consecutiveApplyFailures, err) - } - logger.Warn("transient apply error; marking step transitional", "step", stepIndex, "err", err) - transitional = true - applySkipped = true - lastAction = nil - } else { + if ctx.Err() != nil { return summary, fmt.Errorf("step %d apply: %w", stepIndex, err) } + // Every apply error is a device-side condition (a dropped + // gesture, a typing request the runner's input handler choked + // on, an RPC deadline). None of them individually justify + // killing a fuzz run; what does is an unbroken streak, which + // means the device is wedged. The step is marked transitional + // so the verifier never sees a state the action did not reach. + consecutiveApplyFailures++ + if consecutiveApplyFailures >= maxConsecutiveApplyFailures { + return summary, fmt.Errorf("step %d apply: %d consecutive failures; the device is not recovering: %w", stepIndex, consecutiveApplyFailures, err) + } + logger.Warn("apply error; marking step transitional", "step", stepIndex, "err", err) + transitional = true + applySkipped = true + lastAction = nil } else { consecutiveApplyFailures = 0 actionCopy := nextAction @@ -933,28 +936,3 @@ func isWDADrop(err error) bool { return strings.Contains(err.Error(), "WDA reconnect failed") } -// isTransientApplyError reports whether an applyAction failure is a transient -// device-side hang (sidecar RPC deadline, momentary unavailability) rather than -// a fatal condition. Such steps are recorded as transitional and the loop -// continues. The run context being cancelled is never transient: it means the -// caller wants to stop. -func isTransientApplyError(runCtx context.Context, err error) bool { - if err == nil || runCtx.Err() != nil { - return false - } - if s, ok := status.FromError(err); ok { - switch s.Code() { - case codes.DeadlineExceeded, codes.Unavailable: - return true - case codes.Internal: - message := s.Message() - if strings.Contains(message, "DEADLINE_EXCEEDED") || strings.Contains(message, "UNAVAILABLE") { - return true - } - } - } - if errors.Is(err, context.DeadlineExceeded) { - return true - } - return false -} diff --git a/internal/runner/runner_test.go b/internal/runner/runner_test.go index e2a4884..c2977de 100644 --- a/internal/runner/runner_test.go +++ b/internal/runner/runner_test.go @@ -1321,8 +1321,8 @@ func TestRunner_TransientApplyErrorMarksTransitional(t *testing.T) { if len(summary.Violations) != 0 { t.Errorf("transient apply error must not surface as a violation, got %v", summary.Violations) } - if !strings.Contains(logBuf.String(), "transient apply error") { - t.Errorf("expected transient-apply WARN log, got %q", logBuf.String()) + if !strings.Contains(logBuf.String(), "apply error; marking step transitional") { + t.Errorf("expected apply-error WARN log, got %q", logBuf.String()) } type traceLine struct { @@ -1353,34 +1353,44 @@ func TestRunner_TransientApplyErrorMarksTransitional(t *testing.T) { } } -// TestIsTransientApplyError_Classification covers the helper's matching rules -// directly so future code changes don't quietly drop a transient case. -func TestIsTransientApplyError_Classification(t *testing.T) { - cleanCtx := context.Background() - cancelledCtx, cancel := context.WithCancel(context.Background()) - cancel() +// internalApplyErrorFailFirst wraps a mock driver so the first InputText call +// fails with the bare Internal error the iOS runner's input handler emits +// when it chokes (HTTP 500 with an empty body), then recovers. +type internalApplyErrorFailFirst struct { + *mockdriver.Driver + calls int +} - cases := []struct { - name string - ctx context.Context - err error - want bool - }{ - {"nil error", cleanCtx, nil, false}, - {"deadline exceeded", cleanCtx, status.Error(codes.DeadlineExceeded, "boom"), true}, - {"unavailable", cleanCtx, status.Error(codes.Unavailable, "boom"), true}, - {"internal wrapping deadline", cleanCtx, status.Error(codes.Internal, "io.grpc.StatusRuntimeException: DEADLINE_EXCEEDED: ..."), true}, - {"internal wrapping unavailable", cleanCtx, status.Error(codes.Internal, "io.grpc.StatusRuntimeException: UNAVAILABLE: ..."), true}, - {"internal generic", cleanCtx, status.Error(codes.Internal, "boom"), false}, - {"raw context deadline", cleanCtx, context.DeadlineExceeded, true}, - {"run context cancelled overrides", cancelledCtx, status.Error(codes.DeadlineExceeded, "boom"), false}, +func (d *internalApplyErrorFailFirst) TapSelector(ctx context.Context, selector string) error { + d.calls++ + if d.calls == 1 { + return status.Error(codes.Internal, "UnknownFailure(errorResponse=Request for inputText failed, code: 500, body: )") } - for _, testCase := range cases { - t.Run(testCase.name, func(t *testing.T) { - if got := isTransientApplyError(testCase.ctx, testCase.err); got != testCase.want { - t.Errorf("got %v, want %v", got, testCase.want) - } - }) + return d.Driver.TapSelector(ctx, selector) +} + +// TestRunner_InternalApplyErrorMarksTransitional pins the policy that a +// one-off device-side failure (e.g. the iOS input handler's bare 500) is +// absorbed as a transitional step instead of killing the run. Persistent +// failure is covered by the consecutive-failure cap. +func TestRunner_InternalApplyErrorMarksTransitional(t *testing.T) { + state := newHarness(t) + wrapped := &internalApplyErrorFailFirst{Driver: state.mock} + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + summary, err := Run(ctx, Options{ + Duration: 300 * time.Millisecond, + IdleTimeout: 20 * time.Millisecond, + Driver: wrapped, + Verifier: state.verifier, + TraceWriter: state.writer, + }) + if err != nil { + t.Fatalf("Run must not return on a one-off internal apply error, got %v", err) + } + if summary.Steps < 2 { + t.Fatalf("need at least 2 steps to prove the loop continued, got %d", summary.Steps) } } @@ -1412,9 +1422,6 @@ func TestIsWDADrop_Classification(t *testing.T) { if got := isWDADrop(testCase.err); got != testCase.want { t.Errorf("got %v, want %v", got, testCase.want) } - if testCase.want && isTransientApplyError(context.Background(), testCase.err) { - t.Error("a WDA drop must never also classify as transient") - } }) } } @@ -1449,7 +1456,7 @@ func TestRunner_ConsecutiveTransientApplyFailuresAbort(t *testing.T) { if err == nil { t.Fatal("Run must abort after consecutive transient apply failures") } - if !strings.Contains(err.Error(), "consecutive transient failures") { + if !strings.Contains(err.Error(), "consecutive failures") { t.Errorf("expected consecutive-failure abort, got %v", err) } // The aborting step returns before it is recorded, so the summary holds