fix(client): classify request timeouts as timeout/exit 4 on all paths

README.md

@@ -166,7 +166,7 @@ Every command supports `--help`.
 |--------|----------|
 | **stdout** | Always JSON. Even `video download` — binary writes to disk; stdout emits `{"asset", "message", "path"}` so you can chain on `.path`. |
 | **stderr** | Structured envelope on error: `{"error": {"code", "message", "hint", "param", "doc_url", "request_id"}}`. `code`/`message` are always present; `hint`/`param`/`doc_url`/`request_id` appear when applicable (`param`/`doc_url` are surfaced from the API for validation and documented errors). Stable `code` values for programmatic branching. A code prefixed `cli_` is originated by the CLI itself (client/transport/local conditions, e.g. `cli_download_url_expired`). A bare code is either an API code (or a CLI mirror of one) or one of a small frozen set of legacy CLI codes that predate the prefix. The `cli_` prefix is reserved for the CLI, so a new CLI code can never collide with an API code. |
-| **Exit codes** | `0` ok · `1` API or network · `2` usage · `3` auth / not permitted · `4` timeout under `--wait` (stdout contains partial resource for resume) |
+| **Exit codes** | `0` ok · `1` API or network · `2` usage · `3` auth / not permitted · `4` timeout — a request exceeded its per-operation timeout, or the `--wait` poll window elapsed (stdout contains partial resource for resume) |
 | **Request bodies** | Flags for simple inputs; `-d` for nested JSON (inline, file path, or `-` for stdin). Flags override matching fields. |
 | **Async jobs** | `--wait` blocks with exponential backoff; `--timeout` sets max (default 20m). 429s and 5xx retry automatically. |
 

internal/client/executor.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"io"
 	"mime/multipart"
+	"net"
 	"net/http"
 	"net/url"
 	"os"
@@ -207,6 +208,38 @@ func (c *Client) executeWithContext(ctx context.Context, spec *command.Spec, inv
 				ExitCode: clierrors.ExitAuth,
 			}
 		}
+		// A transport-level timeout (the per-request HTTP deadline elapsed
+		// before the response arrived) is classified as timeout/exit 4 — the
+		// same code on both the plain and --wait paths, so a slow create or
+		// upload no longer masquerades as a generic network_error on one path
+		// and a timeout on the other. net.Error.Timeout() is the robust
+		// detector: http.Client.Timeout's wrapped error does not reliably
+		// satisfy errors.Is(context.DeadlineExceeded), which is why the old
+		// --wait path resorted to string-matching the error message.
+		var netErr net.Error
+		if (errors.As(err, &netErr) && netErr.Timeout()) || errors.Is(err, context.DeadlineExceeded) {
+			// A GET is safe to retry. A non-GET (create/write) may have been
+			// committed server-side before the client deadline elapsed, and CLI
+			// writes carry no idempotency key, so a blind retry can produce a
+			// duplicate (e.g. a second video create + charge). Steer writes to
+			// "check status first", mirroring the poll-deadline guidance in
+			// newCreateContextError. Include the applied budget so support can
+			// see which per-operation timeout tripped.
+			hint := "The request took too long and timed out. Retry — a slow endpoint or a stalled network connection can cause this."
+			if spec.Method != http.MethodGet {
+				hint = "The request timed out, but the operation may have been submitted. Check with the corresponding get/list command before retrying, to avoid creating a duplicate."
+			}
+			msg := "request timed out"
+			if c.httpClient.Timeout > 0 {
+				msg = fmt.Sprintf("request timed out after %s", c.httpClient.Timeout)
+			}
+			return nil, &clierrors.CLIError{
+				Code:     "timeout",
+				Message:  fmt.Sprintf("%s: %v", msg, err),
+				Hint:     hint,
+				ExitCode: clierrors.ExitTimeout,
+			}
+		}
 		// network_error is a grandfathered bare code (not cli_-prefixed): it is
 		// shared with the download-command transport path (cmd/heygen/video_download.go)
 		// and predates the cli_ convention. See the grandfathered set in
@@ -455,29 +488,20 @@ func extractJSONPath(raw json.RawMessage, path string) (string, error) {
 	return value, nil
 }
 
-// translateCreateContextError converts timeout/cancel errors before a resource
-// ID is known into user-friendly CLIErrors. Deadline exceeded returns exit 4
-// (timeout) since the operation may have been created server-side. Cancel
-// returns exit 1 (general) since the user explicitly interrupted.
+// translateCreateContextError maps a canceled/expired poll context to a
+// user-friendly CLIError when the create request fails before a resource ID is
+// known. A tripped poll deadline returns exit 4 (timeout, the operation may
+// exist server-side); an explicit cancel returns exit 1. A transport-level
+// request timeout that fires while the poll context is still live is already
+// classified as timeout/exit 4 by executeWithContext, so it passes through the
+// final return unchanged — no message string-matching required.
 func translateCreateContextError(ctx context.Context, err error) error {
 	if ctxErr := ctx.Err(); ctxErr != nil {
 		return newCreateContextError(ctxErr)
 	}
 	if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) {
 		return newCreateContextError(err)
 	}
-
-	var cliErr *clierrors.CLIError
-	if errors.As(err, &cliErr) && cliErr.Code == "network_error" {
-		msg := strings.ToLower(cliErr.Message)
-		switch {
-		case strings.Contains(msg, "context deadline exceeded"):
-			return newCreateContextError(context.DeadlineExceeded)
-		case strings.Contains(msg, "context canceled"), strings.Contains(msg, "context cancelled"):
-			return newCreateContextError(context.Canceled)
-		}
-	}
-
 	return err
 }
 

internal/client/executor_test.go

@@ -1026,3 +1026,66 @@ func TestExecute_ResponseBodyReadError_NetworkError(t *testing.T) {
 		t.Fatalf("err = %v, want a network_error CLIError", err)
 	}
 }
+
+func TestExecute_RequestTimeout_ClassifiedAsTimeout(t *testing.T) {
+	// Handler sleeps past the client timeout so the request deadline trips
+	// before any response arrives. The transport timeout must surface as a
+	// timeout CLIError (exit 4), not a generic network_error (exit 1).
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		time.Sleep(200 * time.Millisecond)
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write([]byte(`{"data":{}}`))
+	}))
+	defer srv.Close()
+
+	c := New("key", WithBaseURL(srv.URL), WithHTTPClient(srv.Client()), WithMaxRetries(0))
+	c.SetTimeout(50 * time.Millisecond)
+
+	spec := &command.Spec{Endpoint: "/v3/videos", Method: "POST", BodyEncoding: "json"}
+	inv := &command.Invocation{PathParams: make(map[string]string), Body: map[string]any{"x": 1}}
+
+	_, err := c.Execute(spec, inv)
+	var cliErr *clierrors.CLIError
+	if !errors.As(err, &cliErr) {
+		t.Fatalf("want *CLIError, got %v", err)
+	}
+	if cliErr.Code != "timeout" {
+		t.Errorf("code = %q, want %q", cliErr.Code, "timeout")
+	}
+	if cliErr.ExitCode != clierrors.ExitTimeout {
+		t.Errorf("exit code = %d, want %d (ExitTimeout)", cliErr.ExitCode, clierrors.ExitTimeout)
+	}
+	// A write (POST) must not advise a blind retry — the create may have been
+	// committed and CLI writes have no idempotency key.
+	if !strings.Contains(cliErr.Hint, "before retrying") {
+		t.Errorf("POST timeout hint should steer to check-status-first, got: %q", cliErr.Hint)
+	}
+	// The applied per-operation budget is surfaced for triage.
+	if !strings.Contains(cliErr.Message, "50ms") {
+		t.Errorf("timeout message should name the applied budget, got: %q", cliErr.Message)
+	}
+}
+
+func TestExecute_GETRequestTimeout_SafeToRetryHint(t *testing.T) {
+	// A read (GET) that times out is safe to retry, so its hint should not carry
+	// the create-oriented "check before retrying" guidance.
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		time.Sleep(200 * time.Millisecond)
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write([]byte(`{"data":{}}`))
+	}))
+	defer srv.Close()
+
+	c := New("key", WithBaseURL(srv.URL), WithHTTPClient(srv.Client()), WithMaxRetries(0))
+	c.SetTimeout(50 * time.Millisecond)
+
+	_, err := c.Execute(&command.Spec{Endpoint: "/v3/videos", Method: "GET"},
+		&command.Invocation{PathParams: make(map[string]string), QueryParams: url.Values{}})
+	var cliErr *clierrors.CLIError
+	if !errors.As(err, &cliErr) || cliErr.Code != "timeout" {
+		t.Fatalf("want timeout CLIError, got %v", err)
+	}
+	if strings.Contains(cliErr.Hint, "duplicate") {
+		t.Errorf("GET timeout hint should not carry create/duplicate guidance, got: %q", cliErr.Hint)
+	}
+}

internal/client/retry.go

@@ -6,6 +6,7 @@ import (
 	"io"
 	"math"
 	"math/rand/v2"
+	"net"
 	"net/http"
 	"os"
 	"strconv"
@@ -88,9 +89,18 @@ func (t *retryTransport) RoundTrip(req *http.Request) (*http.Response, error) {
 
 func shouldRetry(req *http.Request, resp *http.Response, err error) bool {
 	if err != nil {
+		// A tripped client timeout is not worth retrying: http.Client.Timeout is
+		// a total-Do budget, so the context is already spent. Match on
+		// net.Error.Timeout() as well as the context sentinels, because
+		// http.Client.Timeout's wrapped error does not reliably satisfy
+		// errors.Is(context.DeadlineExceeded) — the same reason the executor's
+		// classifier uses net.Error.Timeout(). Keeps both layers agreeing on
+		// what "timeout" means.
+		var netErr net.Error
 		if req.Context().Err() != nil ||
 			errors.Is(err, context.Canceled) ||
-			errors.Is(err, context.DeadlineExceeded) {
+			errors.Is(err, context.DeadlineExceeded) ||
+			(errors.As(err, &netErr) && netErr.Timeout()) {
 			return false
 		}
 		return isIdempotent(req.Method)