From 1af6c307008aedfdb6c8e21c289f3915711c7636 Mon Sep 17 00:00:00 2001 From: "Customer.io Open Source Bot" Date: Fri, 23 Oct 2026 12:22:22 -0400 Subject: [PATCH] Add Customer.io CLI source CioCliPublicExport-RevId: 38e3bc5da0ebb1d38e0a6968d7b2687c8044a329 --- README.md | 15 +- cmd/api.go | 10 +- cmd/api_test.go | 53 +++++ cmd/auth.go | 2 +- cmd/auth_test.go | 10 +- cmd/billing.go | 23 ++ cmd/billing_add_funds.go | 197 +++++++++++++++ cmd/billing_test.go | 304 ++++++++++++++++++++++++ cmd/root.go | 19 +- internal/client/client.go | 54 ++++- internal/client/client_test.go | 53 +++++ internal/client/retry.go | 10 +- internal/client/transport_retry_test.go | 246 +++++++++++++++++++ internal/validate/params.go | 33 ++- internal/validate/params_test.go | 39 +++ 15 files changed, 1046 insertions(+), 22 deletions(-) create mode 100644 cmd/billing.go create mode 100644 cmd/billing_add_funds.go create mode 100644 cmd/billing_test.go create mode 100644 internal/client/transport_retry_test.go diff --git a/README.md b/README.md index 5ec8333..e041c41 100644 --- a/README.md +++ b/README.md @@ -144,7 +144,7 @@ CIO_TOKEN=sa_live_xxx cio api /v1/environments/{environment_id}/campaigns --para Use `cio api ` for any API endpoint. Path placeholders are resolved from `--params`. The HTTP method defaults to GET (or POST if `--json` is provided); override with `-X`: -`--params` takes a JSON object; URL-query syntax (`--params 'type=event&size=200'`) is also accepted. In the query form a bare `+` is rejected as ambiguous — write `%20` for a space and `%2B` for a plus (`email=a%2Btag@b.com`), or use the JSON form, where both are literal. +`--params` takes a JSON object; URL-query syntax (`--params 'type=event&size=200'`) is also accepted. Repeat `--params` to split values across flags (`--params type=event --params size=200`); a key given more than once is an error. In the query form a bare `+` is rejected as ambiguous — write `%20` for a space and `%2B` for a plus (`email=a%2Btag@b.com`), or use the JSON form, where both are literal. ```bash # List campaigns in workspace 123 @@ -237,6 +237,19 @@ cio api /v1/environments/{environment_id}/campaigns \ | `--limit ` | | Page size | | `--page-all` | | Auto-paginate, emit NDJSON | +`cio api` retries timeouts for GET and HEAD, and for writes only when the request +was not sent. Writes whose response times out after sending are not retried because +they may already have applied. `--timeout` still bounds each HTTP attempt and +defaults to 30s. + +Timeouts share the existing retry limit and exponential backoff with HTTP 429 and +retryable 5xx responses. `CIO_MAX_RETRIES` defaults to 3 retries, or 4 total +attempts; set it to 0 to disable these retries. Caller cancellation or an expired +caller deadline stops retries. Repeated timeouts can therefore take longer than +`--timeout` overall. If `--page-all` exhausts its retries, it exits with an error +and leaves any earlier pages in stdout; callers must check its exit status before +using the output as a complete result. + ## Exit Codes | Code | Meaning | diff --git a/cmd/api.go b/cmd/api.go index 3eeacd8..7359e1c 100644 --- a/cmd/api.go +++ b/cmd/api.go @@ -86,7 +86,7 @@ func runAPI(cmd *cobra.Command, args []string) error { httpMethod := resolveMethod(methodFlag, jsonBody != nil || len(fileParts) > 0) // Parse --params: separate path params from query params. - paramsRaw, _ := cmd.Flags().GetString("params") + paramsRaw, _ := cmd.Flags().GetStringArray("params") pathParams, queryParams, err := parseAPIParams(pathTemplate, paramsRaw) if err != nil { output.PrintError(output.CodeValidationError, err.Error(), nil) @@ -377,20 +377,20 @@ func extractPathParamNames(pathTemplate string) map[string]bool { // parseAPIParams separates path template params from query params. // Path params are those matching {placeholder} in the path template. -// Input is validated through validate.ValidateParams (JSON object or +// Each --params value is validated and merged through validate.MergeParams (JSON object or // query-string sugar; keys must match [a-zA-Z0-9_]+ with an optional [] // suffix, values must not contain control characters, see // MaxParamValueLength). Path params are validated as safe URL path segments; // the API remains the source of truth for endpoint-specific ID semantics. -func parseAPIParams(pathTemplate, paramsJSON string) (pathParams, queryParams map[string]string, err error) { +func parseAPIParams(pathTemplate string, paramsRaw []string) (pathParams, queryParams map[string]string, err error) { pathParams = make(map[string]string) queryParams = make(map[string]string) - if strings.TrimSpace(paramsJSON) == "" { + if len(paramsRaw) == 0 { return pathParams, queryParams, nil } - params, err := validate.ValidateParams(paramsJSON) + params, err := validate.MergeParams(paramsRaw) if err != nil { return nil, nil, err } diff --git a/cmd/api_test.go b/cmd/api_test.go index 9273066..91f0644 100644 --- a/cmd/api_test.go +++ b/cmd/api_test.go @@ -193,6 +193,59 @@ func TestAPI_DryRun(t *testing.T) { } } +func TestAPI_RepeatedParamsMerge(t *testing.T) { + server, cleanup := setupAPITest(t) + defer cleanup() + + stdout, _, err := executeCommand("api", "/v1/environments/{environment_id}/campaigns", + "--api-url", server.URL, + "--params", "environment_id=123", + "--params", "action_id=87", + "--params", `{"size":500}`, + "--params", "end_ts=5", + "--dry-run") + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + + var result struct { + URL string `json:"url"` + Params map[string]string `json:"params"` + } + if err := json.Unmarshal([]byte(stdout), &result); err != nil { + t.Fatalf("invalid JSON: %v", err) + } + if result.URL != server.URL+"/v1/environments/123/campaigns" { + t.Errorf("expected resolved URL, got %v", result.URL) + } + want := map[string]string{"action_id": "87", "size": "500", "end_ts": "5"} + if len(result.Params) != len(want) { + t.Fatalf("expected params %v, got %v", want, result.Params) + } + for k, v := range want { + if result.Params[k] != v { + t.Errorf("param %q: want %q, got %q", k, v, result.Params[k]) + } + } +} + +func TestAPI_RepeatedParamsDuplicateKey(t *testing.T) { + server, cleanup := setupAPITest(t) + defer cleanup() + + _, _, err := executeCommand("api", "/v1/environments/{environment_id}/campaigns", + "--api-url", server.URL, + "--params", "environment_id=123&id=1", + "--params", "id=2", + "--dry-run") + if err == nil { + t.Fatal("expected error for a key repeated across --params flags") + } + if !strings.Contains(err.Error(), "appears more than once") { + t.Errorf("expected duplicate-key error, got: %v", err) + } +} + func TestAPI_MissingPathParam(t *testing.T) { server, cleanup := setupAPITest(t) defer cleanup() diff --git a/cmd/auth.go b/cmd/auth.go index a774e18..3212de2 100644 --- a/cmd/auth.go +++ b/cmd/auth.go @@ -631,7 +631,7 @@ func runLoginCLILink(cmd *cobra.Command, saToken string) error { } timeout, _ := cmd.Flags().GetDuration("timeout") - resp, err := client.MintLoginCLILink(cmd.Context(), baseURL, saToken, timeout) + resp, err := client.MintLoginCLILink(cmd.Context(), baseURL, saToken, "", timeout) if err != nil { return handleAPIError(err) } diff --git a/cmd/auth_test.go b/cmd/auth_test.go index faef651..9fee281 100644 --- a/cmd/auth_test.go +++ b/cmd/auth_test.go @@ -29,7 +29,11 @@ func executeCommand(args ...string) (stdout, stderr string, err error) { // Reset persistent flags before each test. _ = rootCmd.PersistentFlags().Set("jq", "") - _ = rootCmd.PersistentFlags().Set("params", "") + _ = rootCmd.PersistentFlags().Set("raw-output", "false") + _ = rootCmd.PersistentFlags().Set("read-only", "false") + if f, ok := rootCmd.PersistentFlags().Lookup("params").Value.(pflag.SliceValue); ok { + _ = f.Replace(nil) + } _ = rootCmd.PersistentFlags().Set("json", "") _ = rootCmd.PersistentFlags().Set("dry-run", "false") _ = rootCmd.PersistentFlags().Set("api-url", "") @@ -489,6 +493,10 @@ func TestAuthLogin_StoredTokenTriggersWebHandoff(t *testing.T) { if got := r.Header.Get("Authorization"); got != "Bearer sa_live_existing" { t.Errorf("expected Bearer auth with stored token, got %q", got) } + // No destination, so the link keeps landing on the dashboard home. + if body, _ := io.ReadAll(r.Body); string(body) != "{}" { + t.Errorf("expected an empty JSON body, got %q", body) + } _, _ = w.Write([]byte(`{"handoff_token":"handoff-jwt-abc","expires_in":60}`)) return } diff --git a/cmd/billing.go b/cmd/billing.go new file mode 100644 index 0000000..ea83bf6 --- /dev/null +++ b/cmd/billing.go @@ -0,0 +1,23 @@ +package cmd + +import ( + "github.com/spf13/cobra" +) + +var billingCmd = &cobra.Command{ + Use: "billing", + Short: "Add funds to a Flex account", + // Hidden until the Flex plan launches. The commands still run. + Hidden: true, + Long: `Billing commands for Flex accounts, which pay for usage from a prepaid balance. + +Examples: + cio billing add-funds`, + RunE: func(cmd *cobra.Command, args []string) error { + return cmd.Help() + }, +} + +func init() { + rootCmd.AddCommand(billingCmd) +} diff --git a/cmd/billing_add_funds.go b/cmd/billing_add_funds.go new file mode 100644 index 0000000..6f25eea --- /dev/null +++ b/cmd/billing_add_funds.go @@ -0,0 +1,197 @@ +package cmd + +import ( + "encoding/json" + "errors" + "fmt" + "net/http" + "net/url" + "strings" + + "github.com/customerio/cli/internal/client" + "github.com/customerio/cli/internal/output" + "github.com/spf13/cobra" +) + +var billingAddFundsCmd = &cobra.Command{ + Use: "add-funds", + Short: "Print a sign-in link that opens Add funds", + Long: `Print a one-time link that signs the browser in to Customer.io and opens +Add funds on the billing page. The CLI never charges anything: the person +picks an amount and pays on that page. + +The link is valid for 60 seconds and works once. If it expires, run the +command again for a fresh one. + +Flex accounts only. The person who created the token needs permission to +manage billing. + +An account that requires SSO can't use sign-in links, so the command prints +the billing page URL instead, with sign_in_required set. The person opens it +and signs in with SSO there. + +Output: + {"status":"ok","url":"https://fly.customer.io/cli?token=...","expires_in":60} + {"status":"ok","url":"https://fly.customer.io/settings/billing/...","sign_in_required":true} + +Examples: + cio billing add-funds + cio billing add-funds --jq .url -r`, + Args: cobra.NoArgs, + RunE: runBillingAddFunds, +} + +func init() { + billingCmd.AddCommand(billingAddFundsCmd) +} + +// flexBillingVendor is the billing_vendor the account read reports for Flex. +const flexBillingVendor = "modern_stripe" + +// addFundsPagePath is the billing page with Add funds open, where an add_funds +// sign-in link lands. from=cli reports the purchase as one started from the CLI. +const addFundsPagePath = "/settings/billing/your-plan?purchaseModal=account-funds&from=cli" + +func runBillingAddFunds(cmd *cobra.Command, args []string) error { + c := clientFromCmd(cmd) + if c == nil { + return errNoClient(cmd) + } + + // The link is minted outside the client's request path, which is where + // --read-only is otherwise enforced, and the session it opens can spend. + if c.ReadOnly() { + err := fmt.Errorf("read-only mode: add-funds creates a sign-in link (use without --read-only to allow it)") + output.PrintError(output.CodeValidationError, err.Error(), nil) + return err + } + + // Only a service account token can mint a link, and a pre-exchanged access + // token may belong to a different account than the one it would mint for. + if client.ResolveAccessToken() != "" { + err := fmt.Errorf("add-funds can't run with CIO_ACCESS_TOKEN set: the sign-in link needs a service account token") + output.PrintError(output.CodeAuthError, err.Error(), map[string]any{ + "hint": "Unset CIO_ACCESS_TOKEN and sign in with 'cio auth login', or set CIO_TOKEN to a service account token.", + }) + return err + } + saToken := c.ServiceAccountToken() + if saToken == "" { + err := fmt.Errorf("not authenticated. Run 'cio auth login' or set CIO_TOKEN") + output.PrintError(output.CodeAuthError, err.Error(), nil) + return err + } + + raw, err := c.Do(cmd.Context(), "GET", "/v1/accounts/current", nil, nil) + if err != nil { + return handleAPIError(err) + } + var resp struct { + Account struct { + BillingVendor string `json:"billing_vendor"` + SSOSetting string `json:"sso_setting"` + } `json:"account"` + } + if err := json.Unmarshal(raw, &resp); err != nil { + output.PrintError(output.CodeGeneralError, "failed to parse account response", nil) + return err + } + + if resp.Account.BillingVendor != flexBillingVendor { + err := fmt.Errorf("add funds is only available on the Flex plan") + output.PrintError(output.CodeValidationError, err.Error(), map[string]any{ + "hint": "This account isn't on Flex, so it has no balance to add funds to. Its plan is under Settings → Billing in Customer.io.", + }) + return err + } + + // The sign-in link refuses an account that requires SSO, so send the + // person to the page itself to sign in there. + if resp.Account.SSOSetting == "required" { + pageURL := resolveUIBase(c.BaseURL()) + addFundsPagePath + if GetDryRun(cmd) { + return output.FprintJSON(cmd.OutOrStdout(), map[string]any{ + "dry_run": true, + "url": pageURL, + "sign_in_required": true, + }) + } + fmt.Fprintf(cmd.ErrOrStderr(), "This account signs in with SSO, so the CLI can't sign you in. Open this URL in your browser and sign in with SSO to add funds:\n\n %s\n\n", pageURL) + return printAddFundsResult(cmd, map[string]any{ + "status": "ok", + "message": "Open the URL in your browser and sign in with SSO to add funds.", + "url": pageURL, + "sign_in_required": true, + }) + } + + body := map[string]string{"destination": client.LoginCLILinkDestinationAddFunds} + if GetDryRun(cmd) { + return output.FprintJSON(cmd.OutOrStdout(), map[string]any{ + "dry_run": true, + "method": "POST", + "url": c.BaseURL() + "/v1/login_cli/link", + "headers": map[string]string{ + "Authorization": "Bearer [REDACTED]", + "Content-Type": "application/json", + }, + "body": body, + }) + } + + timeout, _ := cmd.Flags().GetDuration("timeout") + link, err := client.MintLoginCLILink(cmd.Context(), c.BaseURL(), saToken, client.LoginCLILinkDestinationAddFunds, timeout) + if err != nil { + return handleAddFundsError(err) + } + + linkURL := resolveCLILoginURL(c.BaseURL()) + "?token=" + url.QueryEscape(link.HandoffToken) + + fmt.Fprintf(cmd.ErrOrStderr(), "Open this URL in your browser to add funds to your Customer.io account:\n\n %s\n\n", linkURL) + fmt.Fprintf(cmd.ErrOrStderr(), "This link is valid for %d seconds and works once. If it expires, run 'cio billing add-funds' again.\n", link.ExpiresIn) + + return printAddFundsResult(cmd, map[string]any{ + "status": "ok", + "message": "Open the URL in your browser to add funds.", + "url": linkURL, + "expires_in": link.ExpiresIn, + }) +} + +func printAddFundsResult(cmd *cobra.Command, result map[string]any) error { + data, _ := json.Marshal(result) + return output.FprintProcess(cmd.OutOrStdout(), json.RawMessage(data), GetJQFlag(cmd), GetRawFlag(cmd)) +} + +// handleAddFundsError passes on the server's reason for refusing a link, which +// says what plan and permission adding funds needs, where handleAPIError +// would reduce a 403 to "Insufficient permissions." +func handleAddFundsError(err error) error { + var apiErr *client.APIError + if errors.As(err, &apiErr) && (apiErr.StatusCode == http.StatusBadRequest || apiErr.StatusCode == http.StatusForbidden) { + if detail := apiErrorDetail(apiErr.Body); detail != "" { + code := output.CodeValidationError + if apiErr.StatusCode == http.StatusForbidden { + code = output.CodeAuthzError + } + output.PrintError(code, detail, map[string]any{ + "status_code": apiErr.StatusCode, + }) + return errors.New(detail) + } + } + return handleAPIError(err) +} + +// apiErrorDetail reads the first message from an {"errors":[{"detail":...}]} body. +func apiErrorDetail(body []byte) string { + var parsed struct { + Errors []struct { + Detail string `json:"detail"` + } `json:"errors"` + } + if json.Unmarshal(body, &parsed) != nil || len(parsed.Errors) == 0 { + return "" + } + return strings.TrimSpace(parsed.Errors[0].Detail) +} diff --git a/cmd/billing_test.go b/cmd/billing_test.go new file mode 100644 index 0000000..e24f7db --- /dev/null +++ b/cmd/billing_test.go @@ -0,0 +1,304 @@ +package cmd + +import ( + "encoding/json" + "io" + "net/http" + "net/http/httptest" + "strings" + "testing" +) + +const flexAccount = `{"id":1,"billing_vendor":"modern_stripe","sso_setting":"off"}` + +// addFundsServer answers the token exchange, the account read and the link +// mint, and records every request so a test can show nothing was minted. +type addFundsServer struct { + *httptest.Server + requests int + mints int + mintAuth string + mintBody string + // commands maps each requested path to the X-CIO-Command header it carried. + commands map[string]string +} + +func newAddFundsServer(t *testing.T, account string, mintStatus int, mintResponse string) *addFundsServer { + t.Helper() + s := &addFundsServer{commands: map[string]string{}} + s.Server = httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + s.requests++ + s.commands[r.URL.Path] = r.Header.Get("X-CIO-Command") + switch r.URL.Path { + case "/v1/service_accounts/oauth/token": + _, _ = w.Write([]byte(`{"access_token":"jwt-test-session","token_type":"Bearer","expires_in":3600}`)) + case "/v1/accounts/current": + if r.Header.Get("Authorization") != "Bearer jwt-test-session" { + w.WriteHeader(http.StatusUnauthorized) + return + } + _, _ = w.Write([]byte(`{"account":` + account + `}`)) + case "/v1/login_cli/link": + s.mints++ + s.mintAuth = r.Header.Get("Authorization") + body, _ := io.ReadAll(r.Body) + s.mintBody = string(body) + w.WriteHeader(mintStatus) + _, _ = w.Write([]byte(mintResponse)) + default: + t.Errorf("unexpected request to %s", r.URL.Path) + w.WriteHeader(http.StatusNotFound) + } + })) + t.Cleanup(s.Close) + return s +} + +func setupAddFunds(t *testing.T) { + t.Helper() + t.Setenv("HOME", t.TempDir()) + t.Setenv("CIO_TOKEN", "sa_live_flex") + t.Setenv("CIO_ACCESS_TOKEN", "") + t.Setenv("CIO_API_URL", "") + t.Setenv("CIO_UI_URL", "https://fly.example.test") +} + +func TestBillingAddFunds_PrintsAddFundsLink(t *testing.T) { + setupAddFunds(t) + server := newAddFundsServer(t, flexAccount, http.StatusOK, `{"handoff_token":"handoff-abc","expires_in":60}`) + + stdout, stderr, err := executeCommand("billing", "add-funds", "--api-url", server.URL) + if err != nil { + t.Fatalf("unexpected error: %v\nstderr: %s", err, stderr) + } + + if server.mints != 1 { + t.Fatalf("expected 1 mint request, got %d", server.mints) + } + if server.mintAuth != "Bearer sa_live_flex" { + t.Errorf("expected the service account token as the Bearer, got %q", server.mintAuth) + } + var body map[string]any + if err := json.Unmarshal([]byte(server.mintBody), &body); err != nil { + t.Fatalf("mint body is not JSON: %v (%q)", err, server.mintBody) + } + if body["destination"] != "add_funds" { + t.Errorf("expected destination add_funds, got %v", body["destination"]) + } + + var result map[string]any + if err := json.Unmarshal([]byte(stdout), &result); err != nil { + t.Fatalf("invalid JSON output: %v\nstdout: %s", err, stdout) + } + wantURL := "https://fly.example.test/cli?token=handoff-abc" + if result["url"] != wantURL { + t.Errorf("expected url %q, got %v", wantURL, result["url"]) + } + if result["status"] != "ok" || result["expires_in"] != float64(60) { + t.Errorf("expected status ok and expires_in 60, got %v", result) + } + if _, ok := result["sign_in_required"]; ok { + t.Errorf("a sign-in link should not report sign_in_required, got %v", result) + } + if !strings.Contains(stderr, wantURL) { + t.Errorf("expected stderr to print the URL, got: %s", stderr) + } +} + +func TestBillingAddFunds_JQReadsTheURL(t *testing.T) { + setupAddFunds(t) + server := newAddFundsServer(t, flexAccount, http.StatusOK, `{"handoff_token":"handoff-abc","expires_in":60}`) + + stdout, stderr, err := executeCommand("billing", "add-funds", "--api-url", server.URL, "--jq", ".url", "-r") + if err != nil { + t.Fatalf("unexpected error: %v\nstderr: %s", err, stderr) + } + if want := "https://fly.example.test/cli?token=handoff-abc\n"; stdout != want { + t.Errorf("expected stdout %q, got %q", want, stdout) + } +} + +func TestBillingAddFunds_NamesTheCommandOnEveryRequest(t *testing.T) { + setupAddFunds(t) + server := newAddFundsServer(t, flexAccount, http.StatusOK, `{"handoff_token":"handoff-abc","expires_in":60}`) + + // Flag values must stay off the header: only the command name is sent. + if _, stderr, err := executeCommand("billing", "add-funds", "--api-url", server.URL, "--jq", ".url", "-r"); err != nil { + t.Fatalf("unexpected error: %v\nstderr: %s", err, stderr) + } + + for _, path := range []string{"/v1/service_accounts/oauth/token", "/v1/accounts/current", "/v1/login_cli/link"} { + got, ok := server.commands[path] + if !ok { + t.Errorf("expected a request to %s", path) + continue + } + if got != "billing add-funds" { + t.Errorf("X-CIO-Command on %s: got %q, want %q", path, got, "billing add-funds") + } + } +} + +func TestBillingAddFunds_RefusesOtherPlans(t *testing.T) { + cases := map[string]string{ + "another vendor": `{"id":1,"billing_vendor":"ordway","sso_setting":"off"}`, + "no vendor": `{"id":1,"sso_setting":"off"}`, + "another vendor and SSO": `{"id":1,"billing_vendor":"ordway","sso_setting":"required"}`, + } + for name, account := range cases { + t.Run(name, func(t *testing.T) { + setupAddFunds(t) + server := newAddFundsServer(t, account, http.StatusOK, `{"handoff_token":"handoff-abc","expires_in":60}`) + + stdout, _, err := executeCommand("billing", "add-funds", "--api-url", server.URL) + if err == nil { + t.Fatalf("expected an error, got stdout: %s", stdout) + } + if !strings.Contains(err.Error(), "Flex") { + t.Errorf("expected the error to name the Flex plan, got %q", err) + } + if server.mints != 0 { + t.Errorf("expected no mint request, got %d", server.mints) + } + if stdout != "" { + t.Errorf("expected no stdout, got %q", stdout) + } + }) + } +} + +func TestBillingAddFunds_SSOPrintsTheBillingPage(t *testing.T) { + setupAddFunds(t) + server := newAddFundsServer(t, `{"id":1,"billing_vendor":"modern_stripe","sso_setting":"required"}`, http.StatusOK, `{"handoff_token":"handoff-abc","expires_in":60}`) + + stdout, stderr, err := executeCommand("billing", "add-funds", "--api-url", server.URL) + if err != nil { + t.Fatalf("unexpected error: %v\nstderr: %s", err, stderr) + } + if server.mints != 0 { + t.Errorf("expected no mint request for an SSO account, got %d", server.mints) + } + + var result map[string]any + if err := json.Unmarshal([]byte(stdout), &result); err != nil { + t.Fatalf("invalid JSON output: %v\nstdout: %s", err, stdout) + } + wantURL := "https://fly.example.test/settings/billing/your-plan?purchaseModal=account-funds&from=cli" + if result["url"] != wantURL { + t.Errorf("expected url %q, got %v", wantURL, result["url"]) + } + if result["sign_in_required"] != true { + t.Errorf("expected sign_in_required true, got %v", result) + } + if _, ok := result["expires_in"]; ok { + t.Errorf("the billing page URL does not expire, got %v", result) + } + if !strings.Contains(stderr, "SSO") { + t.Errorf("expected stderr to explain SSO, got: %s", stderr) + } +} + +func TestBillingAddFunds_DryRunMintsNothing(t *testing.T) { + setupAddFunds(t) + server := newAddFundsServer(t, flexAccount, http.StatusOK, `{"handoff_token":"handoff-abc","expires_in":60}`) + + stdout, stderr, err := executeCommand("billing", "add-funds", "--api-url", server.URL, "--dry-run") + if err != nil { + t.Fatalf("unexpected error: %v\nstderr: %s", err, stderr) + } + if server.mints != 0 { + t.Errorf("expected no mint request on a dry run, got %d", server.mints) + } + if strings.Contains(stdout, "sa_live_flex") { + t.Errorf("dry run must not print the token, got: %s", stdout) + } + + var result struct { + DryRun bool `json:"dry_run"` + Method string `json:"method"` + URL string `json:"url"` + Headers map[string]string `json:"headers"` + Body map[string]string `json:"body"` + } + if err := json.Unmarshal([]byte(stdout), &result); err != nil { + t.Fatalf("invalid JSON output: %v\nstdout: %s", err, stdout) + } + if !result.DryRun || result.Method != "POST" || result.URL != server.URL+"/v1/login_cli/link" { + t.Errorf("expected a POST to /v1/login_cli/link, got %+v", result) + } + if result.Headers["Authorization"] != "Bearer [REDACTED]" { + t.Errorf("expected a redacted Authorization header, got %q", result.Headers["Authorization"]) + } + if result.Body["destination"] != "add_funds" { + t.Errorf("expected destination add_funds, got %v", result.Body) + } +} + +func TestBillingAddFunds_ReadOnlyMintsNothing(t *testing.T) { + setupAddFunds(t) + server := newAddFundsServer(t, flexAccount, http.StatusOK, `{"handoff_token":"handoff-abc","expires_in":60}`) + + _, _, err := executeCommand("billing", "add-funds", "--api-url", server.URL, "--read-only") + if err == nil || !strings.Contains(err.Error(), "read-only") { + t.Fatalf("expected a read-only error, got %v", err) + } + if server.requests != 0 { + t.Errorf("expected no requests under --read-only, got %d", server.requests) + } +} + +func TestBillingAddFunds_RefusesAnAccessToken(t *testing.T) { + setupAddFunds(t) + t.Setenv("CIO_ACCESS_TOKEN", "jwt-from-elsewhere") + server := newAddFundsServer(t, flexAccount, http.StatusOK, `{"handoff_token":"handoff-abc","expires_in":60}`) + + _, _, err := executeCommand("billing", "add-funds", "--api-url", server.URL) + if err == nil || !strings.Contains(err.Error(), "CIO_ACCESS_TOKEN") { + t.Fatalf("expected an error naming CIO_ACCESS_TOKEN, got %v", err) + } + if server.requests != 0 { + t.Errorf("expected no requests with an access token, got %d", server.requests) + } +} + +func TestBillingAddFunds_RequiresAToken(t *testing.T) { + setupAddFunds(t) + t.Setenv("CIO_TOKEN", "") + server := newAddFundsServer(t, flexAccount, http.StatusOK, `{"handoff_token":"handoff-abc","expires_in":60}`) + + _, _, err := executeCommand("billing", "add-funds", "--api-url", server.URL) + if err == nil || !strings.Contains(err.Error(), "not authenticated") { + t.Fatalf("expected a not-authenticated error, got %v", err) + } + if server.requests != 0 { + t.Errorf("expected no requests without a token, got %d", server.requests) + } +} + +func TestBillingAddFunds_PassesOnTheServerRefusal(t *testing.T) { + cases := map[string]struct { + status int + detail string + }{ + "no billing permission": {http.StatusForbidden, "Adding funds needs permission to manage billing on this account. Ask an account admin to add funds or grant that permission."}, + "not on Flex": {http.StatusBadRequest, "Add funds is only available on the Flex plan."}, + } + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + setupAddFunds(t) + refusal, _ := json.Marshal(map[string]any{"errors": []map[string]any{{"detail": tc.detail, "status": tc.status}}}) + server := newAddFundsServer(t, flexAccount, tc.status, string(refusal)) + + stdout, _, err := executeCommand("billing", "add-funds", "--api-url", server.URL) + if err == nil { + t.Fatalf("expected an error, got stdout: %s", stdout) + } + if err.Error() != tc.detail { + t.Errorf("expected the server's message %q, got %q", tc.detail, err) + } + if stdout != "" { + t.Errorf("expected no stdout, got %q", stdout) + } + }) + } +} diff --git a/cmd/root.go b/cmd/root.go index a8993e5..8207d86 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -66,7 +66,7 @@ func init() { flags := rootCmd.PersistentFlags() flags.String("json", "", "Raw JSON request body, @filename to read from a file, or - to read from stdin") - flags.String("params", "", `Query parameters as a JSON object, e.g. '{"email":"a@b.com"}' (query-string form 'email=a@b.com' also accepted; encode spaces as %20 and a plus as %2B)`) + flags.StringArray("params", nil, `Query parameters as a JSON object, e.g. '{"email":"a@b.com"}' (query-string form 'email=a@b.com' also accepted; encode spaces as %20 and a plus as %2B). Repeatable; each key may appear only once`) flags.String("jq", "", "jq expression filter (via gojq)") flags.BoolP("raw-output", "r", false, "Print string results unquoted, like jq -r (no external jq needed)") flags.StringArray("arg", nil, "Bind a string variable for --json's jq program: --arg name=value, or name=@file to read the value from a file (repeatable). Makes --json a jq -n program — no external jq needed") @@ -83,6 +83,12 @@ func init() { flags.Bool("page-all", false, "Auto-paginate, emit NDJSON") rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { + // Record the command for the X-CIO-Command header before anything can + // send a request. CommandPath is built from command names alone, so + // arguments and flag values stay out of the header. Dropping the root + // name leaves bare `cio` with an empty command. + client.SetCommand(strings.TrimPrefix(cmd.CommandPath(), cmd.Root().Name())) + maybeHintSkillsInstall(cmd) // Select the active profile before any credential access so that auth @@ -107,9 +113,8 @@ func init() { } // Validate --params if provided. - paramsFlag, _ := cmd.Flags().GetString("params") - if paramsFlag != "" { - if _, err := validate.ValidateParams(paramsFlag); err != nil { + if paramsFlags, _ := cmd.Flags().GetStringArray("params"); len(paramsFlags) > 0 { + if _, err := validate.MergeParams(paramsFlags); err != nil { output.PrintError(output.CodeValidationError, err.Error(), map[string]string{ "flag": "--params", }) @@ -284,11 +289,11 @@ func GetDryRun(cmd *cobra.Command) bool { // GetParams parses and returns validated --params. func GetParams(cmd *cobra.Command) (map[string]string, error) { - raw, _ := cmd.Flags().GetString("params") - if raw == "" { + raws, _ := cmd.Flags().GetStringArray("params") + if len(raws) == 0 { return nil, nil } - return validate.ValidateParams(raw) + return validate.MergeParams(raws) } // GetJSONBody returns the validated --json payload. diff --git a/internal/client/client.go b/internal/client/client.go index 076b57b..5044534 100644 --- a/internal/client/client.go +++ b/internal/client/client.go @@ -12,12 +12,33 @@ import ( "net/url" "os" "strings" + "sync" "sync/atomic" "time" "github.com/customerio/cli/internal/useragent" ) +var ( + commandMu sync.RWMutex + command string +) + +// SetCommand records the path of the command being run, without the root name +// (for example "billing status"), for the X-CIO-Command header. Called once at +// the start of the root PersistentPreRunE, before any request is sent. +func SetCommand(path string) { + commandMu.Lock() + command = strings.TrimSpace(path) + commandMu.Unlock() +} + +func currentCommand() string { + commandMu.RLock() + defer commandMu.RUnlock() + return command +} + // setStandardHeaders stamps headers that every outgoing CLI request should // carry: // @@ -32,6 +53,11 @@ import ( // - X-CIO-Source — identifies the originating tool for server-side analytics // (e.g. the "Design Studio: Email Created" source). "AI Assistant" when run // by the agent sandbox (CIO_AGENT=1), otherwise "CLI". +// - X-CIO-Command — the command that sent the request (e.g. "billing status"), +// so API logs can tell CLI commands apart when they call the same +// endpoints. The value is the command name alone, so arguments and flag +// values stay out of it. Omitted when no command has been recorded with +// SetCommand. // - X-CIO-Capability-Grant — forwarded from $X_CIO_CAPABILITY_GRANT so the env carries a session-scoped grant without a CLI flag. func setStandardHeaders(req *http.Request) { req.Header.Set("User-Agent", useragent.Get()) @@ -42,6 +68,9 @@ func setStandardHeaders(req *http.Request) { } else { req.Header.Set("X-CIO-Source", "CLI") } + if cmd := currentCommand(); cmd != "" { + req.Header.Set("X-CIO-Command", cmd) + } if grant := os.Getenv("X_CIO_CAPABILITY_GRANT"); grant != "" { req.Header.Set("X-CIO-Capability-Grant", grant) } @@ -476,9 +505,17 @@ func (c *Client) DoWithBody(ctx context.Context, method, path string, params map if err == nil { return result, nil } + if ctx.Err() != nil { + // Preserve send tracking so cancelled writes still report an unknown outcome. + return nil, err + } apiErr, ok := err.(*APIError) if !ok { + if isRetryableTimeout(method, err) { + lastErr = err + continue + } return nil, err } @@ -528,7 +565,7 @@ func sendTracked(httpClient *http.Client, req *http.Request) (*http.Response, er if sent.Load() { return nil, fmt.Errorf("http request: %w: %w", errResponseLost, err) } - return nil, fmt.Errorf("http request: %w", err) + return nil, fmt.Errorf("http request: %w: %w", errRequestNotSent, err) } return resp, nil } @@ -697,18 +734,29 @@ type LoginCLILinkResponse struct { ExpiresIn int `json:"expires_in"` } +// LoginCLILinkDestinationAddFunds lands the signed-in browser on Add funds. +const LoginCLILinkDestinationAddFunds = "add_funds" + // MintLoginCLILink asks the backend for a short-lived JWT that the user can // click to bootstrap a browser session for fly.customer.io. The CLI's // existing sa_live_ token is the credential — we send it as a Bearer token, // not in the URL. -func MintLoginCLILink(ctx context.Context, baseURL, saToken string, timeout time.Duration) (*LoginCLILinkResponse, error) { +// +// destination names where the browser lands once signed in; "" is the +// dashboard home. The server maps the name to a path, so it is never a URL. +func MintLoginCLILink(ctx context.Context, baseURL, saToken, destination string, timeout time.Duration) (*LoginCLILinkResponse, error) { if timeout <= 0 { timeout = defaultTimeout } httpClient := &http.Client{Timeout: timeout} + body := []byte("{}") + if destination != "" { + body, _ = json.Marshal(map[string]string{"destination": destination}) + } + u := strings.TrimRight(baseURL, "/") + "/v1/login_cli/link" - req, err := http.NewRequestWithContext(ctx, http.MethodPost, u, bytes.NewReader([]byte("{}"))) + req, err := http.NewRequestWithContext(ctx, http.MethodPost, u, bytes.NewReader(body)) if err != nil { return nil, fmt.Errorf("create request: %w", err) } diff --git a/internal/client/client_test.go b/internal/client/client_test.go index ab8f8a6..b386bce 100644 --- a/internal/client/client_test.go +++ b/internal/client/client_test.go @@ -759,11 +759,16 @@ func TestClient_Do_UserAgentHeader(t *testing.T) { } func TestDoTrack_StandardHeaders(t *testing.T) { + SetCommand("send email") + t.Cleanup(func() { SetCommand("") }) + var gotUserAgent string var gotValidate string + var gotCommand string server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { gotUserAgent = r.Header.Get("User-Agent") gotValidate = r.Header.Get("X-Validate") + gotCommand = r.Header.Get("X-CIO-Command") _, _ = w.Write([]byte(`{"delivery_id":"abc"}`)) })) defer server.Close() @@ -784,6 +789,9 @@ func TestDoTrack_StandardHeaders(t *testing.T) { if gotValidate != "strict" { t.Errorf("X-Validate: got %q, want %q", gotValidate, "strict") } + if gotCommand != "send email" { + t.Errorf("X-CIO-Command: got %q, want %q", gotCommand, "send email") + } } func TestClient_Do_AgentHeader(t *testing.T) { @@ -836,6 +844,51 @@ func TestClient_Do_AgentHeader(t *testing.T) { } } +func TestClient_Do_CommandHeader(t *testing.T) { + cases := []struct { + name string + command string + want string // expected X-CIO-Command header value; "" means absent + }{ + {"no command recorded", "", ""}, + {"top-level command", "api", "api"}, + {"nested command", "billing status", "billing status"}, + {"surrounding space trimmed", " billing add-funds ", "billing add-funds"}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + SetCommand(tc.command) + t.Cleanup(func() { SetCommand("") }) + + var got []string + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + got = r.Header.Values("X-CIO-Command") + _, _ = w.Write([]byte(`{"ok":true}`)) + })) + defer server.Close() + + c := New(Config{ + BaseURL: server.URL, + AccessToken: "test-jwt", + RetryConfig: &RetryConfig{MaxRetries: 0, SleepFn: ContextSleep}, + }) + if _, err := c.Do(context.Background(), "GET", "/test", nil, nil); err != nil { + t.Fatalf("unexpected error: %v", err) + } + if tc.want == "" { + if len(got) != 0 { + t.Errorf("X-CIO-Command: expected no header, got %q", got) + } + return + } + if len(got) != 1 || got[0] != tc.want { + t.Errorf("X-CIO-Command: got %q, want %q", got, tc.want) + } + }) + } +} + func TestClient_Do_CapabilityGrantHeader(t *testing.T) { cases := []struct { name string diff --git a/internal/client/retry.go b/internal/client/retry.go index b2e0d93..829a2ab 100644 --- a/internal/client/retry.go +++ b/internal/client/retry.go @@ -5,6 +5,7 @@ import ( "errors" "math" "math/rand/v2" + "net" "net/http" "os" "strconv" @@ -44,7 +45,8 @@ func DefaultRetryConfig() RetryConfig { } var ( - errTokenExchange = errors.New("token exchange failed") + errTokenExchange = errors.New("token exchange failed") + errRequestNotSent = errors.New("request not sent") // errResponseLost marks a request that left the machine but whose response // never fully arrived. errResponseLost = errors.New("response lost") @@ -64,6 +66,12 @@ func IsRetryable(method string, statusCode int) bool { return IsReadOnlyMethod(method) && statusCode >= http.StatusInternalServerError } +func isRetryableTimeout(method string, err error) bool { + var netErr net.Error + return errors.As(err, &netErr) && netErr.Timeout() && + (IsReadOnlyMethod(method) || errors.Is(err, errRequestNotSent)) +} + // MayHaveApplied reports whether the server may have carried out a request // that failed: it answered 5xx, or its response never fully arrived. func MayHaveApplied(err error) bool { diff --git a/internal/client/transport_retry_test.go b/internal/client/transport_retry_test.go new file mode 100644 index 0000000..4dbf7b0 --- /dev/null +++ b/internal/client/transport_retry_test.go @@ -0,0 +1,246 @@ +package client + +import ( + "bytes" + "context" + "errors" + "fmt" + "io" + "net" + "net/http" + "net/http/httptest" + "os" + "sync/atomic" + "testing" + "time" +) + +func noRetryDelay(ctx context.Context, _ time.Duration) error { return ctx.Err() } + +func TestClient_RetriesDialTimeout(t *testing.T) { + for _, method := range []string{"GET", "HEAD", "POST", "PUT", "PATCH", "DELETE"} { + t.Run(method, func(t *testing.T) { + var requests atomic.Int32 + bodies := make(chan string, 1) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) + body, _ := io.ReadAll(r.Body) + bodies <- string(body) + _, _ = io.WriteString(w, `{"ok":true}`) + })) + defer server.Close() + + var dials atomic.Int32 + transport := &http.Transport{DialContext: func(ctx context.Context, network, addr string) (net.Conn, error) { + if dials.Add(1) == 1 { + return nil, &net.OpError{Op: "dial", Net: network, Err: os.ErrDeadlineExceeded} + } + return (&net.Dialer{}).DialContext(ctx, network, addr) + }} + defer transport.CloseIdleConnections() + c := New(Config{ + BaseURL: server.URL, AccessToken: "test-jwt", + HTTPClient: &http.Client{Transport: transport}, + RetryConfig: &RetryConfig{MaxRetries: 2, SleepFn: noRetryDelay}, + }) + _, err := c.Do(context.Background(), method, "/test", nil, []byte(`{"name":"test"}`)) + if err != nil { + t.Fatalf("expected recovery after dial timeout: %v", err) + } + if dials.Load() != 2 || requests.Load() != 1 { + t.Fatalf("dials=%d requests=%d, want 2 dials and one server request", dials.Load(), requests.Load()) + } + if got := <-bodies; got != `{"name":"test"}` { + t.Errorf("request body = %q", got) + } + }) + } +} + +func TestClient_RetriesReadTimeout(t *testing.T) { + for _, tc := range []struct{ method, stage string }{{"GET", "headers"}, {"HEAD", "headers"}, {"GET", "body"}} { + t.Run(tc.method+"/"+tc.stage, func(t *testing.T) { + var requests atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if requests.Add(1) == 1 { + if tc.stage == "body" { + _, _ = io.WriteString(w, `{"ok":`) + w.(http.Flusher).Flush() + } + <-r.Context().Done() + return + } + _, _ = io.WriteString(w, `{"ok":true}`) + })) + defer server.Close() + c := New(Config{ + BaseURL: server.URL, AccessToken: "test-jwt", Timeout: 100 * time.Millisecond, + RetryConfig: &RetryConfig{MaxRetries: 2, SleepFn: noRetryDelay}, + }) + result, err := c.Do(context.Background(), tc.method, "/test", nil, nil) + if err != nil { + t.Fatalf("expected recovery after %s timeout: %v", tc.stage, err) + } + want := `{"ok":true}` + if tc.method == "HEAD" { + want = "null" + } + if string(result) != want || requests.Load() != 2 { + t.Errorf("result=%s requests=%d, want %s after 2 requests", result, requests.Load(), want) + } + }) + } +} + +func TestClient_DoesNotRetrySentWriteTimeout(t *testing.T) { + for _, method := range []string{"POST", "PUT", "PATCH", "DELETE"} { + for _, stage := range []string{"headers", "body"} { + t.Run(method+"/"+stage, func(t *testing.T) { + var requests atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) + _, _ = io.Copy(io.Discard, r.Body) + if stage == "body" { + _, _ = io.WriteString(w, `{"ok":`) + w.(http.Flusher).Flush() + } + <-r.Context().Done() + })) + defer server.Close() + c := New(Config{ + BaseURL: server.URL, AccessToken: "test-jwt", Timeout: 100 * time.Millisecond, + RetryConfig: &RetryConfig{MaxRetries: 2, SleepFn: noRetryDelay}, + }) + _, err := c.Do(context.Background(), method, "/test", nil, []byte(`{}`)) + if err == nil || !MayHaveApplied(err) || requests.Load() != 1 { + t.Errorf("err=%v mayHaveApplied=%t requests=%d, want one ambiguous write", err, MayHaveApplied(err), requests.Load()) + } + }) + } + } +} + +func TestClient_TransportRetryLimit(t *testing.T) { + for _, method := range []string{"GET", "POST"} { + for _, limit := range []int{0, 2} { + t.Run(fmt.Sprintf("%s/%d", method, limit), func(t *testing.T) { + var dials atomic.Int32 + transport := &http.Transport{DialContext: func(context.Context, string, string) (net.Conn, error) { + dials.Add(1) + return nil, &net.OpError{Op: "dial", Net: "tcp", Err: os.ErrDeadlineExceeded} + }} + defer transport.CloseIdleConnections() + c := New(Config{ + BaseURL: "http://example.test", AccessToken: "test-jwt", + HTTPClient: &http.Client{Transport: transport}, + RetryConfig: &RetryConfig{MaxRetries: limit, SleepFn: noRetryDelay}, + }) + _, err := c.Do(context.Background(), method, "/test", nil, nil) + var netErr net.Error + if !errors.As(err, &netErr) || !netErr.Timeout() || MayHaveApplied(err) || dials.Load() != int32(limit+1) { + t.Errorf("err=%v dials=%d, want an unsent timeout after %d attempts", err, dials.Load(), limit+1) + } + }) + } + } +} + +func TestClient_DoesNotRetryPermanentTransportError(t *testing.T) { + var dials atomic.Int32 + transport := &http.Transport{DialContext: func(context.Context, string, string) (net.Conn, error) { + dials.Add(1) + return nil, &net.DNSError{Err: "no such host", Name: "example.test", IsNotFound: true} + }} + defer transport.CloseIdleConnections() + c := New(Config{ + BaseURL: "http://example.test", AccessToken: "test-jwt", + HTTPClient: &http.Client{Transport: transport}, + RetryConfig: &RetryConfig{MaxRetries: 2, SleepFn: noRetryDelay}, + }) + _, err := c.Do(context.Background(), "GET", "/test", nil, nil) + if err == nil || dials.Load() != 1 { + t.Errorf("err=%v dials=%d, want one failed attempt", err, dials.Load()) + } +} + +func TestClient_DoesNotRetryCancelledRequest(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + var dials atomic.Int32 + transport := &http.Transport{DialContext: func(context.Context, string, string) (net.Conn, error) { + dials.Add(1) + cancel() + return nil, &net.OpError{Op: "dial", Net: "tcp", Err: os.ErrDeadlineExceeded} + }} + defer transport.CloseIdleConnections() + c := New(Config{ + BaseURL: "http://example.test", AccessToken: "test-jwt", + HTTPClient: &http.Client{Transport: transport}, + RetryConfig: &RetryConfig{MaxRetries: 2, SleepFn: noRetryDelay}, + }) + _, err := c.Do(ctx, "GET", "/test", nil, nil) + if !errors.Is(err, context.Canceled) || dials.Load() != 1 { + t.Errorf("err=%v dials=%d, want cancellation after one attempt", err, dials.Load()) + } +} + +func TestClient_CancelledWriteMayHaveApplied(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + var requests atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + requests.Add(1) + _, _ = io.Copy(io.Discard, r.Body) + cancel() + <-r.Context().Done() + })) + defer server.Close() + c := New(Config{ + BaseURL: server.URL, AccessToken: "test-jwt", + RetryConfig: &RetryConfig{MaxRetries: 2, SleepFn: noRetryDelay}, + }) + _, err := c.Do(ctx, "POST", "/test", nil, []byte(`{}`)) + if !errors.Is(err, context.Canceled) || !MayHaveApplied(err) || requests.Load() != 1 { + t.Errorf("err=%v mayHaveApplied=%t requests=%d, want one cancelled ambiguous write", err, MayHaveApplied(err), requests.Load()) + } +} + +func TestPageAll_TransportTimeout(t *testing.T) { + for _, recoverTimeout := range []bool{true, false} { + name := "exhausted" + if recoverTimeout { + name = "recovered" + } + t.Run(name, func(t *testing.T) { + var pageTwoAttempts atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Query().Get("page") == "1" { + _, _ = io.WriteString(w, `{"items":[1],"total_pages":2}`) + return + } + attempt := pageTwoAttempts.Add(1) + if attempt == 1 || !recoverTimeout { + <-r.Context().Done() + return + } + _, _ = io.WriteString(w, `{"items":[2],"total_pages":2}`) + })) + defer server.Close() + c := New(Config{ + BaseURL: server.URL, AccessToken: "test-jwt", Timeout: 100 * time.Millisecond, + RetryConfig: &RetryConfig{MaxRetries: 2, SleepFn: noRetryDelay}, + }) + var out bytes.Buffer + err := c.PageAll(PageAllConfig{Ctx: context.Background(), Method: "GET", Path: "/test", Writer: &out}) + want := "{\"items\":[1],\"total_pages\":2}\n" + attempts := int32(3) + if recoverTimeout { + want += "{\"items\":[2],\"total_pages\":2}\n" + attempts = 2 + } + if (err == nil) != recoverTimeout || out.String() != want || pageTwoAttempts.Load() != attempts { + t.Errorf("err=%v output=%q pageTwoAttempts=%d, want recovered=%t output=%q attempts=%d", err, out.String(), pageTwoAttempts.Load(), recoverTimeout, want, attempts) + } + }) + } +} diff --git a/internal/validate/params.go b/internal/validate/params.go index e95d77d..84c1ee6 100644 --- a/internal/validate/params.go +++ b/internal/validate/params.go @@ -61,6 +61,35 @@ func ValidateParams(raw string) (map[string]string, error) { return validateParamMap(obj) } +// MergeParams validates each repeated --params value and merges them into one +// map, so a later flag cannot silently replace an earlier one. +func MergeParams(raws []string) (map[string]string, error) { + merged := make(map[string]string) + for _, raw := range raws { + // An empty --params "$VAR" means no params. + if raw == "" { + continue + } + params, err := ValidateParams(raw) + if err != nil { + return nil, err + } + for k, v := range params { + if _, dup := merged[k]; dup { + return nil, duplicateKeyError(k) + } + merged[k] = v + } + } + return merged, nil +} + +func duplicateKeyError(key string) error { + return &ParamsValidationError{ + Reason: fmt.Sprintf("param key %q appears more than once; --params carries a single value per key, so multi-value filters cannot be sent", key), + } +} + // looksLikeQueryString reports whether the input is unambiguously URL-query // syntax, so that malformed JSON still reports as malformed JSON. func looksLikeQueryString(raw string) bool { @@ -117,9 +146,7 @@ func parseQueryString(raw string) (map[string]any, error) { } } if _, dup := obj[decodedKey]; dup { - return nil, &ParamsValidationError{ - Reason: fmt.Sprintf("param key %q appears more than once; --params carries a single value per key, so multi-value filters cannot be sent", decodedKey), - } + return nil, duplicateKeyError(decodedKey) } obj[decodedKey] = decodedValue } diff --git a/internal/validate/params_test.go b/internal/validate/params_test.go index 231f867..b62ff47 100644 --- a/internal/validate/params_test.go +++ b/internal/validate/params_test.go @@ -108,3 +108,42 @@ func TestValidateParams_Invalid(t *testing.T) { }) } } + +func TestMergeParams(t *testing.T) { + got, err := MergeParams([]string{`action_id=87`, `{"size":500}`, `end_ts=5&start_ts=1`, ``}) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + want := map[string]string{"action_id": "87", "size": "500", "end_ts": "5", "start_ts": "1"} + if len(got) != len(want) { + t.Fatalf("length mismatch: want %d, got %d (%v)", len(want), len(got), got) + } + for k, v := range want { + if got[k] != v { + t.Errorf("key %q: want %q, got %q", k, v, got[k]) + } + } +} + +func TestMergeParams_Invalid(t *testing.T) { + cases := []struct { + name string + raws []string + errWant string + }{ + {"same key in two flags", []string{`id=1`, `id=2`}, "appears more than once"}, + {"same key and value in two flags", []string{`id=1`, `{"id":1}`}, "appears more than once"}, + {"one bad value fails the merge", []string{`id=1`, `not json`}, "not valid JSON"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + _, err := MergeParams(tc.raws) + if err == nil { + t.Fatalf("expected error containing %q, got nil", tc.errWant) + } + if !strings.Contains(err.Error(), tc.errWant) { + t.Errorf("error mismatch:\n want substring: %q\n got: %q", tc.errWant, err.Error()) + } + }) + } +}