ctx.Canceled silently swallowed in queryMajorEngineVersionsWithClient (surfaced by PR #1225) #1325

Background

PR #1225 (TEST-02 fix) adds TestQueryMajorEngineVersionsWithClient_EngineErrorContinues in cmd/multi_service_coverage_test.go with the comment "per-engine API failures are warn-and-continue" and require.NoError(t, err) for any per-engine error. That contract collapses transient throttling errors (correctly continue) and context cancellation / deadline expiry (should be terminal) into the same bucket.

The defect

cmd/multi_service_engine_versions.go:213-227:

for _, engine := range engines {
    if err := fetchMajorEngineVersionsForEngine(ctx, rdsClient, engine, versionInfo); err != nil {
        log.Printf("Warning: Failed to describe major engine versions for %s: %v", engine, err)
    }
}
return versionInfo, nil

fetchMajorEngineVersionsForEngine returns ctx.Err() on cancellation (line 236-237). Today that is swallowed as a warning and the loop continues to the next engine -- contradicting feedback_ctx_cancel_terminal.md: "treat context.Canceled/DeadlineExceeded as hard stops in API fan-out loops; never accumulate as lastErr and continue".

Concrete consequence: a CLI invocation that is interrupted (SIGINT, parent ctx cancelled) keeps calling DescribeDBMajorEngineVersions for the remaining engines, wastes API budget, and writes a "successful" version-info map back to the caller -- which then continues the orchestration as if nothing went wrong.

Fix

In queryMajorEngineVersionsWithClient:

for _, engine := range engines {
    if err := fetchMajorEngineVersionsForEngine(ctx, rdsClient, engine, versionInfo); err != nil {
        if errors.Is(err, context.Canceled) || errors.Is(err, context.DeadlineExceeded) {
            return nil, err
        }
        log.Printf("Warning: Failed to describe major engine versions for %s: %v", engine, err)
    }
}

Plus a paired test in cmd/multi_service_coverage_test.go:

func TestQueryMajorEngineVersionsWithClient_CtxCancelIsTerminal(t *testing.T) {
    stub := &engineKeyedRDSMajorVersionsStub{
        errByEngine: map[string]error{"mysql": context.Canceled},
    }
    ctx, cancel := context.WithCancel(context.Background())
    cancel()
    _, err := queryMajorEngineVersionsWithClient(ctx, stub)
    require.ErrorIs(t, err, context.Canceled)
    assert.Len(t, stub.enginesQueried, 1, "ctx cancellation must stop the loop after the first failing engine")
}

And update the comment on _EngineErrorContinues to say "transient per-engine API failures" -- making the contract narrower than "any error".

Same pattern, sibling occurrences worth a sweep

grep -rn "log.Printf.*Warning.*Failed" cmd/ pkg/ | grep -B2 -A2 "for.*range" -- audit other API fan-out loops in cmd/ for the same shape.

Triage

P2 / sev medium -- pre-existing bug, surfaced by PR #1225's new test. Not a money path itself but the function gates downstream filtering logic.