From 19dbc0ca58a7e72f810f63a36bd498b54d6773c7 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Mon, 12 Oct 2026 00:28:25 +0200 Subject: [PATCH 01/25] [Release] Give workflow tokens no permissions by default The goreleaser job keeps its own contents: write to publish the release; the homebrew job pushes with the tap's deploy key and needs no GITHUB_TOKEN scope. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/release.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index c78980f..820d193 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -5,6 +5,8 @@ on: tags: - "v*" +permissions: {} + jobs: goreleaser: runs-on: ubuntu-latest From f743ead46db109b33a8a7c80836942e291743b0f Mon Sep 17 00:00:00 2001 From: ilyazub Date: Mon, 12 Oct 2026 00:28:25 +0200 Subject: [PATCH 02/25] [Release] Build binaries with the latest patched Go release.yml took Go from go.mod's go line, 1.25.0, the minimum for go install users: v0.2.3's binaries carry 57 (darwin/arm64) and 58 (linux/amd64) Go standard library vulnerabilities by govulncheck -mode=binary. Release builds now use the latest stable Go; main built that way has none. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/release.yml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 820d193..30d63e7 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -18,7 +18,10 @@ jobs: fetch-depth: 0 - uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0 with: - go-version-file: go.mod + # Not go.mod: its go line is the minimum for go install users, while + # release binaries need the latest patched Go (v0.2.3 shipped go1.25.0). + go-version: stable + check-latest: true - uses: goreleaser/goreleaser-action@9ed2f89a662bf1735a48bc8557fd212fa902bebf # v6.2.1 with: version: ~> v2 From 1bc08635dd31afa1a69e6c346cef70a975cc48a3 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Mon, 12 Oct 2026 00:27:05 +0200 Subject: [PATCH 03/25] [Docs] Tell agents that only a v* tag releases, not a push to main A push or merge to main runs CI only; release.yml runs on v* tags, publishes 5 archives and checksums with GoReleaser, then pushes the formula to serpapi/homebrew-tap. AGENTS.md also covers the toolchain, the formula's test block, how to check a release and how to pull a bad one. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 AGENTS.md diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..9d32d45 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,13 @@ +# AGENTS.md + +## Releases + +- A push or merge to `main` releases nothing. It runs CI (`.github/workflows/ci.yml`), whose e2e job makes real searches with the `SERPAPI_KEY` secret. +- Only a pushed `v*` tag releases. `.github/workflows/release.yml` then: + 1. runs GoReleaser (`.goreleaser.yml`), which publishes a GitHub release with 5 archives (darwin and linux on amd64 and arm64, windows on amd64) and `serpapi__checksums.txt`, and stamps the version into `serpapi --version`; + 2. writes `Formula/serpapi-cli.rb` in `serpapi/homebrew-tap` from those checksums and pushes it as a `serpapi-cli ` commit. +- A tag reaches every Homebrew and `go install` user, so tagging is a maintainer's call. Tags here are lightweight: `git tag v0.3.0 origin/main && git push origin v0.3.0`. +- Release binaries build with the latest stable Go (`go-version: stable`); the `go` line in `go.mod` is the oldest Go that `go install` users need. +- The formula's `test do` block in `release.yml` runs the released binary, e.g. `serpapi account` without a key must exit 2 with "No API key". Change it together with the behavior it checks. +- Check a release: the Release run is green, the release has the 5 archives and the checksums file, the tap has the `serpapi-cli ` commit, and `brew upgrade serpapi-cli && brew test serpapi-cli` passes. +- To pull a bad release, revert the formula commit in the tap so new installs get the previous version, and fix forward with a new tag: `go install ...@latest` takes the highest tag. From ada4f9ea6b84c7ccaebfdca97936157d00d743b8 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Mon, 12 Oct 2026 00:29:27 +0200 Subject: [PATCH 04/25] [Deps] Update gojq, pretty, x/term and x/sys within Go 1.25 gojq 0.12.19, pretty 1.2.2, x/term 0.45.0 and x/sys 0.47.0 are the newest releases that still build with go 1.25.0; x/term 0.46 and x/sys 0.48 require Go 1.26, which would force a toolchain upgrade on go install users. pflag goes to 1.0.10 with cobra. Co-Authored-By: Claude Opus 5.5 --- go.mod | 12 ++++++------ go.sum | 23 ++++++++++++----------- 2 files changed, 18 insertions(+), 17 deletions(-) diff --git a/go.mod b/go.mod index afc3781..cb631e9 100644 --- a/go.mod +++ b/go.mod @@ -5,18 +5,18 @@ go 1.25.0 require ( github.com/BurntSushi/toml v1.6.0 github.com/briandowns/spinner v1.23.2 - github.com/itchyny/gojq v0.12.18 + github.com/itchyny/gojq v0.12.19 github.com/spf13/cobra v1.10.2 - github.com/tidwall/pretty v1.2.1 - golang.org/x/term v0.41.0 + github.com/tidwall/pretty v1.2.2 + golang.org/x/term v0.45.0 ) require ( github.com/fatih/color v1.7.0 // indirect github.com/inconshreveable/mousetrap v1.1.0 // indirect - github.com/itchyny/timefmt-go v0.1.7 // indirect + github.com/itchyny/timefmt-go v0.1.8 // indirect github.com/mattn/go-colorable v0.1.2 // indirect github.com/mattn/go-isatty v0.0.20 // indirect - github.com/spf13/pflag v1.0.9 // indirect - golang.org/x/sys v0.42.0 // indirect + github.com/spf13/pflag v1.0.10 // indirect + golang.org/x/sys v0.47.0 // indirect ) diff --git a/go.sum b/go.sum index 10eba25..9237421 100644 --- a/go.sum +++ b/go.sum @@ -7,10 +7,10 @@ github.com/fatih/color v1.7.0 h1:DkWD4oS2D8LGGgTQ6IvwJJXSL5Vp2ffcQg58nFV38Ys= github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4= github.com/inconshreveable/mousetrap v1.1.0 h1:wN+x4NVGpMsO7ErUn/mUI3vEoE6Jt13X2s0bqwp9tc8= github.com/inconshreveable/mousetrap v1.1.0/go.mod h1:vpF70FUmC8bwa3OWnCshd2FqLfsEA9PFc4w1p2J65bw= -github.com/itchyny/gojq v0.12.18 h1:gFGHyt/MLbG9n6dqnvlliiya2TaMMh6FFaR2b1H6Drc= -github.com/itchyny/gojq v0.12.18/go.mod h1:4hPoZ/3lN9fDL1D+aK7DY1f39XZpY9+1Xpjz8atrEkg= -github.com/itchyny/timefmt-go v0.1.7 h1:xyftit9Tbw+Dc/huSSPJaEmX1TVL8lw5vxjJLK4GMMA= -github.com/itchyny/timefmt-go v0.1.7/go.mod h1:5E46Q+zj7vbTgWY8o5YkMeYb4I6GeWLFnetPy5oBrAI= +github.com/itchyny/gojq v0.12.19 h1:ttXA0XCLEMoaLOz5lSeFOZ6u6Q3QxmG46vfgI4O0DEs= +github.com/itchyny/gojq v0.12.19/go.mod h1:5galtVPDywX8SPSOrqjGxkBeDhSxEW1gSxoy7tn1iZY= +github.com/itchyny/timefmt-go v0.1.8 h1:1YEo1JvfXeAHKdjelbYr/uCuhkybaHCeTkH8Bo791OI= +github.com/itchyny/timefmt-go v0.1.8/go.mod h1:5E46Q+zj7vbTgWY8o5YkMeYb4I6GeWLFnetPy5oBrAI= github.com/mattn/go-colorable v0.1.2 h1:/bC9yWikZXAL9uJdulbSfyVNIR3n3trXl+v8+1sx8mU= github.com/mattn/go-colorable v0.1.2/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE= github.com/mattn/go-isatty v0.0.8/go.mod h1:Iq45c/XA43vh69/j3iqttzPXn0bhXyGjM0Hdxcsrc5s= @@ -19,15 +19,16 @@ github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D github.com/russross/blackfriday/v2 v2.1.0/go.mod h1:+Rmxgy9KzJVeS9/2gXHxylqXiyQDYRxCVz55jmeOWTM= github.com/spf13/cobra v1.10.2 h1:DMTTonx5m65Ic0GOoRY2c16WCbHxOOw6xxezuLaBpcU= github.com/spf13/cobra v1.10.2/go.mod h1:7C1pvHqHw5A4vrJfjNwvOdzYu0Gml16OCs2GRiTUUS4= -github.com/spf13/pflag v1.0.9 h1:9exaQaMOCwffKiiiYk6/BndUBv+iRViNW+4lEMi0PvY= github.com/spf13/pflag v1.0.9/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= -github.com/tidwall/pretty v1.2.1 h1:qjsOFOWWQl+N3RsoF5/ssm1pHmJJwhjlSbZ51I6wMl4= -github.com/tidwall/pretty v1.2.1/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU= +github.com/spf13/pflag v1.0.10 h1:4EBh2KAYBwaONj6b2Ye1GiHfwjqyROoF4RwYO+vPwFk= +github.com/spf13/pflag v1.0.10/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An2Bg= +github.com/tidwall/pretty v1.2.2 h1:dz1jrRuE7or/74V490B4/GP1pZm5WKlt2bgCP5A83w8= +github.com/tidwall/pretty v1.2.2/go.mod h1:ITEVvHYasfjBbM0u2Pg8T2nJnzm8xPwvNhhsoaGGjNU= go.yaml.in/yaml/v3 v3.0.4/go.mod h1:DhzuOOF2ATzADvBadXxruRBLzYTpT36CKvDb3+aBEFg= golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.42.0 h1:omrd2nAlyT5ESRdCLYdm3+fMfNFE/+Rf4bDIQImRJeo= -golang.org/x/sys v0.42.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= -golang.org/x/term v0.41.0 h1:QCgPso/Q3RTJx2Th4bDLqML4W6iJiaXFq2/ftQF13YU= -golang.org/x/term v0.41.0/go.mod h1:3pfBgksrReYfZ5lvYM0kSO0LIkAl4Yl2bXOkKP7Ec2A= +golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= +golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/term v0.45.0 h1:NwWyBmoJCbfTHpxrWoZ9C6/VxOf7ic219I8xZZFdrf0= +golang.org/x/term v0.45.0/go.mod h1:9aqxs0blBcrm/n0L9QW0aRVD+ktan8ssZromtqJC43w= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= From f3ae4e30387088f8830fafc10107504abe01559f Mon Sep 17 00:00:00 2001 From: ilyazub Date: Mon, 12 Oct 2026 00:29:44 +0200 Subject: [PATCH 05/25] [Style] Run gofmt on the three files main left unformatted Field and comment alignment in root.go and output.go, and two stray blank lines, so the gofmt gate in the next commit passes. No code changes. Co-Authored-By: Claude Opus 5.5 --- pkg/cmd/login.go | 1 - pkg/cmd/root.go | 6 +++--- pkg/output/output.go | 13 ++++++------- 3 files changed, 9 insertions(+), 11 deletions(-) diff --git a/pkg/cmd/login.go b/pkg/cmd/login.go index 39d6e3f..dcdcc91 100644 --- a/pkg/cmd/login.go +++ b/pkg/cmd/login.go @@ -106,4 +106,3 @@ func runLogin(cmd *cobra.Command, args []string) error { fmt.Fprintf(os.Stderr, "Logged in as %s. API key saved to %q\n", email, config.Path()) return nil } - diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index 41eb197..7e27cb2 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -27,9 +27,9 @@ var ( ) var rootCmd = &cobra.Command{ - Use: "serpapi", - Short: "HTTP client for structured web search data via SerpApi", - Example: ` serpapi search engine=google q=coffee + Use: "serpapi", + Short: "HTTP client for structured web search data via SerpApi", + Example: ` serpapi search engine=google q=coffee serpapi search engine=google_light q="best pizza NYC" --jq ".organic_results[:3]" serpapi account`, Version: version.Version, diff --git a/pkg/output/output.go b/pkg/output/output.go index 6817e79..1c45b76 100644 --- a/pkg/output/output.go +++ b/pkg/output/output.go @@ -13,12 +13,12 @@ import ( // darkStyle matches stripe-cli's darkTerminalStyle exactly. var darkStyle = &pretty.Style{ - Key: [2]string{"\x1B[34m", "\x1B[0m"}, // blue - String: [2]string{"\x1B[32m", "\x1B[0m"}, // green - Number: [2]string{"\x1B[33m", "\x1B[0m"}, // yellow - True: [2]string{"\x1B[35m", "\x1B[0m"}, // magenta - False: [2]string{"\x1B[35m", "\x1B[0m"}, // magenta - Null: [2]string{"\x1B[31m", "\x1B[0m"}, // red + Key: [2]string{"\x1B[34m", "\x1B[0m"}, // blue + String: [2]string{"\x1B[32m", "\x1B[0m"}, // green + Number: [2]string{"\x1B[33m", "\x1B[0m"}, // yellow + True: [2]string{"\x1B[35m", "\x1B[0m"}, // magenta + False: [2]string{"\x1B[35m", "\x1B[0m"}, // magenta + Null: [2]string{"\x1B[31m", "\x1B[0m"}, // red } // shouldColor returns true when stdout is a TTY and NO_COLOR is not set. @@ -82,4 +82,3 @@ func formatNumber(f float64) string { } return fmt.Sprintf("%g", f) } - From 876a5a8bea27caac0ee169b7289970ad19d1ae95 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Mon, 12 Oct 2026 00:29:27 +0200 Subject: [PATCH 06/25] [CI] Gate on gofmt and data races, on Go 1.25 and the latest Go The test job runs gofmt and go test -race on two Go versions: 1.25, the minimum in go.mod for go install users, and the latest stable release, which builds the releases. The e2e job also uses the latest Go; it still skips fork pull requests, which get no SERPAPI_KEY. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f085081..3d00907 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,14 +11,23 @@ permissions: {} jobs: test: runs-on: ubuntu-latest + strategy: + matrix: + # 1.25 is the minimum in go.mod for go install users; stable builds the releases. + go: ["1.25", stable] steps: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0 with: - go-version-file: go.mod + go-version: ${{ matrix.go }} + check-latest: true + - name: gofmt + run: | + unformatted=$(gofmt -l .) + [ -z "$unformatted" ] || { echo "Run gofmt -w on: $unformatted"; exit 1; } - run: go vet ./... - run: go build ./cmd/serpapi - - run: go test ./pkg/... + - run: go test -race ./pkg/... e2e: runs-on: ubuntu-latest @@ -27,7 +36,8 @@ jobs: - uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2 - uses: actions/setup-go@d35c59abb061a4a6fb18e82ac0862c26744d6ab5 # v5.5.0 with: - go-version-file: go.mod + go-version: stable + check-latest: true - name: Run E2E tests if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository env: From 71bbba40575c48ad88173831c3ce2799b874ed41 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Mon, 12 Oct 2026 01:01:42 +0200 Subject: [PATCH 07/25] [Output] Escape control characters before they reach a terminal A terminal acts on control characters in results: a page title holding ESC ] 52 can set the clipboard, ESC [ 2J clears the screen. On a terminal, JSON and --jq strings now go through ForTerminal, which writes C0 and C1 controls, DEL and bidi overrides as \u escapes. Piped output is unchanged and keeps the bytes. PrintJSON and PrintJQValue take the writer and whether it is a terminal; handleOutput passes stdout. PrintRaw and SameStream are for later commits (output=md, the large-result warning). Co-Authored-By: Claude Opus 5.5 --- pkg/cmd/root.go | 4 +- pkg/output/output.go | 126 ++++++++++++++++++++++++++++---- pkg/output/output_test.go | 34 +++++++-- pkg/output/terminal_test.go | 142 ++++++++++++++++++++++++++++++++++++ 4 files changed, 284 insertions(+), 22 deletions(-) create mode 100644 pkg/output/terminal_test.go diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index 7e27cb2..62f6efb 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -135,13 +135,13 @@ func handleOutput(raw json.RawMessage) error { return &clierrors.UsageError{Message: err.Error()} } for _, val := range results { - if err := output.PrintJQValue(val, os.Stdout); err != nil { + if err := output.PrintJQValue(val, os.Stdout, output.IsTerminal(os.Stdout)); err != nil { return &clierrors.APIError{Message: fmt.Sprintf("Output error: %s", err)} } } return nil } - if err := output.PrintJSON(raw); err != nil { + if err := output.PrintJSON(os.Stdout, raw); err != nil { return &clierrors.APIError{Message: fmt.Sprintf("Output error: %s", err)} } return nil diff --git a/pkg/output/output.go b/pkg/output/output.go index 1c45b76..e380cee 100644 --- a/pkg/output/output.go +++ b/pkg/output/output.go @@ -6,6 +6,7 @@ import ( "fmt" "io" "os" + "unicode/utf8" "github.com/tidwall/pretty" "golang.org/x/term" @@ -21,32 +22,123 @@ var darkStyle = &pretty.Style{ Null: [2]string{"\x1B[31m", "\x1B[0m"}, // red } -// shouldColor returns true when stdout is a TTY and NO_COLOR is not set. -func shouldColor() bool { - if os.Getenv("NO_COLOR") != "" { +// IsTerminal reports whether w is a terminal. Tests replace it. +var IsTerminal = func(w io.Writer) bool { + f, ok := w.(*os.File) + return ok && term.IsTerminal(int(f.Fd())) +} + +// SameStream reports whether a and b write to the same file or pipe, as +// stdout and stderr do after 2>&1. +func SameStream(a, b io.Writer) bool { + fa, okA := a.(*os.File) + fb, okB := b.(*os.File) + if !okA || !okB { return false } - return term.IsTerminal(int(os.Stdout.Fd())) + sa, errA := fa.Stat() + sb, errB := fb.Stat() + return errA == nil && errB == nil && os.SameFile(sa, sb) } -// PrintJSON prints raw JSON bytes to stdout, pretty-formatted. -// Colorized when stdout is a TTY and NO_COLOR is not set. Key order is preserved. -func PrintJSON(data []byte) error { - var err error - if shouldColor() { - _, err = fmt.Fprint(os.Stdout, string(pretty.Color(pretty.Pretty(data), darkStyle))) - } else { - _, err = fmt.Fprint(os.Stdout, string(pretty.Pretty(data))) - } +// PrintJSON writes a JSON document to w as FormatJSON formats it. +func PrintJSON(w io.Writer, data []byte) error { + _, err := w.Write(FormatJSON(w, data)) return err } +// FormatJSON returns a JSON document as it is shown on w, preserving key +// order. A terminal gets it indented, through ForTerminal, and colored +// (unless NO_COLOR is set); anything else, such as a pipe, gets it indented as +// tidwall/pretty formats it. +func FormatJSON(w io.Writer, data []byte) []byte { + if IsTerminal(w) { + out := ForTerminal(pretty.Pretty(data)) + if os.Getenv("NO_COLOR") == "" { + out = pretty.Color(out, darkStyle) + } + return out + } + return pretty.Pretty(data) +} + +// PrintRaw writes a non-JSON response body (Markdown, HTML, CSV) to w as is, +// ending it with a newline. A terminal gets it through ForTerminal. +func PrintRaw(w io.Writer, body []byte) error { + if len(body) == 0 { + return nil + } + if IsTerminal(w) { + body = ForTerminal(body) + } + if _, err := w.Write(body); err != nil { + return err + } + if body[len(body)-1] != '\n' { + _, err := io.WriteString(w, "\n") + return err + } + return nil +} + +// ForTerminal makes text from search results safe to show on a terminal, +// which acts on control characters: a page title holding ESC ] 52 could set +// the clipboard, ESC [ 2J clears the screen. It writes as \u00XX every C0 +// control but tab and newline, DEL and the C1 controls, writes bidirectional +// override and isolate characters as \u20XX, drops the CR of a CRLF and turns +// invalid UTF-8 into U+FFFD. The escapes are valid inside JSON strings. +func ForTerminal(b []byte) []byte { + i := 0 + for i < len(b) { + r, size := utf8.DecodeRune(b[i:]) + if unsafeForTerminal(b, i, r, size) { + break + } + i += size + } + if i == len(b) { + return b + } + out := bytes.NewBuffer(make([]byte, 0, len(b)+16)) + out.Write(b[:i]) + for i < len(b) { + r, size := utf8.DecodeRune(b[i:]) + switch { + case r == '\r' && i+1 < len(b) && b[i+1] == '\n': + case r == utf8.RuneError && size == 1: + out.WriteRune(utf8.RuneError) + case unsafeForTerminal(b, i, r, size): + fmt.Fprintf(out, `\u%04x`, r) + default: + out.Write(b[i : i+size]) + } + i += size + } + return out.Bytes() +} + +func unsafeForTerminal(b []byte, i int, r rune, size int) bool { + switch { + case r == '\t' || r == '\n': + return false + case r < 0x20 || r == 0x7f || (r >= 0x80 && r <= 0x9f): + return true + case r >= 0x202a && r <= 0x202e, r >= 0x2066 && r <= 0x2069: + return true + } + return r == utf8.RuneError && size == 1 +} + // PrintJQValue prints a single jq result value with raw scalar output. // Strings are unquoted. Numbers and bools are printed as-is. // Null produces an empty line. Objects and arrays are JSON-encoded. -func PrintJQValue(v any, w io.Writer) error { +// terminal sends strings and JSON through ForTerminal. +func PrintJQValue(v any, w io.Writer, terminal bool) error { switch val := v.(type) { case string: + if terminal { + val = string(ForTerminal([]byte(val))) + } _, err := fmt.Fprintln(w, val) return err case bool: @@ -71,7 +163,11 @@ func PrintJQValue(v any, w io.Writer) error { if err := enc.Encode(val); err != nil { return err } - _, err := fmt.Fprint(w, buf.String()) + out := buf.Bytes() + if terminal { + out = ForTerminal(out) + } + _, err := w.Write(out) return err } } diff --git a/pkg/output/output_test.go b/pkg/output/output_test.go index e4b5e52..f8b6dc1 100644 --- a/pkg/output/output_test.go +++ b/pkg/output/output_test.go @@ -8,7 +8,7 @@ import ( func TestPrintJQValueString(t *testing.T) { var buf bytes.Buffer - err := PrintJQValue("hello", &buf) + err := PrintJQValue("hello", &buf, false) if err != nil { t.Fatal(err) } @@ -19,7 +19,7 @@ func TestPrintJQValueString(t *testing.T) { func TestPrintJQValueNumber(t *testing.T) { var buf bytes.Buffer - err := PrintJQValue(42.0, &buf) + err := PrintJQValue(42.0, &buf, false) if err != nil { t.Fatal(err) } @@ -30,7 +30,7 @@ func TestPrintJQValueNumber(t *testing.T) { func TestPrintJQValueBool(t *testing.T) { var buf bytes.Buffer - err := PrintJQValue(true, &buf) + err := PrintJQValue(true, &buf, false) if err != nil { t.Fatal(err) } @@ -41,7 +41,7 @@ func TestPrintJQValueBool(t *testing.T) { func TestPrintJQValueNull(t *testing.T) { var buf bytes.Buffer - err := PrintJQValue(nil, &buf) + err := PrintJQValue(nil, &buf, false) if err != nil { t.Fatal(err) } @@ -54,7 +54,7 @@ func TestPrintJQValueNull(t *testing.T) { func TestPrintJQValueObject(t *testing.T) { var buf bytes.Buffer obj := map[string]any{"key": "value"} - err := PrintJQValue(obj, &buf) + err := PrintJQValue(obj, &buf, false) if err != nil { t.Fatal(err) } @@ -63,3 +63,27 @@ func TestPrintJQValueObject(t *testing.T) { t.Errorf("expected JSON object, got %q", output) } } + +func TestPrintJSONIndentsWhenNotATerminal(t *testing.T) { + var buf bytes.Buffer + in := `{"b":1, "a":" & y","c":[1,{"d":2}]}` + if err := PrintJSON(&buf, []byte(in)); err != nil { + t.Fatal(err) + } + want := "{\n \"b\": 1,\n \"a\": \" & y\",\n \"c\": [\n 1,\n {\n \"d\": 2\n }\n ]\n}\n" + if buf.String() != want { + t.Errorf("got %q, want %q", buf.String(), want) + } +} + +func TestPrintRaw(t *testing.T) { + for in, want := range map[string]string{"": "", "# Title": "# Title\n", "a\n": "a\n"} { + var buf bytes.Buffer + if err := PrintRaw(&buf, []byte(in)); err != nil { + t.Fatal(err) + } + if buf.String() != want { + t.Errorf("PrintRaw(%q) = %q, want %q", in, buf.String(), want) + } + } +} diff --git a/pkg/output/terminal_test.go b/pkg/output/terminal_test.go new file mode 100644 index 0000000..07387b6 --- /dev/null +++ b/pkg/output/terminal_test.go @@ -0,0 +1,142 @@ +package output + +import ( + "bytes" + "encoding/json" + "io" + "reflect" + "strings" + "testing" + "unicode/utf8" +) + +func asTerminal(t *testing.T) { + t.Helper() + old := IsTerminal + IsTerminal = func(io.Writer) bool { return true } + t.Cleanup(func() { IsTerminal = old }) +} + +func TestForTerminal(t *testing.T) { + for _, tc := range []struct{ name, in, want string }{ + {"ordinary unicode", "Café ☕ 東京 🙂 مرحبا", "Café ☕ 東京 🙂 مرحبا"}, + {"tab and newline", "a\tb\nc", "a\tb\nc"}, + {"CSI clears the screen", "x\x1b[2Jy", `x\u001b[2Jy`}, + {"OSC 52 sets the clipboard", "\x1b]52;c;ZXZpbA==\x07", `\u001b]52;c;ZXZpbA==\u0007`}, + {"OSC 8 hyperlink", "\x1b]8;;https://evil.example\x1b\\click\x1b]8;;\x1b\\", `\u001b]8;;https://evil.example\u001b\click\u001b]8;;\u001b\`}, + {"C1 CSI", "a\u009b31mb", `a\u009b31mb`}, + {"DEL", "a\x7fb", `a\u007fb`}, + {"CRLF becomes LF", "a\r\nb\r\n", "a\nb\n"}, + {"lone CR", "abc\rX", `abc\u000dX`}, + {"NUL", "a\x00b", `a\u0000b`}, + {"bidi override", "invoice\u202Efdp.exe", `invoice\u202efdp.exe`}, + {"bidi isolate", "a\u2066b\u2069", `a\u2066b\u2069`}, + {"invalid UTF-8", "a\xffb\xc2", "a\uFFFDb\uFFFD"}, + {"literal U+FFFD stays", "a\uFFFDb", "a\uFFFDb"}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := string(ForTerminal([]byte(tc.in))); got != tc.want { + t.Errorf("ForTerminal(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } +} + +func TestForTerminalReturnsCleanInputAsIs(t *testing.T) { + in := []byte(`{"title":"Café","link":"https://example.com"}`) + if out := ForTerminal(in); &out[0] != &in[0] { + t.Error("clean input was copied") + } +} + +func TestTerminalOutputIsSanitized(t *testing.T) { + asTerminal(t) + t.Setenv("NO_COLOR", "") + hostile := "Shop\x1b]52;c;cGF3bmVk\x07 \u009b2J" + + var raw bytes.Buffer + if err := PrintRaw(&raw, []byte("# "+hostile+"\r\n")); err != nil { + t.Fatal(err) + } + if got, want := raw.String(), "# Shop\\u001b]52;c;cGF3bmVk\\u0007 \\u009b2J\n"; got != want { + t.Errorf("PrintRaw = %q, want %q", got, want) + } + + var jqOut bytes.Buffer + if err := PrintJQValue(hostile, &jqOut, true); err != nil { + t.Fatal(err) + } + if strings.ContainsAny(jqOut.String(), "\x1b\x07\u009b") { + t.Errorf("jq string kept control characters: %q", jqOut.String()) + } + jqOut.Reset() + if err := PrintJQValue(map[string]any{"t": "a\x7fb\u009b"}, &jqOut, true); err != nil { + t.Fatal(err) + } + if got := jqOut.String(); got != "{\"t\":\"a\\u007fb\\u009b\"}\n" { + t.Errorf("jq object = %q", got) + } + + var js bytes.Buffer + if err := PrintJSON(&js, []byte("{\"title\":\"a\u009bb\x7f\"}")); err != nil { + t.Fatal(err) + } + out := js.String() + if strings.Contains(out, "\u009b") || strings.Contains(out, "\x7f") { + t.Errorf("colored JSON kept C1 or DEL: %q", out) + } + if !strings.Contains(out, "\x1b[34m") { + t.Errorf("colored JSON lost its own colors: %q", out) + } +} + +func TestPipedOutputIsExact(t *testing.T) { + hostile := "a\x1b[2Jb\u009b\r\n" + var raw, jqOut bytes.Buffer + if err := PrintRaw(&raw, []byte(hostile)); err != nil { + t.Fatal(err) + } + if err := PrintJQValue(hostile, &jqOut, false); err != nil { + t.Fatal(err) + } + if raw.String() != hostile || jqOut.String() != hostile+"\n" { + t.Errorf("piped output changed: %q, %q", raw.String(), jqOut.String()) + } +} + +func FuzzTerminalJSONKeepsValue(f *testing.F) { + f.Add([]byte("{\"a\":\"\u009b[2J\x7f\",\"b\":[1,\"\u202e\"]}")) + f.Add([]byte(`{"t":"\u001b]52;c;eA==\u0007"}`)) + f.Add([]byte("[\"\xff\xfe\",null]")) + f.Fuzz(func(t *testing.T, data []byte) { + if !json.Valid(data) { + return + } + old := IsTerminal + IsTerminal = func(io.Writer) bool { return true } + defer func() { IsTerminal = old }() + t.Setenv("NO_COLOR", "1") + var buf bytes.Buffer + if err := PrintJSON(&buf, data); err != nil { + t.Fatal(err) + } + out := buf.Bytes() + for i := 0; i < len(out); { + r, size := utf8.DecodeRune(out[i:]) + if unsafeForTerminal(out, i, r, size) { + t.Fatalf("terminal output keeps %U at %d: %q", r, i, out) + } + i += size + } + var before, after any + if err := json.Unmarshal(data, &before); err != nil { + return + } + if err := json.Unmarshal(out, &after); err != nil { + t.Fatalf("terminal output is not JSON: %v: %q", err, out) + } + if !reflect.DeepEqual(before, after) { + t.Fatalf("terminal output changed the value: %q -> %q", data, out) + } + }) +} From 32d1dcb3da118f89748ca40aba067ce6d6817a07 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Mon, 12 Oct 2026 00:31:10 +0200 Subject: [PATCH 08/25] [Errors] Add the HTTP status to JSON error lines, and JSON warning lines FprintError writes {"error":{"code","message","status"}} to any writer; status is the HTTP status of the API's answer, 200 when a successful response reported the error, and absent when there was none. It finds the message through wrapped errors, keeps redacting api_key values, and escapes control characters when stderr is a terminal. PrintWarning writes {"warning":{"message"}} the same way, and InterruptedError is exit 130, code interrupted. The API client sets the status in the next commit; warnings and interrupts get their callers later. Co-Authored-By: Claude Opus 5.5 --- pkg/errors/errors.go | 132 +++++++++------- pkg/errors/errors_test.go | 149 ++++++++++++++++++ .../326f249c16308530 | 4 + 3 files changed, 229 insertions(+), 56 deletions(-) create mode 100644 pkg/errors/testdata/fuzz/FuzzFprintErrorIsOneRedactedJSONLine/326f249c16308530 diff --git a/pkg/errors/errors.go b/pkg/errors/errors.go index 325b0d9..36ab44b 100644 --- a/pkg/errors/errors.go +++ b/pkg/errors/errors.go @@ -4,9 +4,11 @@ import ( "bytes" "encoding/json" "errors" - "fmt" + "io" "os" "strings" + + "github.com/serpapi/serpapi-cli/pkg/output" ) // UsageError indicates incorrect CLI usage (exit code 2). @@ -17,7 +19,11 @@ func (e *UsageError) Error() string { return "Usage error: " + e.Message } // APIError indicates an error returned by the SerpApi service (exit code 1). type APIError struct { Message string - Cause error + // Status is the HTTP status of the response: 200 when the API reported the + // error inside a successful response, 0 when there was no usable answer, + // such as a 200 with a body that is not JSON. + Status int + Cause error } func (e *APIError) Error() string { return "API error: " + e.Message } @@ -32,43 +38,35 @@ type NetworkError struct { func (e *NetworkError) Error() string { return "Network error: " + e.Message } func (e *NetworkError) Unwrap() error { return e.Cause } +// InterruptedError means Ctrl-C or SIGTERM stopped the CLI (exit code 130). +type InterruptedError struct{ Message string } + +func (e *InterruptedError) Error() string { return "Interrupted: " + e.Message } + // ExitCode returns the appropriate process exit code for the given error. func ExitCode(err error) int { - var ue *UsageError - if errors.As(err, &ue) { - return 2 - } - var ae *APIError - if errors.As(err, &ae) { - return 1 - } - var ne *NetworkError - if errors.As(err, &ne) { - return 1 - } - if isCobraUsageError(err) { - return 2 - } - return 1 + _, exit := classify(err) + return exit } -func errorCode(err error) string { +func classify(err error) (code string, exit int) { var ue *UsageError - if errors.As(err, &ue) { - return "usage_error" - } var ae *APIError - if errors.As(err, &ae) { - return "api_error" - } var ne *NetworkError - if errors.As(err, &ne) { - return "network_error" - } - if isCobraUsageError(err) { - return "usage_error" + var ie *InterruptedError + switch { + case errors.As(err, &ie): + return "interrupted", 130 + case errors.As(err, &ue): + return "usage_error", 2 + case errors.As(err, &ae): + return "api_error", 1 + case errors.As(err, &ne): + return "network_error", 1 + case isCobraUsageError(err): + return "usage_error", 2 } - return "api_error" + return "api_error", 1 } // isCobraUsageError detects cobra argument-validation errors by message pattern. @@ -84,39 +82,61 @@ func isCobraUsageError(err error) bool { strings.HasPrefix(msg, "invalid argument") } -// PrintError writes a structured JSON error to stderr. +// PrintError writes err to stderr; see FprintError. func PrintError(err error) { - code := errorCode(err) - message := RedactAPIKey(err.Error()) - - // Strip the "Usage error: " / "API error: " / "Network error: " prefix - // that the Error() method adds, so the JSON message stays clean. - switch e := err.(type) { - case *UsageError: - message = RedactAPIKey(e.Message) - case *APIError: - message = RedactAPIKey(e.Message) - case *NetworkError: - message = RedactAPIKey(e.Message) - } + FprintError(os.Stderr, err) +} + +// FprintError writes err to w as one line of JSON: +// +// {"error":{"code":"api_error","message":"...","status":429}} +// +// status is the HTTP status of the API's answer, 200 when a successful +// response reported an error, and absent when there was no usable answer. +func FprintError(w io.Writer, err error) { + code, _ := classify(err) + body := struct { + Code string `json:"code"` + Message string `json:"message"` + Status int `json:"status,omitempty"` + }{Code: code, Message: err.Error()} - payload := map[string]any{ - "error": map[string]any{ - "code": code, - "message": message, - }, + var ue *UsageError + var ae *APIError + var ne *NetworkError + var ie *InterruptedError + switch { + case errors.As(err, &ie): + body.Message = ie.Message + case errors.As(err, &ue): + body.Message = ue.Message + case errors.As(err, &ae): + body.Message, body.Status = ae.Message, ae.Status + case errors.As(err, &ne): + body.Message = ne.Message } + body.Message = RedactAPIKey(body.Message) + writeJSONLine(w, map[string]any{"error": body}) +} + +// PrintWarning writes a non-fatal problem to w as one line of JSON, so stderr +// stays parseable line by line: +// +// {"warning":{"message":"..."}} +func PrintWarning(w io.Writer, message string) { + writeJSONLine(w, map[string]any{"warning": map[string]string{"message": RedactAPIKey(message)}}) +} +func writeJSONLine(w io.Writer, v any) { var buf bytes.Buffer enc := json.NewEncoder(&buf) enc.SetEscapeHTML(false) - if jsonErr := enc.Encode(payload); jsonErr != nil { - // Fallback if JSON encoding fails - fmt.Fprintf(os.Stderr, "{\"error\":{\"code\":\"%s\",\"message\":\"%s\"}}\n", - code, strings.ReplaceAll(strings.ReplaceAll(message, "\\", "\\\\"), "\"", "\\\"")) - return + _ = enc.Encode(v) + b := buf.Bytes() + if output.IsTerminal(w) { + b = output.ForTerminal(b) } - fmt.Fprint(os.Stderr, buf.String()) + _, _ = w.Write(b) } // RedactAPIKey replaces api_key values in s with [REDACTED]. diff --git a/pkg/errors/errors_test.go b/pkg/errors/errors_test.go index 1da2d15..e64e852 100644 --- a/pkg/errors/errors_test.go +++ b/pkg/errors/errors_test.go @@ -5,9 +5,13 @@ import ( "encoding/json" "errors" "fmt" + "io" "os" "strings" "testing" + "unicode/utf8" + + "github.com/serpapi/serpapi-cli/pkg/output" ) func TestRedactNoAPIKey(t *testing.T) { @@ -119,3 +123,148 @@ func TestExitCodeNetworkError(t *testing.T) { t.Errorf("expected 1, got %d", code) } } + +func decodeError(t *testing.T, err error) map[string]any { + t.Helper() + var buf bytes.Buffer + FprintError(&buf, err) + out := buf.String() + if strings.Count(out, "\n") != 1 || !strings.HasSuffix(out, "\n") { + t.Fatalf("expected exactly one line of output, got %q", out) + } + var parsed struct { + Error map[string]any `json:"error"` + } + if jsonErr := json.Unmarshal(buf.Bytes(), &parsed); jsonErr != nil { + t.Fatalf("invalid error JSON %q: %v", out, jsonErr) + } + return parsed.Error +} + +func TestWriteErrorFields(t *testing.T) { + cases := []struct { + name string + err error + code string + message string + status any + }{ + {"api error with HTTP status", &APIError{Message: "Invalid API key.", Status: 401}, "api_error", "Invalid API key.", float64(401)}, + {"api error inside a 200 response", &APIError{Message: "No results.", Status: 200}, "api_error", "No results.", float64(200)}, + {"api error without a usable answer", &APIError{Message: "Unexpected non-JSON response from server"}, "api_error", "Unexpected non-JSON response from server", nil}, + {"wrapped network error", fmt.Errorf("ctx: %w", &NetworkError{Message: "timed out"}), "network_error", "timed out", nil}, + {"wrapped usage error", fmt.Errorf("ctx: %w", &UsageError{Message: "bad flag"}), "usage_error", "bad flag", nil}, + {"untyped error", errors.New("boom"), "api_error", "boom", nil}, + {"cobra usage error", errors.New(`unknown command "x" for "serpapi"`), "usage_error", `unknown command "x" for "serpapi"`, nil}, + {"redacts api key", &NetworkError{Message: "GET /search?api_key=secret123&q=x failed"}, "network_error", "GET /search?api_key=[REDACTED]&q=x failed", nil}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := decodeError(t, tc.err) + if got["code"] != tc.code { + t.Errorf("code = %v, want %v", got["code"], tc.code) + } + if got["message"] != tc.message { + t.Errorf("message = %v, want %v", got["message"], tc.message) + } + if got["status"] != tc.status { + t.Errorf("status = %v, want %v", got["status"], tc.status) + } + }) + } +} + +func TestWriteErrorKeepsHTMLCharacters(t *testing.T) { + got := decodeError(t, &APIError{Message: "Use & "}) + if got["message"] != "Use & " { + t.Errorf("message = %v", got["message"]) + } +} + +func TestPrintWarningIsOneRedactedJSONLine(t *testing.T) { + var buf bytes.Buffer + PrintWarning(&buf, "page 2 failed: GET /search?api_key=secret & more") + line := buf.String() + if strings.Count(line, "\n") != 1 || !strings.HasSuffix(line, "\n") { + t.Fatalf("want exactly one line, got %q", line) + } + var got struct { + Warning struct { + Message string `json:"message"` + } `json:"warning"` + } + if err := json.Unmarshal(buf.Bytes(), &got); err != nil { + t.Fatalf("not JSON: %v", err) + } + if strings.Contains(got.Warning.Message, "secret") || !strings.Contains(got.Warning.Message, "& more") { + t.Errorf("message = %q", got.Warning.Message) + } +} + +func FuzzFprintErrorIsOneRedactedJSONLine(f *testing.F) { + f.Add("request failed: ", "abc123def456", "&q=coffee") + f.Add("api_key=", "0123456789abcdef", "\nsecond line") + f.Add("x\"y\\", "SECRETSECRET", "' end") + f.Fuzz(func(t *testing.T, prefix, secret, suffix string) { + msg := prefix + "api_key=" + secret + suffix + var buf bytes.Buffer + FprintError(&buf, &APIError{Message: msg, Status: 500}) + out := buf.String() + if strings.Count(out, "\n") != 1 || !strings.HasSuffix(out, "\n") { + t.Fatalf("not one line: %q", out) + } + var parsed struct { + Error struct { + Message string `json:"message"` + } `json:"error"` + } + if err := json.Unmarshal(buf.Bytes(), &parsed); err != nil { + t.Fatalf("not JSON: %q: %v", out, err) + } + if utf8.ValidString(msg) && parsed.Error.Message != RedactAPIKey(msg) { + t.Fatalf("message %q, want %q", parsed.Error.Message, RedactAPIKey(msg)) + } + if len(secret) >= 8 && isAlnum(secret) && !strings.Contains(prefix+suffix, secret) && strings.Contains(parsed.Error.Message, secret) { + t.Fatalf("secret %q leaked in %q", secret, parsed.Error.Message) + } + }) +} + +func isAlnum(s string) bool { + for _, r := range s { + if !('a' <= r && r <= 'z' || 'A' <= r && r <= 'Z' || '0' <= r && r <= '9') { + return false + } + } + return true +} + +// gojq errors quote response values, such as a page title, so stderr gets +// the same terminal sanitizing as stdout, and the JSON still decodes to the +// original message. +func TestTerminalStderrIsSanitized(t *testing.T) { + isTerminal := output.IsTerminal + output.IsTerminal = func(io.Writer) bool { return true } + t.Cleanup(func() { output.IsTerminal = isTerminal }) + + msg := "expected an object but got: string (\"invoice\u202etxt.exe \u009b31m red\x7f\")" + for name, print := range map[string]func(io.Writer){ + "error": func(w io.Writer) { FprintError(w, &UsageError{Message: msg}) }, + "warning": func(w io.Writer) { PrintWarning(w, msg) }, + } { + var buf bytes.Buffer + print(&buf) + if strings.ContainsAny(buf.String(), "\u202e\u009b\x7f") { + t.Errorf("%s: raw control characters reached the terminal: %q", name, buf.String()) + } + var got map[string]struct { + Message string `json:"message"` + } + if err := json.Unmarshal(buf.Bytes(), &got); err != nil { + t.Fatalf("%s: not JSON: %v", name, err) + } + if got[name].Message != msg { + t.Errorf("%s: message = %q, want %q", name, got[name].Message, msg) + } + } +} diff --git a/pkg/errors/testdata/fuzz/FuzzFprintErrorIsOneRedactedJSONLine/326f249c16308530 b/pkg/errors/testdata/fuzz/FuzzFprintErrorIsOneRedactedJSONLine/326f249c16308530 new file mode 100644 index 0000000..0125cfb --- /dev/null +++ b/pkg/errors/testdata/fuzz/FuzzFprintErrorIsOneRedactedJSONLine/326f249c16308530 @@ -0,0 +1,4 @@ +go test fuzz v1 +string("\x0000000") +string("00000000") +string("0") From 4483ee7501794bb3516d561a4188c76e4d6815f0 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Mon, 12 Oct 2026 00:31:49 +0200 Subject: [PATCH 09/25] [API] Keep the HTTP status on errors and the request URL out of them - A non-200 answer is an APIError with its status and SerpApi's message; an error inside a 200 body gets status 200. - Transport errors name the endpoint, not the request URL that carries the api_key. A timeout suggests twice the limit that ran out, at least 120 s, and says the search may still finish and be in the archive. - search, account and archive refuse a body that is not a JSON object, and locations one that is not an array, instead of printing it as a result. - SearchRaw and ArchiveRaw return output=md, html, csv and other formats as is, for a later commit; a JSON error body is still an error. - New takes WithBaseURL and WithTimeout options; NewWithTimeout stays. Co-Authored-By: Claude Opus 5.5 --- pkg/api/client.go | 268 +++++++++++++++++++-------------- pkg/api/client_test.go | 329 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 485 insertions(+), 112 deletions(-) diff --git a/pkg/api/client.go b/pkg/api/client.go index 9a6d7ab..0245e1d 100644 --- a/pkg/api/client.go +++ b/pkg/api/client.go @@ -10,6 +10,8 @@ import ( "net" "net/http" "net/url" + "strconv" + "strings" "time" clierrors "github.com/serpapi/serpapi-cli/pkg/errors" @@ -17,51 +19,69 @@ import ( ) const ( - // DefaultTimeout bounds a single HTTP request. Slow engines such as - // google_ai_mode can take well over 30s with no_cache=true, so this - // matches the 60s default used by the official serpapi-golang library. - DefaultTimeout = 60 * time.Second - maxResponseBytes = 100 << 20 // 100 MB + // DefaultBaseURL is the SerpApi production endpoint. + DefaultBaseURL = "https://serpapi.com" + // DefaultTimeout bounds one HTTP request, including reading the response + // body. It matches the 60s default of the official serpapi-golang library. + // Slow engines such as google_ai_mode can take longer with no_cache=true, + // and SerpApi still finishes and counts a search the client gave up on. + DefaultTimeout = 60 * time.Second ) +// maxResponseBytes caps the response body; a variable so tests can lower it. +var maxResponseBytes = 100 << 20 + // Client is an HTTP client for the SerpApi service. type Client struct { apiKey string baseURL string - timeout time.Duration http *http.Client } -// New creates a new SerpApi client with DefaultTimeout. apiKey may be empty -// for unauthenticated requests. -func New(apiKey string) *Client { - return NewWithTimeout(apiKey, DefaultTimeout) +// Option configures a Client. +type Option func(*Client) + +// WithBaseURL points the client at a different SerpApi-compatible host. +func WithBaseURL(baseURL string) Option { + return func(c *Client) { c.baseURL = strings.TrimRight(baseURL, "/") } } -// NewWithTimeout creates a new SerpApi client whose requests are bounded by -// timeout. A zero or negative timeout disables the limit entirely. +// WithTimeout sets the per-request timeout. Zero or negative disables it. +func WithTimeout(d time.Duration) Option { + return func(c *Client) { c.http.Timeout = max(d, 0) } +} + +// NewWithTimeout creates a client whose requests are bounded by timeout; zero +// or negative disables the limit. func NewWithTimeout(apiKey string, timeout time.Duration) *Client { - if timeout < 0 { - timeout = 0 - } - return &Client{ + return New(apiKey, WithTimeout(timeout)) +} + +// New creates a new SerpApi client. apiKey may be empty for unauthenticated requests. +func New(apiKey string, opts ...Option) *Client { + c := &Client{ apiKey: apiKey, - baseURL: "https://serpapi.com", - timeout: timeout, + baseURL: DefaultBaseURL, http: &http.Client{ - Timeout: timeout, - CheckRedirect: func(req *http.Request, via []*http.Request) error { + Timeout: DefaultTimeout, + // Never follow redirects: the request URL carries the API key. + CheckRedirect: func(*http.Request, []*http.Request) error { return http.ErrUseLastResponse }, }, } + for _, opt := range opts { + opt(c) + } + return c } -func (c *Client) userAgent() string { +func userAgent() string { return "serpapi-go-cli/" + version.Version } -func (c *Client) doGet(ctx context.Context, endpoint string, params map[string]string) ([]byte, error) { +// get performs a GET request and returns the body of a 200 response verbatim. +func (c *Client) get(ctx context.Context, endpoint string, params map[string]string) ([]byte, error) { u, err := url.Parse(c.baseURL + endpoint) if err != nil { return nil, &clierrors.NetworkError{Message: "Invalid URL: " + err.Error(), Cause: err} @@ -73,97 +93,104 @@ func (c *Client) doGet(ctx context.Context, endpoint string, params map[string]s } u.RawQuery = q.Encode() - req, err := http.NewRequestWithContext(ctx, "GET", u.String(), nil) + req, err := http.NewRequestWithContext(ctx, http.MethodGet, u.String(), nil) if err != nil { - return nil, &clierrors.NetworkError{Message: err.Error(), Cause: err} + return nil, &clierrors.NetworkError{Message: "Invalid request: " + endpoint, Cause: err} } - req.Header.Set("User-Agent", c.userAgent()) + req.Header.Set("User-Agent", userAgent()) resp, err := c.http.Do(req) if err != nil { - if c.isTimeout(req, err) { - return nil, &clierrors.NetworkError{Message: c.timeoutMessage(), Cause: err} - } - return nil, &clierrors.NetworkError{Message: err.Error(), Cause: err} + return nil, c.networkError(endpoint, err) } defer resp.Body.Close() - body, err := io.ReadAll(io.LimitReader(resp.Body, maxResponseBytes)) + body, err := io.ReadAll(io.LimitReader(resp.Body, int64(maxResponseBytes)+1)) if err != nil { - return nil, &clierrors.NetworkError{Message: "Failed to read response: " + err.Error(), Cause: err} - } - // Detect truncation: try reading one more byte; if it succeeds the body exceeded the limit. - if len(body) == maxResponseBytes { - var extra [1]byte - if n, _ := resp.Body.Read(extra[:]); n > 0 { - return nil, &clierrors.APIError{Message: "Response exceeded 100 MB limit"} - } + return nil, c.networkError(endpoint, err) } if resp.StatusCode != http.StatusOK { - // Try to extract error message from JSON body - var parsed map[string]any - if json.Unmarshal(body, &parsed) == nil { - if errVal, ok := parsed["error"]; ok { - if msg, ok := errVal.(string); ok { - return nil, &clierrors.APIError{Message: msg} - } - } - } - return nil, &clierrors.APIError{ - Message: fmt.Sprintf("HTTP %d: %s", resp.StatusCode, http.StatusText(resp.StatusCode)), + message := fmt.Sprintf("HTTP %d: %s", resp.StatusCode, http.StatusText(resp.StatusCode)) + if msg := errorMessage(body); msg != "" { + message = msg } + return nil, &clierrors.APIError{Message: message, Status: resp.StatusCode} } - - // Validate response is JSON (guard against HTML error pages with 200 status). - trimmed := bytes.TrimSpace(body) - if len(trimmed) == 0 || (trimmed[0] != '{' && trimmed[0] != '[') { - return nil, &clierrors.APIError{Message: "Unexpected non-JSON response from server"} + if len(body) > maxResponseBytes { + return nil, &clierrors.APIError{Message: "Response exceeded 100 MB limit"} } - return body, nil } -// isTimeout reports whether err was caused by the client's own timeout rather -// than by the caller cancelling the request context. -func (c *Client) isTimeout(req *http.Request, err error) bool { - if c.timeout <= 0 { - return false - } - if ctxErr := req.Context().Err(); ctxErr != nil { - // The caller's context ended first (Ctrl-C, parent deadline, ...). - return false +// networkError describes a transport failure without the request URL, which contains the API key. +func (c *Client) networkError(endpoint string, err error) error { + cause := err + var urlErr *url.Error + if errors.As(err, &urlErr) { + cause = urlErr.Err } var netErr net.Error - if errors.As(err, &netErr) && netErr.Timeout() { - return true + if c.http.Timeout > 0 && (errors.Is(err, context.DeadlineExceeded) || (errors.As(err, &netErr) && netErr.Timeout())) { + return &clierrors.NetworkError{ + Message: fmt.Sprintf("Request to %s timed out after %s. Slow engines, such as google_ai_mode, and no_cache=true can take longer: "+ + "retry with a higher --timeout, such as --timeout %d, or 0 to wait indefinitely. "+ + "The search may still finish on SerpApi's side, count against your searches and be available in your search archive", + endpoint, seconds(c.http.Timeout), suggestedTimeout(c.http.Timeout)), + Cause: cause, + } + } + return &clierrors.NetworkError{ + Message: fmt.Sprintf("Request to %s failed: %v", endpoint, cause), + Cause: cause, } - return errors.Is(err, context.DeadlineExceeded) } -func (c *Client) timeoutMessage() string { - return fmt.Sprintf( - "Request timed out after %gs waiting for SerpApi to respond. "+ - "Slow engines (e.g. google_ai_mode) or no_cache=true can take longer; "+ - "retry with --timeout (0 to wait indefinitely). "+ - "The search may still have completed server-side and be available in your SerpApi search archive.", - c.timeout.Seconds(), - ) +// suggestedTimeout is the --timeout to suggest after d ran out: twice d in +// whole seconds, and at least 120. +func suggestedTimeout(d time.Duration) int64 { + return max(int64((2*d+time.Second-1)/time.Second), 120) +} + +// seconds formats d the way --timeout takes it: 60s rather than 1m0s. +func seconds(d time.Duration) string { + if d%time.Second == 0 { + return strconv.FormatInt(int64(d/time.Second), 10) + "s" + } + return d.String() } -// checkAPIError returns an APIError if the JSON body contains a top-level "error" key. -func checkAPIError(body []byte) error { +// errorMessage returns the top-level "error" string of a JSON object body, if any. +func errorMessage(body []byte) string { + trimmed := bytes.TrimSpace(body) + if len(trimmed) == 0 || trimmed[0] != '{' { + return "" + } var envelope struct { - Error string `json:"error"` + Error any `json:"error"` } - if json.Unmarshal(body, &envelope) == nil && envelope.Error != "" { - return &clierrors.APIError{Message: envelope.Error} + if json.Unmarshal(trimmed, &envelope) != nil { + return "" } - return nil + msg, _ := envelope.Error.(string) + return msg } -// Search performs a search request. If the client has an API key it is added to params. -func (c *Client) Search(ctx context.Context, params map[string]string) (json.RawMessage, error) { +func (c *Client) getJSON(ctx context.Context, endpoint string, params map[string]string) (json.RawMessage, error) { + body, err := c.get(ctx, endpoint, params) + if err != nil { + return nil, err + } + if !json.Valid(body) { + return nil, &clierrors.APIError{Message: "Unexpected non-JSON response from server"} + } + if msg := errorMessage(body); msg != "" { + return nil, &clierrors.APIError{Message: msg, Status: http.StatusOK} + } + return json.RawMessage(body), nil +} + +func (c *Client) searchParams(params map[string]string) map[string]string { p := make(map[string]string, len(params)+1) for k, v := range params { p[k] = v @@ -171,57 +198,74 @@ func (c *Client) Search(ctx context.Context, params map[string]string) (json.Raw if c.apiKey != "" { p["api_key"] = c.apiKey } + return p +} - body, err := c.doGet(ctx, "/search.json", p) +// Search performs a search request. If the client has an API key it is added to params. +func (c *Client) Search(ctx context.Context, params map[string]string) (json.RawMessage, error) { + return c.getObject(ctx, "/search.json", c.searchParams(params)) +} + +// getObject is getJSON for endpoints that answer with a JSON object, so a +// stray null or array is an error rather than an empty-looking success. +func (c *Client) getObject(ctx context.Context, endpoint string, params map[string]string) (json.RawMessage, error) { + body, err := c.getJSON(ctx, endpoint, params) if err != nil { return nil, err } - if err := checkAPIError(body); err != nil { - return nil, err + if bytes.TrimSpace(body)[0] != '{' { + return nil, &clierrors.APIError{Message: "Unexpected response from server: not a JSON object"} } - return json.RawMessage(body), nil + return body, nil } -// Account retrieves account information. -func (c *Client) Account(ctx context.Context) (json.RawMessage, error) { - params := map[string]string{"api_key": c.apiKey} - - body, err := c.doGet(ctx, "/account.json", params) +// getRaw returns a 200 body verbatim unless it is a JSON error object, which +// SerpApi sends instead of the requested format when a search fails. +func (c *Client) getRaw(ctx context.Context, endpoint string, params map[string]string) ([]byte, error) { + body, err := c.get(ctx, endpoint, params) if err != nil { return nil, err } - if err := checkAPIError(body); err != nil { - return nil, err + if msg := errorMessage(body); msg != "" { + return nil, &clierrors.APIError{Message: msg, Status: http.StatusOK} } - return json.RawMessage(body), nil + return body, nil +} + +// SearchRaw performs a search request and returns the body verbatim, for +// non-JSON formats selected with the output parameter (for example output=md). +func (c *Client) SearchRaw(ctx context.Context, params map[string]string) ([]byte, error) { + return c.getRaw(ctx, "/search.json", c.searchParams(params)) +} + +// Account retrieves account information. +func (c *Client) Account(ctx context.Context) (json.RawMessage, error) { + return c.getObject(ctx, "/account.json", map[string]string{"api_key": c.apiKey}) } // Locations queries the locations endpoint. No API key is added. func (c *Client) Locations(ctx context.Context, params map[string]string) (json.RawMessage, error) { - body, err := c.doGet(ctx, "/locations.json", params) + body, err := c.getJSON(ctx, "/locations.json", params) if err != nil { return nil, err } - // Locations returns a JSON array; check for an error object. - if trimmed := bytes.TrimSpace(body); len(trimmed) > 0 && trimmed[0] == '{' { - if err := checkAPIError(body); err != nil { - return nil, err - } + if trimmed := bytes.TrimSpace(body); trimmed[0] != '[' { return nil, &clierrors.APIError{Message: "Unexpected response from locations endpoint"} } - return json.RawMessage(body), nil + return body, nil } -// Archive retrieves a previously cached search result by ID. +// Archive retrieves a previous search by ID. func (c *Client) Archive(ctx context.Context, id string) (json.RawMessage, error) { - params := map[string]string{"api_key": c.apiKey} + return c.getObject(ctx, archivePath(id), map[string]string{"api_key": c.apiKey}) +} - body, err := c.doGet(ctx, "/searches/"+id+".json", params) - if err != nil { - return nil, err - } - if err := checkAPIError(body); err != nil { - return nil, err - } - return json.RawMessage(body), nil +// ArchiveRaw retrieves a previous search in another output format (for +// example md) and returns the body verbatim. +func (c *Client) ArchiveRaw(ctx context.Context, id, output string) ([]byte, error) { + return c.getRaw(ctx, archivePath(id), map[string]string{"api_key": c.apiKey, "output": output}) +} + +func archivePath(id string) string { + return "/searches/" + url.PathEscape(id) + ".json" } diff --git a/pkg/api/client_test.go b/pkg/api/client_test.go index 646c9dc..c2b0c55 100644 --- a/pkg/api/client_test.go +++ b/pkg/api/client_test.go @@ -5,6 +5,7 @@ import ( "errors" "net/http" "net/http/httptest" + "net/url" "strings" "testing" "time" @@ -12,6 +13,301 @@ import ( clierrors "github.com/serpapi/serpapi-cli/pkg/errors" ) +func newTestServer(t *testing.T, handler http.HandlerFunc) *httptest.Server { + t.Helper() + srv := httptest.NewServer(handler) + t.Cleanup(srv.Close) + return srv +} + +func reply(status int, body string) http.HandlerFunc { + return func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(status) + _, _ = w.Write([]byte(body)) + } +} + +func TestSearchSendsParamsKeyAndUserAgent(t *testing.T) { + var got *http.Request + srv := newTestServer(t, func(w http.ResponseWriter, r *http.Request) { + got = r + _, _ = w.Write([]byte(`{"search_metadata":{"status":"Success"}}`)) + }) + + params := map[string]string{"engine": "google", "q": "coffee & tea"} + body, err := New("secret", WithBaseURL(srv.URL+"/")).Search(context.Background(), params) + if err != nil { + t.Fatalf("Search: %v", err) + } + if string(body) != `{"search_metadata":{"status":"Success"}}` { + t.Errorf("body = %s", body) + } + if got.URL.Path != "/search.json" { + t.Errorf("path = %q", got.URL.Path) + } + q := got.URL.Query() + if q.Get("engine") != "google" || q.Get("q") != "coffee & tea" || q.Get("api_key") != "secret" { + t.Errorf("query = %v", q) + } + if !strings.HasPrefix(got.UserAgent(), "serpapi-go-cli/") { + t.Errorf("User-Agent = %q", got.UserAgent()) + } + if _, ok := params["api_key"]; ok { + t.Error("Search mutated the caller's params") + } +} + +func TestSearchWithoutKeyOmitsAPIKey(t *testing.T) { + srv := newTestServer(t, func(w http.ResponseWriter, r *http.Request) { + if r.URL.Query().Has("api_key") { + t.Errorf("unexpected api_key in %s", r.URL.RawQuery) + } + _, _ = w.Write([]byte(`{}`)) + }) + if _, err := New("", WithBaseURL(srv.URL)).Search(context.Background(), map[string]string{"q": "x"}); err != nil { + t.Fatal(err) + } +} + +func TestHTTPErrorsCarryStatus(t *testing.T) { + cases := []struct { + name string + status int + body string + message string + }{ + {"json error message", 401, `{"error":"Invalid API key."}`, "Invalid API key."}, + {"rate limit", 429, `{"error":"Your account has run out of searches."}`, "Your account has run out of searches."}, + {"html error page", 500, `oops`, "HTTP 500: Internal Server Error"}, + {"empty body", 503, ``, "HTTP 503: Service Unavailable"}, + {"redirect is not followed", 302, ``, "HTTP 302: Found"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + srv := newTestServer(t, func(w http.ResponseWriter, r *http.Request) { + if tc.status == http.StatusFound { + w.Header().Set("Location", "/elsewhere") + } + if r.URL.Path == "/elsewhere" { + t.Error("redirect was followed") + } + w.WriteHeader(tc.status) + _, _ = w.Write([]byte(tc.body)) + }) + _, err := New("k", WithBaseURL(srv.URL)).Search(context.Background(), nil) + var apiErr *clierrors.APIError + if !errors.As(err, &apiErr) { + t.Fatalf("want APIError, got %T %v", err, err) + } + if apiErr.Message != tc.message || apiErr.Status != tc.status { + t.Errorf("got (%q, %d), want (%q, %d)", apiErr.Message, apiErr.Status, tc.message, tc.status) + } + }) + } +} + +func TestErrorInsideOKResponse(t *testing.T) { + srv := newTestServer(t, reply(200, `{"search_metadata":{"status":"Success"},"error":"Google hasn't returned any results for this query."}`)) + _, err := New("k", WithBaseURL(srv.URL)).Search(context.Background(), nil) + var apiErr *clierrors.APIError + if !errors.As(err, &apiErr) { + t.Fatalf("want APIError, got %v", err) + } + if apiErr.Status != http.StatusOK || apiErr.Message != "Google hasn't returned any results for this query." { + t.Errorf("got (%q, %d)", apiErr.Message, apiErr.Status) + } +} + +func TestNonObjectErrorFieldIsData(t *testing.T) { + srv := newTestServer(t, reply(200, `{"error":{"nested":true}}`)) + if _, err := New("k", WithBaseURL(srv.URL)).Search(context.Background(), nil); err != nil { + t.Errorf("non-string error field should pass through, got %v", err) + } +} + +func TestInvalidJSONResponse(t *testing.T) { + for _, body := range []string{`maintenance`, `{"truncated":`, ``} { + srv := newTestServer(t, reply(200, body)) + _, err := New("k", WithBaseURL(srv.URL)).Search(context.Background(), nil) + var apiErr *clierrors.APIError + if !errors.As(err, &apiErr) || apiErr.Message != "Unexpected non-JSON response from server" || apiErr.Status != 0 { + t.Errorf("body %q: got %v", body, err) + } + } +} + +func TestResponseSizeLimit(t *testing.T) { + old := maxResponseBytes + maxResponseBytes = 16 + t.Cleanup(func() { maxResponseBytes = old }) + + srv := newTestServer(t, reply(200, `{"a":"0123456789"}`)) // 18 bytes + _, err := New("k", WithBaseURL(srv.URL)).Search(context.Background(), nil) + var apiErr *clierrors.APIError + if !errors.As(err, &apiErr) || !strings.Contains(apiErr.Message, "exceeded") || apiErr.Status != 0 { + t.Fatalf("want size error, got %v", err) + } + + srv = newTestServer(t, reply(200, `{"a":"012345678"}`)) // 17 bytes, over by one + if _, err := New("k", WithBaseURL(srv.URL)).Search(context.Background(), nil); err == nil { + t.Fatal("want size error for a body one byte over the limit") + } + + srv = newTestServer(t, reply(200, `{"a":"01234567"}`)) // exactly 16 bytes + if _, err := New("k", WithBaseURL(srv.URL)).Search(context.Background(), nil); err != nil { + t.Fatalf("body at the limit should pass: %v", err) + } + + srv = newTestServer(t, reply(503, `{"error":"0123456789"}`)) // over the limit + _, err = New("k", WithBaseURL(srv.URL)).Search(context.Background(), nil) + if !errors.As(err, &apiErr) || apiErr.Status != 503 { + t.Fatalf("an oversized error response should keep its status, got %v", err) + } +} + +func TestNetworkErrorHidesURLAndKey(t *testing.T) { + srv := httptest.NewServer(http.NotFoundHandler()) + base := srv.URL + srv.Close() + + _, err := New("secret-key-123", WithBaseURL(base)).Search(context.Background(), map[string]string{"q": "coffee"}) + var netErr *clierrors.NetworkError + if !errors.As(err, &netErr) { + t.Fatalf("want NetworkError, got %T %v", err, err) + } + for _, leak := range []string{"secret-key-123", "api_key", "q=coffee"} { + if strings.Contains(err.Error(), leak) { + t.Errorf("error leaks %q: %v", leak, err) + } + } + if !strings.Contains(netErr.Message, "/search.json") { + t.Errorf("message should name the endpoint: %q", netErr.Message) + } +} + +func TestTimeout(t *testing.T) { + release := make(chan struct{}) + srv := newTestServer(t, func(w http.ResponseWriter, r *http.Request) { + select { + case <-release: + case <-r.Context().Done(): + } + }) + defer close(release) + + _, err := New("secret", WithBaseURL(srv.URL), WithTimeout(50*time.Millisecond)).Search(context.Background(), nil) + var netErr *clierrors.NetworkError + if !errors.As(err, &netErr) { + t.Fatalf("want NetworkError, got %T %v", err, err) + } + if !strings.Contains(netErr.Message, "timed out after 50ms") || !strings.Contains(netErr.Message, "--timeout") { + t.Errorf("message = %q", netErr.Message) + } + if strings.Contains(err.Error(), "secret") { + t.Errorf("error leaks key: %v", err) + } +} + +func TestSearchRaw(t *testing.T) { + srv := newTestServer(t, func(w http.ResponseWriter, r *http.Request) { + if r.URL.Query().Get("output") != "html" { + t.Errorf("output param not forwarded: %s", r.URL.RawQuery) + } + _, _ = w.Write([]byte("results")) + }) + raw, err := New("k", WithBaseURL(srv.URL)).SearchRaw(context.Background(), map[string]string{"output": "html"}) + if err != nil { + t.Fatal(err) + } + if string(raw) != "results" { + t.Errorf("raw = %q", raw) + } +} + +func TestSearchRawErrors(t *testing.T) { + srv := newTestServer(t, reply(200, `{"error":"Unsupported output."}`)) + _, err := New("k", WithBaseURL(srv.URL)).SearchRaw(context.Background(), nil) + var apiErr *clierrors.APIError + if !errors.As(err, &apiErr) || apiErr.Message != "Unsupported output." { + t.Errorf("want APIError from JSON error body, got %v", err) + } + + srv = newTestServer(t, reply(401, `{"error":"Invalid API key."}`)) + _, err = New("k", WithBaseURL(srv.URL)).SearchRaw(context.Background(), nil) + if !errors.As(err, &apiErr) || apiErr.Status != 401 { + t.Errorf("want APIError with status 401, got %v", err) + } +} + +func TestLocations(t *testing.T) { + srv := newTestServer(t, func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/locations.json" || r.URL.Query().Has("api_key") { + t.Errorf("unexpected request %s", r.URL) + } + _, _ = w.Write([]byte(`[{"name":"Austin"}]`)) + }) + body, err := New("secret", WithBaseURL(srv.URL)).Locations(context.Background(), map[string]string{"q": "austin"}) + if err != nil || string(body) != `[{"name":"Austin"}]` { + t.Fatalf("got %s, %v", body, err) + } + + for body, want := range map[string]string{ + `{"error":"Missing query."}`: "Missing query.", + `{"unexpected":true}`: "Unexpected response from locations endpoint", + } { + srv := newTestServer(t, reply(200, body)) + _, err := New("", WithBaseURL(srv.URL)).Locations(context.Background(), nil) + var apiErr *clierrors.APIError + if !errors.As(err, &apiErr) || apiErr.Message != want { + t.Errorf("body %s: got %v, want %q", body, err, want) + } + } +} + +func TestAccountAndArchiveRequests(t *testing.T) { + var paths []string + srv := newTestServer(t, func(w http.ResponseWriter, r *http.Request) { + paths = append(paths, r.URL.Path+"?"+r.URL.RawQuery) + _, _ = w.Write([]byte(`{}`)) + }) + c := New("secret", WithBaseURL(srv.URL)) + if _, err := c.Account(context.Background()); err != nil { + t.Fatal(err) + } + if _, err := c.Archive(context.Background(), "abc_123-x"); err != nil { + t.Fatal(err) + } + if body, err := c.ArchiveRaw(context.Background(), "abc_123-x", "rank:example.com"); err != nil || string(body) != "{}" { + t.Fatalf("ArchiveRaw = %q, %v", body, err) + } + want := []string{ + "/account.json?api_key=secret", + "/searches/abc_123-x.json?api_key=secret", + "/searches/abc_123-x.json?api_key=secret&output=rank%3Aexample.com", + } + if strings.Join(paths, " ") != strings.Join(want, " ") { + t.Errorf("paths = %v, want %v", paths, want) + } +} + +func TestObjectEndpointsRejectOtherJSON(t *testing.T) { + for _, body := range []string{`null`, `[{"position":1}]`, `"text"`, `42`} { + srv := newTestServer(t, reply(200, body)) + c := New("k", WithBaseURL(srv.URL)) + calls := map[string]func() error{ + "search": func() error { _, err := c.Search(context.Background(), nil); return err }, + "account": func() error { _, err := c.Account(context.Background()); return err }, + "archive": func() error { _, err := c.Archive(context.Background(), "abc"); return err }, + } + for name, call := range calls { + var apiErr *clierrors.APIError + if err := call(); !errors.As(err, &apiErr) || apiErr.Message != "Unexpected response from server: not a JSON object" || apiErr.Status != 0 { + t.Errorf("%s with body %s: got %v", name, body, err) + } + } + } +} + // newSlowServer returns a client pointed at a server that waits for `delay` // (or until the request is cancelled) before answering with a tiny JSON body. func newSlowServer(t *testing.T, delay time.Duration, timeout time.Duration) *Client { @@ -95,3 +391,36 @@ func TestCallerCancellationIsNotReportedAsTimeout(t *testing.T) { t.Errorf("caller cancellation should not be reported as a timeout: %q", netErr.Message) } } + +type osTimeout struct{} + +func (osTimeout) Error() string { return "i/o timeout" } +func (osTimeout) Timeout() bool { return true } +func (osTimeout) Temporary() bool { return true } + +// With --timeout 0 there is no client limit to raise, so an operating +// system timeout, such as a TCP connect timeout, is reported as is. +func TestOSTimeoutWithoutAClientLimit(t *testing.T) { + c := New("secret", WithTimeout(0)) + err := c.networkError("/search.json", &url.Error{Op: "Get", URL: "https://serpapi.com/search.json?api_key=secret", Err: osTimeout{}}) + var netErr *clierrors.NetworkError + if !errors.As(err, &netErr) || netErr.Message != "Request to /search.json failed: i/o timeout" { + t.Errorf("got %v, want the OS timeout without --timeout advice", err) + } +} + +// The advice names a --timeout above the one that just ran out. +func TestTimeoutAdviceRaisesTheLimit(t *testing.T) { + for limit, want := range map[time.Duration]string{ + 50 * time.Millisecond: "timed out after 50ms. Slow engines, such as google_ai_mode, and no_cache=true can take longer: retry with a higher --timeout, such as --timeout 120,", + 60 * time.Second: "timed out after 60s. Slow engines, such as google_ai_mode, and no_cache=true can take longer: retry with a higher --timeout, such as --timeout 120,", + 120 * time.Second: "--timeout 240,", + 301 * time.Second: "--timeout 602,", + } { + err := New("secret", WithTimeout(limit)).networkError("/search.json", osTimeout{}) + var netErr *clierrors.NetworkError + if !errors.As(err, &netErr) || !strings.Contains(netErr.Message, want) { + t.Errorf("limit %s: got %v, want it to contain %q", limit, err, want) + } + } +} From 94c3bc12ec13b34115c0cac0954fb2a41b9e7c36 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Mon, 12 Oct 2026 00:32:56 +0200 Subject: [PATCH 10/25] [Params] Join bare words into q and refuse parameters that carry a key Words without = are joined into one q, so serpapi search engine=google coffee shops searches for "coffee shops" instead of "shops"; mixing them with q=... is a usage error. Every parameter SerpApi reads a key from (api_key, apikey, key, serpapi_key, serp_api_key) is refused, so the key never lands in params or shell history. ApplyFields drops empty json_restrictor segments, which SerpApi answers with the whole response, and can append paths such as error that must survive the filter; Repeated lists parameters given twice. Both are used by the search command later in this stack. Co-Authored-By: Claude Opus 5.5 --- pkg/params/params.go | 104 +++++++++++++++++++++----- pkg/params/params_test.go | 149 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 236 insertions(+), 17 deletions(-) diff --git a/pkg/params/params.go b/pkg/params/params.go index 74c1ef8..2525194 100644 --- a/pkg/params/params.go +++ b/pkg/params/params.go @@ -1,6 +1,7 @@ package params import ( + "slices" "strings" clierrors "github.com/serpapi/serpapi-cli/pkg/errors" @@ -12,48 +13,88 @@ type Param struct { Value string } +// keyParams are the query parameters SerpApi reads an API key from. +var keyParams = map[string]bool{ + "api_key": true, + "apikey": true, + "key": true, + "serp_api_key": true, + "serpapi_key": true, +} + // ParseParam parses a CLI parameter string into a Param. -// Bare words (no "=") are treated as q=. +// A bare word (no "=") is treated as q=. func ParseParam(s string) (Param, error) { if s == "" { return Param{}, &clierrors.UsageError{Message: "Empty parameter"} } - idx := strings.IndexByte(s, '=') - if idx < 0 { - // Bare word → q= + key, value, found := strings.Cut(s, "=") + if !found { return Param{Key: "q", Value: s}, nil } - - key := s[:idx] - value := s[idx+1:] - if key == "" { - return Param{}, &clierrors.UsageError{Message: "Empty parameter key"} + return Param{}, &clierrors.UsageError{Message: "Empty parameter key in " + s} } - if key == "api_key" { + if keyParams[key] { return Param{}, &clierrors.UsageError{ - Message: "Use --api-key or SERPAPI_KEY instead of passing api_key as a parameter", + Message: "Pass the API key with --api-key or SERPAPI_KEY, not as the " + key + " parameter", } } - return Param{Key: key, Value: value}, nil } -// ParseAll parses a slice of CLI argument strings into Params. +// ParseAll parses CLI arguments into Params. Bare words are joined with spaces +// into one q parameter, so `coffee shops` searches for "coffee shops". func ParseAll(args []string) ([]Param, error) { parsed := make([]Param, 0, len(args)) + query, explicitQuery := -1, false for _, s := range args { p, err := ParseParam(s) if err != nil { return nil, err } - parsed = append(parsed, p) + if strings.Contains(s, "=") { + explicitQuery = explicitQuery || p.Key == "q" + parsed = append(parsed, p) + } else if query >= 0 { + parsed[query].Value += " " + p.Value + } else { + query = len(parsed) + parsed = append(parsed, p) + } + } + if query >= 0 && explicitQuery { + return nil, &clierrors.UsageError{ + Message: "Got both q=... and bare words (" + parsed[query].Value + "); quote the query as one q=... parameter", + } } return parsed, nil } -// ParamsToMap converts a slice of Params to a map. +// Repeated returns the last occurrence of each key given more than once with +// different values, in the order the keys first appear. +func Repeated(params []Param) []Param { + first := make(map[string]string, len(params)) + last := make(map[string]string, len(params)) + var order []string + for _, p := range params { + v, seen := first[p.Key] + if !seen { + first[p.Key] = p.Value + } else if v != p.Value && !slices.Contains(order, p.Key) { + order = append(order, p.Key) + } + last[p.Key] = p.Value + } + repeated := make([]Param, 0, len(order)) + for _, k := range order { + repeated = append(repeated, Param{Key: k, Value: last[k]}) + } + return repeated +} + +// ParamsToMap converts a slice of Params to a map. A repeated key keeps its last value. func ParamsToMap(params []Param) map[string]string { m := make(map[string]string, len(params)) for _, p := range params { @@ -62,9 +103,38 @@ func ParamsToMap(params []Param) map[string]string { return m } -// ApplyFields sets the json_restrictor parameter if fields is non-empty. -func ApplyFields(params map[string]string, fields string) { +// ApplyFields sets json_restrictor from fields, which overrides a +// json_restrictor parameter, and returns the restrictor in effect ("" for none). +// Empty segments are dropped because SerpApi answers them with the whole +// response, and the extra paths (such as "error") are appended so that a +// pruned response keeps them. +func ApplyFields(params map[string]string, fields string, extra ...string) string { if fields != "" { params["json_restrictor"] = fields } + restrictor, ok := params["json_restrictor"] + if !ok { + return "" + } + + var segments []string + present := make(map[string]bool) + for _, s := range strings.Split(restrictor, ",") { + if trimmed := strings.TrimSpace(s); trimmed != "" { + segments = append(segments, s) + present[trimmed] = true + } + } + if len(segments) == 0 { + delete(params, "json_restrictor") + return "" + } + for _, path := range extra { + if !present[path] { + segments = append(segments, path) + } + } + restrictor = strings.TrimSpace(strings.Join(segments, ",")) + params["json_restrictor"] = restrictor + return restrictor } diff --git a/pkg/params/params_test.go b/pkg/params/params_test.go index 8085b96..b953da5 100644 --- a/pkg/params/params_test.go +++ b/pkg/params/params_test.go @@ -1,6 +1,9 @@ package params import ( + "errors" + "fmt" + "strings" "testing" clierrors "github.com/serpapi/serpapi-cli/pkg/errors" @@ -55,3 +58,149 @@ func TestParseParamEmpty(t *testing.T) { t.Errorf("expected UsageError, got %T", err) } } + +func TestParseParamRejectsAPIKeyAliases(t *testing.T) { + for _, key := range []string{"api_key", "apikey", "key", "serp_api_key", "serpapi_key"} { + _, err := ParseParam(key + "=secret") + var usageErr *clierrors.UsageError + if !errors.As(err, &usageErr) || strings.Contains(usageErr.Message, "secret") { + t.Errorf("%s: got %v, want UsageError without the value", key, err) + } + } +} + +func TestParseAllJoinsBareWordsIntoQuery(t *testing.T) { + got, err := ParseAll([]string{"engine=google", "coffee", "shops", "location=Austin", "open"}) + if err != nil { + t.Fatal(err) + } + want := []Param{{"engine", "google"}, {"q", "coffee shops open"}, {"location", "Austin"}} + if fmt.Sprint(got) != fmt.Sprint(want) { + t.Errorf("got %v, want %v", got, want) + } +} + +func TestParseAllRejectsBareWordsWithExplicitQuery(t *testing.T) { + _, err := ParseAll([]string{"q=coffee", "shops"}) + var usageErr *clierrors.UsageError + if !errors.As(err, &usageErr) { + t.Fatalf("got %v, want UsageError", err) + } +} + +func TestRepeated(t *testing.T) { + tests := []struct { + args []string + want []Param + }{ + {[]string{"engine=google", "q=a"}, []Param{}}, + {[]string{"engine=google", "engine=google"}, []Param{}}, + {[]string{"q=a", "gl=us", "q=b", "gl=uk", "q=c"}, []Param{{"q", "c"}, {"gl", "uk"}}}, + {[]string{"tbs=x", "tbs=y", "tbs=x"}, []Param{{"tbs", "x"}}}, + {[]string{"location=", "location=Austin"}, []Param{{"location", "Austin"}}}, + } + for _, tt := range tests { + parsed, err := ParseAll(tt.args) + if err != nil { + t.Fatal(err) + } + if got := Repeated(parsed); fmt.Sprint(got) != fmt.Sprint(tt.want) { + t.Errorf("Repeated(%v) = %v, want %v", tt.args, got, tt.want) + } + } +} + +func TestApplyFields(t *testing.T) { + tests := []struct { + name, fields, param string + extra []string + want string + }{ + {name: "none"}, + {name: "flag", fields: "organic_results[].{title, link}", extra: []string{"error"}, + want: "organic_results[].{title, link},error"}, + {name: "param", param: "organic_results", extra: []string{"error"}, want: "organic_results,error"}, + {name: "flag overrides param", fields: "a", param: "b", want: "a"}, + {name: "empty segments dropped", fields: " a,, b ,", extra: []string{"error"}, want: "a, b ,error"}, + {name: "only commas", fields: ", ,", extra: []string{"error"}}, + {name: "extra already present", fields: "a, error", extra: []string{"error", "serpapi_pagination.next"}, + want: "a, error,serpapi_pagination.next"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := map[string]string{} + if tt.param != "" { + p["json_restrictor"] = tt.param + } + got := ApplyFields(p, tt.fields, tt.extra...) + if got != tt.want || p["json_restrictor"] != tt.want { + t.Errorf("got %q (param %q), want %q", got, p["json_restrictor"], tt.want) + } + if _, ok := p["json_restrictor"]; ok != (tt.want != "") { + t.Errorf("json_restrictor presence = %v", ok) + } + }) + } +} + +func FuzzParseAll(f *testing.F) { + f.Add("engine=google", "coffee", "shops") + f.Add("q=a=b", "api_key=x", "") + f.Add("=x", "key=", "café ☕") + f.Fuzz(func(t *testing.T, a, b, c string) { + args := []string{a, b, c} + parsed, err := ParseAll(args) + if err != nil { + return + } + for _, p := range parsed { + if keyParams[p.Key] { + t.Fatalf("ParseAll(%q) kept key parameter %q", args, p.Key) + } + } + if !strings.Contains(a+b+c, "=") && (len(parsed) != 1 || parsed[0] != (Param{"q", strings.Join(args, " ")})) { + t.Fatalf("ParseAll(%q) = %v, want one q of the joined words", args, parsed) + } + }) +} + +func FuzzApplyFields(f *testing.F) { + f.Add("organic_results[0:3]", "", "error") + f.Add(" , ,", "search_metadata", "serpapi_pagination.next") + f.Add("", "a,,b ,", "a") + f.Fuzz(func(t *testing.T, fields, existing, extra string) { + if strings.Contains(extra, ",") || strings.TrimSpace(extra) != extra || extra == "" { + return + } + p := map[string]string{} + if existing != "" { + p["json_restrictor"] = existing + } + got := ApplyFields(p, fields, extra) + if got == "" { + if _, ok := p["json_restrictor"]; ok { + t.Fatalf("no restrictor in effect, but json_restrictor=%q is still sent", p["json_restrictor"]) + } + return + } + if p["json_restrictor"] != got { + t.Fatalf("returned %q but sends %q", got, p["json_restrictor"]) + } + count := 0 + for _, s := range strings.Split(got, ",") { + if strings.TrimSpace(s) == "" { + t.Fatalf("restrictor %q has an empty segment", got) + } + if strings.TrimSpace(s) == extra { + count++ + } + } + if count == 0 { + t.Fatalf("restrictor %q lost %q", got, extra) + } + again := map[string]string{"json_restrictor": got} + if twice := ApplyFields(again, "", extra); twice != got { + t.Fatalf("not idempotent: %q then %q", got, twice) + } + }) +} From b01b6eeb920a0c232e93bd823becb4ee86c580d6 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Mon, 12 Oct 2026 00:33:00 +0200 Subject: [PATCH 11/25] [JQ] Compile filters up front and stream their results Compile parses and compiles a filter, so a later commit can reject a typo as a usage error before a billed search runs. Run passes each result to a callback as gojq produces it and stops when the context is canceled; halt now stops output instead of being reported as an error. Apply is built on both. Co-Authored-By: Claude Opus 5.5 --- pkg/jq/jq.go | 48 +++++++++++++++++++++++---- pkg/jq/jq_test.go | 84 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 125 insertions(+), 7 deletions(-) diff --git a/pkg/jq/jq.go b/pkg/jq/jq.go index f28cc05..9f406ef 100644 --- a/pkg/jq/jq.go +++ b/pkg/jq/jq.go @@ -1,34 +1,68 @@ package jq import ( + "context" + "errors" "fmt" "github.com/itchyny/gojq" ) -// Apply runs a jq expression against the input and returns all output values. -func Apply(expression string, input any) ([]any, error) { +// Compile parses and compiles a jq expression, so a typo can be reported +// before a billed search runs. +func Compile(expression string) (*gojq.Code, error) { query, err := gojq.Parse(expression) if err != nil { return nil, fmt.Errorf("jq parse error: %w", err) } - code, err := gojq.Compile(query) if err != nil { return nil, fmt.Errorf("jq compile error: %w", err) } + return code, nil +} - var results []any - iter := code.Run(input) +// Run evaluates code against input and passes each result to emit as soon as +// it is produced, so memory stays flat however many results there are. halt +// ends the output quietly; another runtime error is returned after the +// results before it were emitted. An error from emit stops the run and is +// returned as is, and so is ctx.Err() once ctx is done. +func Run(ctx context.Context, code *gojq.Code, input any, emit func(any) error) error { + iter := code.RunWithContext(ctx, input) for { v, ok := iter.Next() if !ok { - break + return nil } if err, isErr := v.(error); isErr { - return nil, fmt.Errorf("jq runtime error: %w", err) + if ctx.Err() != nil && errors.Is(err, ctx.Err()) { + return ctx.Err() + } + var halt *gojq.HaltError + if errors.As(err, &halt) && halt.Value() == nil { + return nil + } + return fmt.Errorf("jq runtime error: %w", err) + } + if err := emit(v); err != nil { + return err } + } +} + +// Apply runs a jq expression against the input and returns all output values. +func Apply(expression string, input any) ([]any, error) { + code, err := Compile(expression) + if err != nil { + return nil, err + } + var results []any + err = Run(context.Background(), code, input, func(v any) error { results = append(results, v) + return nil + }) + if err != nil { + return nil, err } return results, nil } diff --git a/pkg/jq/jq_test.go b/pkg/jq/jq_test.go index 0e9c18f..ed14bfa 100644 --- a/pkg/jq/jq_test.go +++ b/pkg/jq/jq_test.go @@ -1,7 +1,11 @@ package jq import ( + "context" + "errors" + "strings" "testing" + "time" ) func TestApplyIdentity(t *testing.T) { @@ -68,3 +72,83 @@ func TestApplyInvalidExpression(t *testing.T) { t.Fatal("expected error for invalid expression") } } + +func TestApplyHaltStopsWithoutError(t *testing.T) { + results, err := Apply("1, halt, 2", nil) + if err != nil { + t.Fatal(err) + } + if len(results) != 1 || results[0] != 1 { + t.Errorf("results = %v, want [1]", results) + } +} + +func TestApplyHaltErrorFails(t *testing.T) { + if _, err := Apply(`"boom" | halt_error`, nil); err == nil { + t.Error("halt_error should be an error") + } +} + +func TestCompileReportsParseAndCompileErrors(t *testing.T) { + if _, err := Compile(".organic_results[].title"); err != nil { + t.Fatalf("valid expression: %v", err) + } + if _, err := Compile(".["); err == nil || !strings.Contains(err.Error(), "jq parse error") { + t.Errorf("want parse error, got %v", err) + } + if _, err := Compile("nosuchfn(1)"); err == nil || !strings.Contains(err.Error(), "jq compile error") { + t.Errorf("want compile error, got %v", err) + } +} + +func TestRunEmitsEachResultBeforeAnError(t *testing.T) { + code, err := Compile(`.[] | . + 1`) + if err != nil { + t.Fatal(err) + } + var got []any + err = Run(context.Background(), code, []any{1, 2, "x"}, func(v any) error { + got = append(got, v) + return nil + }) + if len(got) != 2 || err == nil || !strings.Contains(err.Error(), "cannot add") { + t.Errorf("got %v, %v; want [2 3] then a runtime error", got, err) + } +} + +func TestRunStopsEndlessOutputWhenEmitFails(t *testing.T) { + code, err := Compile(`repeat(1)`) + if err != nil { + t.Fatal(err) + } + stop := errors.New("stop") + n := 0 + err = Run(context.Background(), code, nil, func(any) error { + if n++; n == 1000 { + return stop + } + return nil + }) + if err != stop || n != 1000 { + t.Errorf("Run returned %v after %d results; want the emit error after 1000", err, n) + } +} + +// A filter that computes without printing, such as a large reduce, must +// still stop on Ctrl-C. +func TestRunStopsWhenTheContextIsCanceled(t *testing.T) { + code, err := Compile(`reduce range(1e12) as $i (0; . + 1)`) + if err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) + defer cancel() + start := time.Now() + err = Run(ctx, code, nil, func(any) error { return nil }) + if !errors.Is(err, context.DeadlineExceeded) { + t.Errorf("Run returned %v, want the context's error", err) + } + if took := time.Since(start); took > 2*time.Second { + t.Errorf("Run took %s to notice the canceled context", took) + } +} From c61f879e63763319b73ad8ecbc15dc5f0ebab214 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Mon, 12 Oct 2026 00:33:26 +0200 Subject: [PATCH 12/25] [Config] Keep the old key until the new one is saved; name a broken file os.Rename replaces the config file atomically on Windows too, so the old key is no longer deleted first and lost when the rename fails. A config.toml that isn't valid TOML, or can't be read, came out as "No API key found": account and archive now name the file and the line, and login rewrites the file. TOML parse errors quote part of the value, which may be the key, so only the line number is shown. The missing-key error says where a key can come from and where to get one. LoadAPIKey returns "" without an error when there is no file, so login and search keep treating a broken file as no key. Co-Authored-By: Claude Opus 5.5 --- pkg/cmd/login.go | 2 +- pkg/cmd/root.go | 2 +- pkg/config/config.go | 58 +++++++++++++++++++++++++-------------- pkg/config/config_test.go | 49 +++++++++++++++++++++++++++++---- 4 files changed, 83 insertions(+), 28 deletions(-) diff --git a/pkg/cmd/login.go b/pkg/cmd/login.go index dcdcc91..7c1d22d 100644 --- a/pkg/cmd/login.go +++ b/pkg/cmd/login.go @@ -28,7 +28,7 @@ func init() { func runLogin(cmd *cobra.Command, args []string) error { // Check if already authenticated - if existingKey, ok := config.LoadAPIKey(); ok { + if existingKey, err := config.LoadAPIKey(); err == nil && existingKey != "" { client, err := newClient(existingKey) if err != nil { return err diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index 62f6efb..18208f7 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -80,7 +80,7 @@ func resolveAPIKeyOptional() string { if key != "" { return key } - if k, ok := config.LoadAPIKey(); ok { + if k, err := config.LoadAPIKey(); err == nil && k != "" { return k } return "" diff --git a/pkg/config/config.go b/pkg/config/config.go index 4deb20c..8825d23 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -2,8 +2,12 @@ package config import ( "bytes" + "errors" + "fmt" + "io/fs" "os" "path/filepath" + "strings" "github.com/BurntSushi/toml" clierrors "github.com/serpapi/serpapi-cli/pkg/errors" @@ -30,20 +34,34 @@ func Path() string { return filepath.Join(Dir(), "config.toml") } -// LoadAPIKey reads the API key from the config file, if present. -func LoadAPIKey() (string, bool) { +// ErrNoAPIKey is returned by ResolveAPIKey when no source has a key. +var ErrNoAPIKey = &clierrors.UsageError{ + Message: "No API key found. Pass --api-key, set SERPAPI_KEY, or run 'serpapi login'. Get a key at https://serpapi.com/manage-api-key", +} + +// LoadAPIKey returns the API key saved in the config file, or "" when there +// is none. A file that can't be read or parsed is an error naming the file +// and line; it never quotes the file, which holds the key. +func LoadAPIKey() (string, error) { data, err := os.ReadFile(Path()) + if errors.Is(err, fs.ErrNotExist) { + return "", nil + } if err != nil { - return "", false + return "", &clierrors.UsageError{Message: "Cannot read the config file: " + err.Error() + + "; fix its permissions, or pass --api-key or SERPAPI_KEY"} } var cfg configFile if err := toml.Unmarshal(data, &cfg); err != nil { - return "", false - } - if cfg.APIKey == "" { - return "", false + where := "" + var parseErr toml.ParseError + if errors.As(err, &parseErr) && parseErr.Position.Line > 0 { + where = fmt.Sprintf(" (line %d)", parseErr.Position.Line) + } + return "", &clierrors.UsageError{Message: "The config file " + Path() + " is not valid TOML" + where + + "; run 'serpapi login' to rewrite it, or pass --api-key or SERPAPI_KEY"} } - return cfg.APIKey, true + return strings.TrimSpace(cfg.APIKey), nil } // SaveConfig writes the API key to the config file atomically. @@ -66,12 +84,8 @@ func SaveConfig(apiKey string) error { return &clierrors.UsageError{Message: err.Error()} } - // os.Remove before os.Rename is needed for Windows, which cannot - // atomically rename over an existing file. On Unix, os.Rename is atomic. - if err := os.Remove(finalPath); err != nil && !os.IsNotExist(err) { - os.Remove(tmpPath) - return &clierrors.UsageError{Message: "Failed to save config: " + err.Error()} - } + // os.Rename replaces an existing file on Windows too (MoveFileEx with + // MOVEFILE_REPLACE_EXISTING), so the old key stays until the new one lands. if err := os.Rename(tmpPath, finalPath); err != nil { os.Remove(tmpPath) return &clierrors.UsageError{Message: "Failed to save config: " + err.Error()} @@ -89,16 +103,20 @@ func tomlMarshal(cfg configFile) ([]byte, error) { return buf.Bytes(), nil } -// ResolveAPIKey resolves the API key from the given flag value (which may -// already incorporate the env var), falling back to the config file. +// ResolveAPIKey returns the given key (from a flag or environment variable) +// or, when it is empty, the key saved in the config file. It returns +// ErrNoAPIKey when there is none, and the config file's error when the +// saved key is needed but the file is broken. func ResolveAPIKey(fromFlag string) (string, error) { if fromFlag != "" { return fromFlag, nil } - if key, ok := LoadAPIKey(); ok { - return key, nil + key, err := LoadAPIKey() + if err != nil { + return "", err } - return "", &clierrors.UsageError{ - Message: "No API key found. Run 'serpapi login' or set SERPAPI_KEY.", + if key == "" { + return "", ErrNoAPIKey } + return key, nil } diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index 5d946fb..664163f 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -1,13 +1,28 @@ package config import ( + "errors" + "os" + "path/filepath" + "runtime" "strings" "testing" clierrors "github.com/serpapi/serpapi-cli/pkg/errors" ) +// isolate points the user config directory at a temporary directory so tests +// never read or overwrite the real config. +func isolate(t *testing.T) { + t.Helper() + dir := t.TempDir() + t.Setenv("HOME", dir) + t.Setenv("XDG_CONFIG_HOME", filepath.Join(dir, ".config")) + t.Setenv("AppData", dir) +} + func TestConfigPathContainsSerpapi(t *testing.T) { + isolate(t) path := Path() if !strings.Contains(path, "serpapi") { t.Errorf("config path %q does not contain 'serpapi'", path) @@ -15,6 +30,7 @@ func TestConfigPathContainsSerpapi(t *testing.T) { } func TestResolveAPIKeyFromFlag(t *testing.T) { + isolate(t) key, err := ResolveAPIKey("test-key-123") if err != nil { t.Fatal(err) @@ -25,13 +41,34 @@ func TestResolveAPIKeyFromFlag(t *testing.T) { } func TestResolveAPIKeyMissingReturnsError(t *testing.T) { - // With empty flag and no config file, should return error + isolate(t) _, err := ResolveAPIKey("") - if err == nil { - // It might succeed if there's a real config file; skip in that case - t.Skip("config file exists, cannot test missing key") + var usageErr *clierrors.UsageError + if !errors.As(err, &usageErr) || !strings.Contains(usageErr.Message, "No API key") { + t.Errorf("expected UsageError about the missing key, got %v", err) + } +} + +func TestSaveConfigRoundTripAndOverwrite(t *testing.T) { + isolate(t) + for _, key := range []string{"first-key", "second-key"} { + if err := SaveConfig(key); err != nil { + t.Fatal(err) + } + got, err := ResolveAPIKey("") + if err != nil || got != key { + t.Fatalf("ResolveAPIKey = %q, %v; want %q", got, err, key) + } + } + + info, err := os.Stat(Path()) + if err != nil { + t.Fatal(err) + } + if runtime.GOOS != "windows" && info.Mode().Perm() != 0o600 { + t.Errorf("config mode = %v, want 0600", info.Mode().Perm()) } - if _, ok := err.(*clierrors.UsageError); !ok { - t.Errorf("expected UsageError, got %T", err) + if _, err := os.Stat(Path() + ".tmp"); !os.IsNotExist(err) { + t.Errorf("temporary file left behind: %v", err) } } From a36c3a1bf0ce7d9e0d166bc53c627ddf62c0d76e Mon Sep 17 00:00:00 2001 From: ilyazub Date: Mon, 12 Oct 2026 00:33:30 +0200 Subject: [PATCH 13/25] [Version] Report the module version for go install builds Release builds still set the version with -ldflags; go install builds now report the module version instead of dev, in --version and the User-Agent. Co-Authored-By: Claude Opus 5.5 --- pkg/version/version.go | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/pkg/version/version.go b/pkg/version/version.go index 2076ecc..d81ec7c 100644 --- a/pkg/version/version.go +++ b/pkg/version/version.go @@ -1,4 +1,23 @@ +// Package version holds the CLI version. package version -// Version is set at build time via -ldflags. +import ( + "runtime/debug" + "strings" +) + +// Version is set at build time with +// -ldflags "-X github.com/serpapi/serpapi-cli/pkg/version.Version=1.2.3". +// Other builds, such as go install, report the module version instead. var Version = "dev" + +func init() { + if Version != "dev" { + return + } + if info, ok := debug.ReadBuildInfo(); ok { + if v := info.Main.Version; v != "" && v != "(devel)" { + Version = strings.TrimPrefix(v, "v") + } + } +} From bea8bcfd960a8cde9e14f0ddfb26aa905ec4c621 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Mon, 12 Oct 2026 00:39:58 +0200 Subject: [PATCH 14/25] [CLI] Report errors as JSON on each command's stderr; check flags first - Every command reports errors through run(): one JSON line on the command's own stderr, exit 2 for usage errors and 1 for API and network errors. serpapi without a command prints help to stderr and exits 2. - --timeout, --fields and --jq are checked before any request: --fields works only with search, a jq syntax error fails before a billed search, and --timeout takes seconds or a Go duration such as 2m, refusing NaN, negatives and values under a nanosecond. - The API key comes from --api-key, SERPAPI_KEY, SERPAPI_API_KEY (the name LangChain and the SerpApi MCP server use), then the config file, trimmed. A broken config file makes search warn and run without a key. - account and login refuse arguments without echoing them, since a stray argument is often the key; account masks the key by character, so an escaped or multi-byte key is never cut. - login takes --api-key, reads a hidden prompt on a terminal or the first line of stdin, keeps the saved key unless told to replace it, and warns when SERPAPI_KEY or SERPAPI_API_KEY overrides the saved key. - locations needs a query instead of fetching all 66 MB of locations. - Output goes to the command's stdout, so tests can run the CLI in-process, and --jq prints each result as gojq produces it. Co-Authored-By: Claude Opus 5.5 --- pkg/cmd/account.go | 55 ++++----- pkg/cmd/archive.go | 29 +++-- pkg/cmd/locations.go | 38 +++--- pkg/cmd/login.go | 136 ++++++++++++---------- pkg/cmd/root.go | 270 +++++++++++++++++++++++++++++++------------ pkg/cmd/search.go | 32 ++--- pkg/config/config.go | 2 +- 7 files changed, 349 insertions(+), 213 deletions(-) diff --git a/pkg/cmd/account.go b/pkg/cmd/account.go index 9aa4843..6c8d4a4 100644 --- a/pkg/cmd/account.go +++ b/pkg/cmd/account.go @@ -9,53 +9,54 @@ import ( var accountCmd = &cobra.Command{ Use: "account", - Short: "Retrieve account information and usage statistics", - Args: cobra.NoArgs, - RunE: runAccount, + Short: "Show plan, usage and searches left (free)", + Long: `Show the account's plan, usage this month and hour, and searches left. +This request is free. The API key in the output is masked.`, + Example: ` serpapi account --jq .total_searches_left + serpapi account --jq "{plan_name, total_searches_left, account_rate_limit_per_hour}"`, + Args: noArgs("pass the key with SERPAPI_KEY or --api-key"), + RunE: runAccount, } func init() { rootCmd.AddCommand(accountCmd) } -func runAccount(cmd *cobra.Command, args []string) error { +func runAccount(cmd *cobra.Command, _ []string) error { apiKey, err := resolveAPIKey() if err != nil { return err } - - client, err := newClient(apiKey) - if err != nil { - return err - } - - sp := newSpinner("Fetching account...") - sp.Start() - defer sp.Stop() - result, err := client.Account(cmd.Context()) + client := newClient(apiKey) + result, err := withSpinner("Fetching account...", func() (json.RawMessage, error) { + return client.Account(cmd.Context()) + }) if err != nil { return err } - - return handleOutput(maskAPIKey(result)) + return handleOutput(cmd, maskAPIKey(result)) } -var reAPIKey = regexp.MustCompile(`("api_key"\s*:\s*")([^"]{9,})(")`) +var reAPIKey = regexp.MustCompile(`("api_key"\s*:\s*)("(?:[^"\\]|\\.)*")`) -// maskAPIKey replaces the api_key value in raw JSON with a masked version (first 4 + last 4). -// Operates on raw bytes to preserve original field order. +// maskAPIKey replaces the api_key value in raw JSON with its first and last +// four characters. It edits the bytes in place to keep the field order, and +// decodes the value so escapes and multi-byte characters are never cut. func maskAPIKey(raw json.RawMessage) json.RawMessage { return reAPIKey.ReplaceAllFunc(raw, func(match []byte) []byte { sub := reAPIKey.FindSubmatch(match) - if len(sub) < 4 { + var key string + if json.Unmarshal(sub[2], &key) != nil { + return match + } + r := []rune(key) + if len(r) < 9 { + return match + } + masked, err := json.Marshal(string(r[:4]) + "…" + string(r[len(r)-4:])) + if err != nil { return match } - key := string(sub[2]) - masked := key[:4] + "…" + key[len(key)-4:] - var buf []byte - buf = append(buf, sub[1]...) - buf = append(buf, masked...) - buf = append(buf, sub[3]...) - return buf + return append(append([]byte{}, sub[1]...), masked...) }) } diff --git a/pkg/cmd/archive.go b/pkg/cmd/archive.go index 01497d4..288d5c1 100644 --- a/pkg/cmd/archive.go +++ b/pkg/cmd/archive.go @@ -1,6 +1,7 @@ package cmd import ( + "encoding/json" "regexp" "github.com/spf13/cobra" @@ -11,10 +12,15 @@ import ( var reArchiveID = regexp.MustCompile(`^[a-zA-Z0-9_-]+$`) var archiveCmd = &cobra.Command{ - Use: "archive ", - Short: "Retrieve a previously cached search by ID", - Args: cobra.ExactArgs(1), - RunE: runArchive, + Use: "archive ID", + Short: "Fetch an earlier search by its search_metadata.id (free)", + Long: `Fetch an earlier search by the ID in its search_metadata.id. This request is +free, and it is how you collect the results of a search made with async=true: +until search_metadata.status is Success it is Queued or Processing, so poll.`, + Example: ` serpapi archive 6a2b3c4d5e6f7a8b9c0d1e2f + serpapi archive 6a2b3c4d5e6f7a8b9c0d1e2f --jq ".organic_results[0]"`, + Args: cobra.ExactArgs(1), + RunE: runArchive, } func init() { @@ -31,19 +37,10 @@ func runArchive(cmd *cobra.Command, args []string) error { if err != nil { return err } - - client, err := newClient(apiKey) - if err != nil { - return err - } - - sp := newSpinner("Fetching archive...") - sp.Start() - defer sp.Stop() - result, err := client.Archive(cmd.Context(), id) + client := newClient(apiKey) + result, err := withSpinner("Fetching archive...", func() (json.RawMessage, error) { return client.Archive(cmd.Context(), id) }) if err != nil { return err } - - return handleOutput(result) + return handleOutput(cmd, result) } diff --git a/pkg/cmd/locations.go b/pkg/cmd/locations.go index 741e554..b1fceae 100644 --- a/pkg/cmd/locations.go +++ b/pkg/cmd/locations.go @@ -1,16 +1,24 @@ package cmd import ( + "encoding/json" + "strings" + "github.com/spf13/cobra" + clierrors "github.com/serpapi/serpapi-cli/pkg/errors" "github.com/serpapi/serpapi-cli/pkg/params" ) var locationsCmd = &cobra.Command{ - Use: "locations [PARAMS...]", - Short: "Lookup available locations for search queries", - Args: cobra.ArbitraryArgs, - RunE: runLocations, + Use: "locations q=QUERY [limit=N]", + Short: "Find values for the location parameter (free, no API key)", + Long: `Find supported locations. Pass a result's canonical_name as location= in a +search. limit is at most 10. This request is free and needs no API key.`, + Example: ` serpapi locations q=Austin limit=3 + serpapi locations q=Austin --jq ".[0].canonical_name"`, + Args: cobra.ArbitraryArgs, + RunE: runLocations, } func init() { @@ -22,20 +30,18 @@ func runLocations(cmd *cobra.Command, args []string) error { if err != nil { return err } - - paramsMap := params.ParamsToMap(parsed) - client, err := newClient("") - if err != nil { - return err + p := params.ParamsToMap(parsed) + if blank(p["q"]) && blank(p["location"]) && blank(p["limit"]) { + return &clierrors.UsageError{Message: "Pass a location query, e.g. serpapi locations q=Austin limit=5; without one the server returns every location (66 MB)"} } - - sp := newSpinner("Fetching locations...") - sp.Start() - defer sp.Stop() - result, err := client.Locations(cmd.Context(), paramsMap) + client := newClient("") + result, err := withSpinner("Fetching locations...", func() (json.RawMessage, error) { + return client.Locations(cmd.Context(), p) + }) if err != nil { return err } - - return handleOutput(result) + return handleOutput(cmd, result) } + +func blank(s string) bool { return strings.TrimSpace(s) == "" } diff --git a/pkg/cmd/login.go b/pkg/cmd/login.go index 7c1d22d..fc6122b 100644 --- a/pkg/cmd/login.go +++ b/pkg/cmd/login.go @@ -2,7 +2,9 @@ package cmd import ( "bufio" + "context" "encoding/json" + "errors" "fmt" "io" "os" @@ -17,92 +19,98 @@ import ( var loginCmd = &cobra.Command{ Use: "login", - Short: "Save API key to config file", - Args: cobra.NoArgs, - RunE: runLogin, + Short: "Check an API key and save it for later commands", + Long: `Check an API key with a free account request, then save it to the config +file. The key comes from --api-key, a hidden prompt on a terminal, or else the +first line of stdin, which login waits for. In scripts and agents, set +SERPAPI_KEY instead; it takes precedence over the saved key. Get a key at +https://serpapi.com/manage-api-key`, + Example: ` serpapi login + printf %s "$KEY" | serpapi login`, + Args: noArgs(`pipe the key instead: printf %s "$KEY" | serpapi login`), + RunE: runLogin, } func init() { rootCmd.AddCommand(loginCmd) } -func runLogin(cmd *cobra.Command, args []string) error { - // Check if already authenticated - if existingKey, err := config.LoadAPIKey(); err == nil && existingKey != "" { - client, err := newClient(existingKey) - if err != nil { +func runLogin(cmd *cobra.Command, _ []string) error { + apiKey := strings.TrimSpace(apiKeyFlag) + if apiKey == "" { + key, err := readAPIKey(cmd) + if err != nil || key == "" { return err } - raw, err := client.Account(cmd.Context()) - if err == nil { - var account struct { - AccountEmail string `json:"account_email"` - AccountStatus string `json:"account_status"` - } - if json.Unmarshal(raw, &account) == nil && account.AccountEmail != "" { - fmt.Fprintf(os.Stderr, "Already logged in as %s (%s).\n", account.AccountEmail, account.AccountStatus) - if term.IsTerminal(int(os.Stdin.Fd())) { - fmt.Fprint(os.Stderr, "Re-authenticate with a different key? [y/N] ") - reader := bufio.NewReader(os.Stdin) - line, _ := reader.ReadString('\n') - if strings.TrimSpace(strings.ToLower(line)) != "y" { - return nil - } - } else { - // Non-interactive: exit non-zero so CI detects the no-op - return fmt.Errorf("already logged in as %s. Delete %s to re-authenticate", account.AccountEmail, config.Path()) - } - } - } - // If API call failed (e.g. expired key), fall through to re-login + apiKey = key } - var apiKey string + email, err := accountEmail(cmd.Context(), apiKey) + if err != nil { + return err + } + if err := config.SaveConfig(apiKey); err != nil { + return err + } + // Without a terminal, success is exit 0 and stderr carries only JSON lines. + if f, ok := cmd.ErrOrStderr().(*os.File); ok && term.IsTerminal(int(f.Fd())) { + fmt.Fprintf(f, "Logged in as %s. API key saved to %s\n", email, config.Path()) + } + for _, name := range apiKeyEnvVars { + if strings.TrimSpace(os.Getenv(name)) != "" { + warn(cmd, "%s is set and takes precedence over the saved key", name) + break + } + } + return nil +} - if term.IsTerminal(int(os.Stdin.Fd())) { - fmt.Fprint(os.Stderr, "Enter your SerpApi API key: ") - raw, err := term.ReadPassword(int(os.Stdin.Fd())) - fmt.Fprintln(os.Stderr) // newline after hidden input +// readAPIKey prompts on a terminal and otherwise reads the first line of +// stdin. It returns "" and no error when the user keeps the saved key. +func readAPIKey(cmd *cobra.Command) (string, error) { + stderr := cmd.ErrOrStderr() + var key string + if f, ok := cmd.InOrStdin().(*os.File); ok && term.IsTerminal(int(f.Fd())) { + if saved, err := config.LoadAPIKey(); err == nil && saved != "" { + if email, err := accountEmail(cmd.Context(), saved); err == nil { + fmt.Fprintf(stderr, "Already logged in as %s. Replace the saved key? [y/N] ", email) + line, _ := bufio.NewReader(f).ReadString('\n') + if !strings.EqualFold(strings.TrimSpace(line), "y") { + fmt.Fprintln(stderr, "Kept the saved key.") + return "", nil + } + } + } + fmt.Fprint(stderr, "SerpApi API key (input hidden): ") + b, err := term.ReadPassword(int(f.Fd())) + fmt.Fprintln(stderr) if err != nil { - return &clierrors.UsageError{Message: "Failed to read input: " + err.Error()} + return "", &clierrors.UsageError{Message: "Failed to read the API key: " + err.Error()} } - apiKey = string(raw) + key = string(b) } else { - fmt.Fprint(os.Stderr, "Enter your SerpApi API key: ") - reader := bufio.NewReader(os.Stdin) - line, err := reader.ReadString('\n') - if err != nil && err != io.EOF { - return &clierrors.UsageError{Message: "Failed to read input: " + err.Error()} + line, err := bufio.NewReader(cmd.InOrStdin()).ReadString('\n') + if err != nil && !errors.Is(err, io.EOF) { + return "", &clierrors.UsageError{Message: "Failed to read the API key: " + err.Error()} } - apiKey = line + key = line } - - apiKey = strings.TrimSpace(apiKey) - if apiKey == "" { - return &clierrors.UsageError{Message: "API key cannot be empty."} + if key = strings.TrimSpace(key); key == "" { + return "", &clierrors.UsageError{Message: `No API key given. Use: printf %s "$KEY" | serpapi login`} } + return key, nil +} - client, err := newClient(apiKey) +func accountEmail(ctx context.Context, apiKey string) (string, error) { + raw, err := newClient(apiKey).Account(ctx) if err != nil { - return err + return "", err } - raw, err := client.Account(cmd.Context()) - if err != nil { - return err - } - - email := "unknown" var account struct { AccountEmail string `json:"account_email"` } - if json.Unmarshal(raw, &account) == nil && account.AccountEmail != "" { - email = account.AccountEmail - } - - if err := config.SaveConfig(apiKey); err != nil { - return err + if json.Unmarshal(raw, &account) != nil || account.AccountEmail == "" { + return "unknown", nil } - - fmt.Fprintf(os.Stderr, "Logged in as %s. API key saved to %q\n", email, config.Path()) - return nil + return account.AccountEmail, nil } diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index 18208f7..dff318a 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -1,11 +1,14 @@ package cmd import ( + "bufio" "bytes" + "context" "encoding/json" "fmt" "os" "strconv" + "strings" "time" "github.com/spf13/cobra" @@ -24,37 +27,99 @@ var ( fieldsFlag string jqFlag string timeoutFlag string + + // requestTimeout is --timeout, else SERPAPI_TIMEOUT, as validateGlobalFlags + // resolved it for this run. + requestTimeout = api.DefaultTimeout + + // clientOptions are applied to every client after the defaults; tests use + // it to point the CLI at a local server. + clientOptions []api.Option ) +// apiKeyEnvVars are read in order after --api-key. SERPAPI_API_KEY is the name +// LangChain and the SerpApi MCP server use. +var apiKeyEnvVars = []string{"SERPAPI_KEY", "SERPAPI_API_KEY"} + var rootCmd = &cobra.Command{ Use: "serpapi", - Short: "HTTP client for structured web search data via SerpApi", - Example: ` serpapi search engine=google q=coffee - serpapi search engine=google_light q="best pizza NYC" --jq ".organic_results[:3]" - serpapi account`, + Short: "Search engine results as JSON from SerpApi", + Long: `Search engine results as JSON from SerpApi (https://serpapi.com). + +Parameters are key=value pairs sent to the API as-is; words without "=" are +joined into q. Engines and their parameters: https://serpapi.com/search-engine-apis + +API key, first match wins: --api-key, SERPAPI_KEY, SERPAPI_API_KEY, then the +key saved by 'serpapi login'. Without one, search still runs, but the API +answers only a few demo queries, such as q=coffee, and returns 401 for others. + +Results go to stdout as JSON indented by two spaces, colored on a terminal. +Piped, it is indented too, so head, grep and file viewers show whole lines; +head cuts the JSON short, so take parts with --jq. A terminal gets control +characters from results escaped, so a page cannot drive it; piped output keeps +them. Keep results small: + --fields "organic_results[].{title, link}" server-side filter (search only) + --jq ".organic_results[].link" jq filter; strings print unquoted + +Errors and warnings go to stderr as one JSON object per line: + {"error":{"code":"api_error","message":"Invalid API key. ...","status":401}} + {"warning":{"message":"..."}} +Exit 2, code usage_error: the command is wrong; fix it before retrying. +Exit 1, code api_error or network_error. status is the HTTP status of the +answer: 200 the search ran but reported a problem, such as no results; 400 +invalid parameters; 401 invalid API key; 403 account not allowed; 429 rate +limited or out of searches; 5xx SerpApi failed. No status means no usable +answer: a network_error, such as a timeout, or an unreadable response. +Requests time out after 60s: --timeout or SERPAPI_TIMEOUT sets seconds, 0 +waits indefinitely. + +Cost: each successful search counts against your monthly searches, including +each page of --all-pages; cached, errored and failed ones don't, though errored +ones count toward the hourly limit (https://serpapi.com/pricing). An identical +search, --fields included, repeated within an hour comes from the cache. +no_cache=true forces a new, counted search; cached results are at most an +hour old, so add it only when that is too old. account, locations and archive +are free.`, + Example: ` serpapi search engine=google q=coffee --fields "organic_results[0:3]" + serpapi search engine=google coffee shops --jq ".organic_results[].link" + serpapi locations q=Austin limit=3 + serpapi account --jq .total_searches_left`, Version: version.Version, SilenceUsage: true, SilenceErrors: true, RunE: func(cmd *cobra.Command, args []string) error { + cmd.SetOut(cmd.ErrOrStderr()) _ = cmd.Help() - return &clierrors.UsageError{Message: "No command specified. See usage above."} + return &clierrors.UsageError{Message: "No command given; see 'serpapi --help'"} }, } -// Execute runs the root command. +// Execute runs the root command and exits with the code run returns. func Execute() { - if err := rootCmd.Execute(); err != nil { - clierrors.PrintError(err) - os.Exit(clierrors.ExitCode(err)) + os.Exit(run(context.Background())) +} + +// run executes the root command, reports any error on its stderr as JSON and +// returns the exit code. +func run(ctx context.Context) int { + err := rootCmd.ExecuteContext(ctx) + if err == nil { + return 0 } + clierrors.FprintError(rootCmd.ErrOrStderr(), err) + return clierrors.ExitCode(err) } func init() { - rootCmd.PersistentFlags().StringVar(&apiKeyFlag, "api-key", "", "SerpApi API key (env: SERPAPI_KEY)") - rootCmd.PersistentFlags().StringVar(&fieldsFlag, "fields", "", "Restrict JSON fields (server-side json_restrictor)") - rootCmd.PersistentFlags().StringVar(&jqFlag, "jq", "", "Apply jq filter to output") - rootCmd.PersistentFlags().StringVar(&timeoutFlag, "timeout", "", - fmt.Sprintf("HTTP request timeout in seconds, 0 to disable (default %d; env: SERPAPI_TIMEOUT)", int(api.DefaultTimeout/time.Second))) + pf := rootCmd.PersistentFlags() + pf.StringVar(&apiKeyFlag, "api-key", "", "SerpApi API key; prefer SERPAPI_KEY, since flags show up in ps and shell history") + pf.StringVar(&fieldsFlag, "fields", "", `search only: server-side json_restrictor, e.g. "organic_results[].{title, link}"`) + pf.StringVar(&jqFlag, "jq", "", `jq filter applied to the JSON result, e.g. ".organic_results[].link"`) + pf.StringVar(&timeoutFlag, "timeout", "", fmt.Sprintf( + "HTTP request timeout in seconds, such as 120, or a duration, such as 2m; 0 waits indefinitely (default %d; env SERPAPI_TIMEOUT)", + int(api.DefaultTimeout/time.Second))) + + rootCmd.PersistentPreRunE = validateGlobalFlags // Wrap cobra flag-parsing errors as UsageError so they get exit code 2. rootCmd.SetFlagErrorFunc(func(_ *cobra.Command, err error) error { @@ -62,38 +127,31 @@ func init() { }) } -// resolveAPIKey merges --api-key flag with SERPAPI_KEY env var, then config file. -func resolveAPIKey() (string, error) { - key := apiKeyFlag - if key == "" { - key = os.Getenv("SERPAPI_KEY") - } - return config.ResolveAPIKey(key) -} - -// resolveAPIKeyOptional resolves the API key but returns empty string instead of error. -func resolveAPIKeyOptional() string { - key := apiKeyFlag - if key == "" { - key = os.Getenv("SERPAPI_KEY") +// validateGlobalFlags rejects bad flags before any request, so a typo never +// costs a search. +func validateGlobalFlags(cmd *cobra.Command, _ []string) error { + timeout, err := resolveTimeout() + if err != nil { + return err } - if key != "" { - return key + requestTimeout = timeout + if fieldsFlag != "" && cmd != searchCmd { + return &clierrors.UsageError{Message: "--fields works only with search; use --jq to filter " + cmd.Name() + " output"} } - if k, err := config.LoadAPIKey(); err == nil && k != "" { - return k + if jqFlag != "" { + if _, err := jq.Compile(jqFlag); err != nil { + return &clierrors.UsageError{Message: err.Error()} + } } - return "" + return nil } -// resolveTimeout merges --timeout with SERPAPI_TIMEOUT, falling back to -// api.DefaultTimeout. Values are whole or fractional seconds; 0 disables. +// resolveTimeout returns --timeout, else SERPAPI_TIMEOUT, else +// api.DefaultTimeout. 0 disables the limit. func resolveTimeout() (time.Duration, error) { - raw := timeoutFlag - source := "--timeout" + raw, source := strings.TrimSpace(timeoutFlag), "--timeout" if raw == "" { - raw = os.Getenv("SERPAPI_TIMEOUT") - source = "SERPAPI_TIMEOUT" + raw, source = strings.TrimSpace(os.Getenv("SERPAPI_TIMEOUT")), "SERPAPI_TIMEOUT" } if raw == "" { return api.DefaultTimeout, nil @@ -101,53 +159,119 @@ func resolveTimeout() (time.Duration, error) { return parseTimeout(raw, source) } +// parseTimeout reads seconds, such as 120 or 1.5, or a duration, such as 2m. func parseTimeout(raw, source string) (time.Duration, error) { - secs, err := strconv.ParseFloat(raw, 64) - if err != nil || secs < 0 || secs != secs { // secs != secs rejects NaN - return 0, &clierrors.UsageError{ - Message: fmt.Sprintf("Invalid %s value %q: expected a non-negative number of seconds", source, raw), + invalid := &clierrors.UsageError{Message: fmt.Sprintf( + "Invalid %s value %q: expected seconds, such as 120, or a duration, such as 2m; 0 waits indefinitely", source, raw)} + if secs, err := strconv.ParseFloat(raw, 64); err == nil { + // Also rejects NaN and infinity; 1e9 s (31 years) keeps time.Duration from overflowing. + if !(secs >= 0 && secs <= 1e9) { + return 0, invalid } + d := time.Duration(secs * float64(time.Second)) + if secs > 0 && d == 0 { // under a nanosecond, which would mean no limit + return 0, invalid + } + return d, nil + } + d, err := time.ParseDuration(raw) + // A nonzero duration under a nanosecond, such as 0.5ns, also parses as 0. + if err != nil || d < 0 || (d == 0 && strings.ContainsAny(raw, "123456789")) { + return 0, invalid } - return time.Duration(secs * float64(time.Second)), nil + return d, nil } -// newClient builds an API client honoring the configured request timeout. -func newClient(apiKey string) (*api.Client, error) { - timeout, err := resolveTimeout() - if err != nil { - return nil, err +// resolveAPIKey returns the first non-empty key from --api-key, the +// environment, then the config file. +func resolveAPIKey() (string, error) { + if key := strings.TrimSpace(apiKeyFlag); key != "" { + return key, nil + } + for _, name := range apiKeyEnvVars { + if key := strings.TrimSpace(os.Getenv(name)); key != "" { + return key, nil + } } - return api.NewWithTimeout(apiKey, timeout), nil + return config.ResolveAPIKey("") } -// handleOutput applies --jq filtering and prints result. -func handleOutput(raw json.RawMessage) error { - if jqFlag != "" { - // Unmarshal for jq processing; use decoder with UseNumber to preserve integer fidelity. - var v any - dec := json.NewDecoder(bytes.NewReader(raw)) - dec.UseNumber() - if err := dec.Decode(&v); err != nil { - return &clierrors.APIError{Message: "Failed to parse response: " + err.Error()} - } - results, err := jq.Apply(jqFlag, v) - if err != nil { - return &clierrors.UsageError{Message: err.Error()} - } - for _, val := range results { - if err := output.PrintJQValue(val, os.Stdout, output.IsTerminal(os.Stdout)); err != nil { - return &clierrors.APIError{Message: fmt.Sprintf("Output error: %s", err)} - } +func newClient(apiKey string) *api.Client { + opts := append([]api.Option{api.WithTimeout(requestTimeout)}, clientOptions...) + return api.New(apiKey, opts...) +} + +// withSpinner shows a spinner on an interactive stderr while fn runs and +// clears it before the caller prints anything. +func withSpinner[T any](label string, fn func() (T, error)) (T, error) { + sp := spinner.New(label) + sp.Start() + defer sp.Stop() + return fn() +} + +func warn(cmd *cobra.Command, format string, a ...any) { + clierrors.PrintWarning(cmd.ErrOrStderr(), fmt.Sprintf(format, a...)) +} + +// noArgs rejects arguments without echoing them, as cobra.NoArgs would: a +// stray argument to login or account is often the API key. +func noArgs(hint string) cobra.PositionalArgs { + return func(cmd *cobra.Command, args []string) error { + if len(args) > 0 { + return &clierrors.UsageError{Message: cmd.Name() + " takes no arguments; " + hint} } return nil } - if err := output.PrintJSON(os.Stdout, raw); err != nil { - return &clierrors.APIError{Message: fmt.Sprintf("Output error: %s", err)} +} + +// handleOutput applies --jq, if set, and prints the JSON result to stdout. +func handleOutput(cmd *cobra.Command, raw []byte) error { + if jqFlag == "" { + return outputError(output.PrintJSON(cmd.OutOrStdout(), raw)) + } + var v any + dec := json.NewDecoder(bytes.NewReader(raw)) + dec.UseNumber() + if err := dec.Decode(&v); err != nil { + return &clierrors.APIError{Message: "Failed to parse response: " + err.Error()} + } + return printJQ(cmd, v) +} + +// printJQ runs --jq on v and prints each result as it is produced. +func printJQ(cmd *cobra.Command, v any) error { + code, err := jq.Compile(jqFlag) + if err != nil { + return &clierrors.UsageError{Message: err.Error()} + } + w := cmd.OutOrStdout() + tty := output.IsTerminal(w) + bw := bufio.NewWriter(w) + var writeErr error + ctx := cmd.Context() + if ctx == nil { + ctx = context.Background() + } + err = jq.Run(ctx, code, v, func(val any) error { + writeErr = output.PrintJQValue(val, bw, tty) + return writeErr + }) + if flushErr := bw.Flush(); writeErr == nil { + writeErr = flushErr + } + if writeErr != nil { + return outputError(writeErr) + } + if err != nil { + return &clierrors.UsageError{Message: err.Error()} } return nil } -// newSpinner creates a spinner with the given label. -func newSpinner(label string) *spinner.Spinner { - return spinner.New(label) +func outputError(err error) error { + if err == nil { + return nil + } + return &clierrors.APIError{Message: "Output error: " + err.Error()} } diff --git a/pkg/cmd/search.go b/pkg/cmd/search.go index 2470a3f..2bf6b84 100644 --- a/pkg/cmd/search.go +++ b/pkg/cmd/search.go @@ -3,13 +3,14 @@ package cmd import ( "bytes" "encoding/json" + "errors" "fmt" "net/url" - "os" "sort" "github.com/spf13/cobra" + "github.com/serpapi/serpapi-cli/pkg/config" clierrors "github.com/serpapi/serpapi-cli/pkg/errors" "github.com/serpapi/serpapi-cli/pkg/params" ) @@ -48,30 +49,30 @@ func runSearch(cmd *cobra.Command, args []string) error { return err } - apiKey := resolveAPIKeyOptional() paramsMap := params.ParamsToMap(parsed) params.ApplyFields(paramsMap, fieldsFlag) - client, err := newClient(apiKey) - if err != nil { - return err + // Search runs without a key, as in v0.2: the API answers a few demo + // queries without one and returns 401 for the rest. + apiKey, keyErr := resolveAPIKey() + var cfgErr *clierrors.UsageError + if keyErr != config.ErrNoAPIKey && errors.As(keyErr, &cfgErr) { + warn(cmd, "%s; searching without a key", cfgErr.Message) } + client := newClient(apiKey) hasMaxPages := cmd.Flags().Changed("max-pages") if hasMaxPages && !allPagesFlag { - fmt.Fprintln(os.Stderr, "Warning: --max-pages has no effect without --all-pages") + warn(cmd, "--max-pages has no effect without --all-pages") } if !allPagesFlag { - sp := newSpinner("Searching...") - sp.Start() - defer sp.Stop() - raw, err := client.Search(cmd.Context(), paramsMap) + raw, err := withSpinner("Searching...", func() (json.RawMessage, error) { return client.Search(cmd.Context(), paramsMap) }) if err != nil { return err } - return handleOutput(raw) + return handleOutput(cmd, raw) } // Pagination mode @@ -92,10 +93,9 @@ func runSearch(cmd *cobra.Command, args []string) error { visitedPages[canonicalParamsKey(currentParams)] = true for { - sp := newSpinner(fmt.Sprintf("Fetching page %d...", pagesFetched+1)) - sp.Start() - raw, err := client.Search(cmd.Context(), currentParams) - sp.Stop() + raw, err := withSpinner(fmt.Sprintf("Fetching page %d...", pagesFetched+1), func() (json.RawMessage, error) { + return client.Search(cmd.Context(), currentParams) + }) if err != nil { return err } @@ -149,7 +149,7 @@ func runSearch(cmd *cobra.Command, args []string) error { if err := enc.Encode(accumulated); err != nil { return &clierrors.APIError{Message: "Failed to encode result: " + err.Error()} } - return handleOutput(json.RawMessage(bytes.TrimRight(buf.Bytes(), "\n"))) + return handleOutput(cmd, json.RawMessage(bytes.TrimRight(buf.Bytes(), "\n"))) } // withPaginationRestrictor adds the pagination field needed internally while diff --git a/pkg/config/config.go b/pkg/config/config.go index 8825d23..8d8c306 100644 --- a/pkg/config/config.go +++ b/pkg/config/config.go @@ -36,7 +36,7 @@ func Path() string { // ErrNoAPIKey is returned by ResolveAPIKey when no source has a key. var ErrNoAPIKey = &clierrors.UsageError{ - Message: "No API key found. Pass --api-key, set SERPAPI_KEY, or run 'serpapi login'. Get a key at https://serpapi.com/manage-api-key", + Message: "No API key found. Pass --api-key, set SERPAPI_KEY (or SERPAPI_API_KEY), or run 'serpapi login'. Get a key at https://serpapi.com/manage-api-key", } // LoadAPIKey returns the API key saved in the config file, or "" when there From f364f774e548c956faa48353184edc61b32f2b8d Mon Sep 17 00:00:00 2001 From: ilyazub Date: Mon, 12 Oct 2026 00:40:07 +0200 Subject: [PATCH 15/25] [Tests] Run the CLI in-process against a fake SerpApi cmd_test.go runs commands in-process through run(), with a fake API from httptest, an isolated HOME and no key in the environment, and checks the exit code, stdout, every JSON line on stderr and the requests sent. edge_test.go is a table of 62 edge cases from stress testing, grouped by input: arguments, flags, API keys, the config file, server answers and output; fuzz_test.go fuzzes the key masking. pflag becomes a direct dependency for resetting flags between runs. Cases for features later in this stack are added with them. Co-Authored-By: Claude Opus 5.5 --- go.mod | 2 +- pkg/cmd/cmd_test.go | 435 +++++++++++++++++++++++++++++++++++++++++++ pkg/cmd/edge_test.go | 314 +++++++++++++++++++++++++++++++ pkg/cmd/fuzz_test.go | 33 ++++ 4 files changed, 783 insertions(+), 1 deletion(-) create mode 100644 pkg/cmd/cmd_test.go create mode 100644 pkg/cmd/edge_test.go create mode 100644 pkg/cmd/fuzz_test.go diff --git a/go.mod b/go.mod index cb631e9..052d7d3 100644 --- a/go.mod +++ b/go.mod @@ -7,6 +7,7 @@ require ( github.com/briandowns/spinner v1.23.2 github.com/itchyny/gojq v0.12.19 github.com/spf13/cobra v1.10.2 + github.com/spf13/pflag v1.0.10 github.com/tidwall/pretty v1.2.2 golang.org/x/term v0.45.0 ) @@ -17,6 +18,5 @@ require ( github.com/itchyny/timefmt-go v0.1.8 // indirect github.com/mattn/go-colorable v0.1.2 // indirect github.com/mattn/go-isatty v0.0.20 // indirect - github.com/spf13/pflag v1.0.10 // indirect golang.org/x/sys v0.47.0 // indirect ) diff --git a/pkg/cmd/cmd_test.go b/pkg/cmd/cmd_test.go new file mode 100644 index 0000000..82f557b --- /dev/null +++ b/pkg/cmd/cmd_test.go @@ -0,0 +1,435 @@ +package cmd + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "net/url" + "strconv" + "strings" + "sync" + "testing" + + "github.com/spf13/cobra" + "github.com/spf13/pflag" + + "github.com/serpapi/serpapi-cli/pkg/api" + "github.com/serpapi/serpapi-cli/pkg/config" +) + +type result struct { + code int + stdout, stderr string +} + +type requests struct { + mu sync.Mutex + urls []*url.URL +} + +func (r *requests) add(u *url.URL) { + r.mu.Lock() + defer r.mu.Unlock() + r.urls = append(r.urls, u) +} + +func (r *requests) list() []*url.URL { + r.mu.Lock() + defer r.mu.Unlock() + return append([]*url.URL(nil), r.urls...) +} + +func (r *requests) only(t *testing.T) *url.URL { + t.Helper() + urls := r.list() + if len(urls) != 1 { + t.Fatalf("got %d requests, want 1: %v", len(urls), urls) + } + return urls[0] +} + +// fakeAPI isolates the environment and the config directory and points the +// CLI at handler. A nil handler fails the test on any request. +func fakeAPI(t *testing.T, handler http.HandlerFunc) *requests { + t.Helper() + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("XDG_CONFIG_HOME", home) + t.Setenv("AppData", home) + t.Setenv("NO_COLOR", "1") + for _, name := range apiKeyEnvVars { + t.Setenv(name, "") + } + + reqs := &requests{} + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + reqs.add(r.URL) + if handler == nil { + t.Errorf("unexpected request to %s", r.URL.Path) + http.Error(w, "unexpected request", http.StatusTeapot) + return + } + handler(w, r) + })) + t.Cleanup(srv.Close) + clientOptions = []api.Option{api.WithBaseURL(srv.URL)} + t.Cleanup(func() { clientOptions = nil }) + return reqs +} + +func respond(body string) http.HandlerFunc { + return func(w http.ResponseWriter, _ *http.Request) { fmt.Fprint(w, body) } +} + +func respondStatus(status int, body string) http.HandlerFunc { + return func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(status) + fmt.Fprint(w, body) + } +} + +// execute runs the CLI in-process with every flag back at its default, as in +// a new process. +func execute(t *testing.T, stdin string, args ...string) result { + t.Helper() + return executeContext(t, context.Background(), stdin, args...) +} + +// executeContext is execute with a context the test can cancel, as Ctrl-C does. +func executeContext(t *testing.T, ctx context.Context, stdin string, args ...string) result { + t.Helper() + resetFlags(rootCmd) + var stdout, stderr bytes.Buffer + rootCmd.SetOut(&stdout) + rootCmd.SetErr(&stderr) + rootCmd.SetIn(strings.NewReader(stdin)) + // A nil slice would make cobra parse the test binary's own arguments. + rootCmd.SetArgs(append([]string{}, args...)) + code := run(ctx) + return result{code: code, stdout: stdout.String(), stderr: stderr.String()} +} + +// resetFlags puts every flag back at its default and clears each command's +// context, which cobra otherwise keeps from the first run in this process. +func resetFlags(c *cobra.Command) { + reset := func(f *pflag.Flag) { + _ = f.Value.Set(f.DefValue) + f.Changed = false + } + c.Flags().VisitAll(reset) + c.PersistentFlags().VisitAll(reset) + //lint:ignore SA1012 cobra hands a command the run's context only while its own is nil + c.SetContext(nil) + for _, sub := range c.Commands() { + resetFlags(sub) + } +} + +// parseStderr requires every stderr line to be a JSON error or warning and +// returns the warning messages and the error, if any. +func parseStderr(t *testing.T, stderr string) ([]string, map[string]any) { + t.Helper() + var warnings []string + var errObj map[string]any + for _, line := range strings.Split(stderr, "\n") { + if line == "" { + continue + } + var v struct { + Error map[string]any `json:"error"` + Warning *struct { + Message string `json:"message"` + } `json:"warning"` + } + if err := json.Unmarshal([]byte(line), &v); err != nil || (v.Error == nil && v.Warning == nil) { + t.Fatalf("stderr line is not a JSON error or warning: %q", line) + } + if v.Warning != nil { + warnings = append(warnings, v.Warning.Message) + } + if v.Error != nil { + if errObj != nil { + t.Fatalf("more than one error on stderr: %q", stderr) + } + errObj = v.Error + } + } + return warnings, errObj +} + +func wantOK(t *testing.T, res result) []string { + t.Helper() + if res.code != 0 { + t.Fatalf("exit code = %d, want 0; stderr: %s", res.code, res.stderr) + } + warnings, errObj := parseStderr(t, res.stderr) + if errObj != nil { + t.Fatalf("unexpected error: %v", errObj) + } + return warnings +} + +func wantError(t *testing.T, res result, exit int, code, msgPart string) map[string]any { + t.Helper() + if res.code != exit { + t.Fatalf("exit code = %d, want %d; stderr: %s", res.code, exit, res.stderr) + } + _, errObj := parseStderr(t, res.stderr) + if errObj == nil { + t.Fatalf("no error on stderr: %q", res.stderr) + } + if errObj["code"] != code { + t.Errorf("error code = %v, want %s", errObj["code"], code) + } + if msg, _ := errObj["message"].(string); !strings.Contains(msg, msgPart) { + t.Errorf("error message = %q, want it to contain %q", msg, msgPart) + } + return errObj +} + +func wantWarnings(t *testing.T, got []string, want ...string) { + t.Helper() + if strings.Join(got, "\n") != strings.Join(want, "\n") { + t.Errorf("warnings = %q, want %q", got, want) + } +} + +func wantQuery(t *testing.T, u *url.URL, path string, want map[string]string) { + t.Helper() + if u.Path != path { + t.Errorf("path = %s, want %s", u.Path, path) + } + q := u.Query() + for k, v := range want { + if got := q.Get(k); got != v { + t.Errorf("%s = %q, want %q", k, got, v) + } + } +} + +func TestSearchSendsParamsAndPrintsIndentedJSON(t *testing.T) { + reqs := fakeAPI(t, respond(`{"organic_results": [{"title": "Café Blue & co"}]}`)) + t.Setenv("SERPAPI_API_KEY", "altkey") + + res := execute(t, "", "search", "engine=google", "coffee", "shops", "no_cache=true") + + wantWarnings(t, wantOK(t, res)) + if want := "{\n \"organic_results\": [\n {\n \"title\": \"Café Blue & co\"\n }\n ]\n}\n"; res.stdout != want { + t.Errorf("stdout = %q, want %q", res.stdout, want) + } + wantQuery(t, reqs.only(t), "/search.json", map[string]string{ + "engine": "google", "q": "coffee shops", "no_cache": "true", "api_key": "altkey", + }) +} + +func TestAPIKeyPrecedence(t *testing.T) { + tests := []struct { + name, flag, serpapiKey, serpapiAPIKey, want string + }{ + {"flag", "flagkey", "envkey", "altkey", "flagkey"}, + {"SERPAPI_KEY", "", "envkey", "altkey", "envkey"}, + {"SERPAPI_API_KEY", "", " ", "altkey", "altkey"}, + {"saved", "", "", "", "savedkey"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + reqs := fakeAPI(t, respond(`{"account_email":"dev@example.com"}`)) + if err := config.SaveConfig("savedkey"); err != nil { + t.Fatal(err) + } + t.Setenv("SERPAPI_KEY", tt.serpapiKey) + t.Setenv("SERPAPI_API_KEY", tt.serpapiAPIKey) + args := []string{"account"} + if tt.flag != "" { + args = append(args, "--api-key", tt.flag) + } + + wantOK(t, execute(t, "", args...)) + wantQuery(t, reqs.only(t), "/account.json", map[string]string{"api_key": tt.want}) + }) + } +} + +func TestJQPrintsStringsRawAndKeepsNumbers(t *testing.T) { + fakeAPI(t, respond(`{"organic_results":[{"link":"https://a.example/?x=1&y=<2>"},{"link":"https://b.example"}],`+ + `"search_information":{"total_results":12345678901234567890}}`)) + t.Setenv("SERPAPI_KEY", "envkey") + + tests := []struct{ filter, want string }{ + {".organic_results[].link", "https://a.example/?x=1&y=<2>\nhttps://b.example\n"}, + {".search_information.total_results", "12345678901234567890\n"}, + {".organic_results[0]", `{"link":"https://a.example/?x=1&y=<2>"}` + "\n"}, + } + for _, tt := range tests { + res := execute(t, "", "search", "--jq", tt.filter, "engine=google", "q=coffee") + wantOK(t, res) + if res.stdout != tt.want { + t.Errorf("--jq %s: stdout = %q, want %q", tt.filter, res.stdout, tt.want) + } + } +} + +// page returns results page n of a search; every page but the last links to +// the next one, as SerpApi does, without the parameters it drops. +func page(n int, hasNext bool) string { + next := "" + if hasNext { + next = fmt.Sprintf(`,"serpapi_pagination":{"next":"https://serpapi.com/search.json?engine=google&q=coffee&start=%d"}`, n*10) + } + return fmt.Sprintf(`{"search_metadata":{"id":"page%d"},"organic_results":[{"position":%d}]%s}`, n, n, next) +} + +func pageNumber(r *http.Request) int { + start, _ := strconv.Atoi(r.URL.Query().Get("start")) + return start/10 + 1 +} + +func TestMaxPagesWithoutAllPagesWarns(t *testing.T) { + reqs := fakeAPI(t, respond(page(1, true))) + t.Setenv("SERPAPI_KEY", "envkey") + + res := execute(t, "", "search", "engine=google", "q=coffee", "--max-pages", "3") + + wantWarnings(t, wantOK(t, res), "--max-pages has no effect without --all-pages") + reqs.only(t) +} + +func TestAccountMasksAPIKey(t *testing.T) { + const key = "0123456789abcdef0123456789abcdef" + fakeAPI(t, respond(`{"account_email":"dev@example.com","api_key":"`+key+`","total_searches_left":42}`)) + t.Setenv("SERPAPI_KEY", key) + + res := execute(t, "", "account") + wantOK(t, res) + if strings.Contains(res.stdout, key) || !strings.Contains(res.stdout, `"api_key": "0123…cdef"`) { + t.Errorf("stdout does not mask the key: %s", res.stdout) + } + + res = execute(t, "", "account", "--jq", ".total_searches_left") + wantOK(t, res) + if res.stdout != "42\n" { + t.Errorf("stdout = %q, want 42", res.stdout) + } +} + +func TestLocationsSendsNoAPIKey(t *testing.T) { + reqs := fakeAPI(t, respond(`[{"canonical_name":"Austin,TX,Texas,United States"}]`)) + t.Setenv("SERPAPI_KEY", "envkey") + + res := execute(t, "", "locations", "Austin", "limit=3", "--jq", ".[0].canonical_name") + + wantOK(t, res) + if res.stdout != "Austin,TX,Texas,United States\n" { + t.Errorf("stdout = %q", res.stdout) + } + u := reqs.only(t) + wantQuery(t, u, "/locations.json", map[string]string{"q": "Austin", "limit": "3"}) + if u.Query().Has("api_key") { + t.Errorf("locations sent an API key: %s", u.RawQuery) + } +} + +func TestLoginSavesPipedKey(t *testing.T) { + reqs := fakeAPI(t, respond(`{"account_email":"dev@example.com"}`)) + if err := config.SaveConfig("oldkey"); err != nil { + t.Fatal(err) + } + t.Setenv("SERPAPI_API_KEY", "altkey") + + res := execute(t, " newkey \nignored\n", "login") + + wantWarnings(t, wantOK(t, res), "SERPAPI_API_KEY is set and takes precedence over the saved key") + if key, _ := config.LoadAPIKey(); key != "newkey" { + t.Errorf("saved key = %q, want newkey", key) + } + wantQuery(t, reqs.only(t), "/account.json", map[string]string{"api_key": "newkey"}) + if res.stdout != "" { + t.Errorf("stdout = %q, want empty", res.stdout) + } +} + +func TestLoginRewritesABrokenConfig(t *testing.T) { + fakeAPI(t, respond(`{"account_email":"dev@example.com"}`)) + writeConfig("api_key = " + strayKey)(t) + + res := execute(t, "newkey\n", "login") + + wantOK(t, res) + if key, err := config.LoadAPIKey(); key != "newkey" || err != nil { + t.Errorf("saved key = %q, %v; want newkey", key, err) + } +} + +func TestLoginWithFlagSkipsStdin(t *testing.T) { + reqs := fakeAPI(t, respond(`{"account_email":"dev@example.com"}`)) + + res := execute(t, "stdinkey\n", "login", "--api-key", "flagkey") + + if res.code != 0 { + t.Fatalf("exit code = %d; stderr: %s", res.code, res.stderr) + } + if key, _ := config.LoadAPIKey(); key != "flagkey" { + t.Errorf("saved key = %q, want flagkey", key) + } + wantQuery(t, reqs.only(t), "/account.json", map[string]string{"api_key": "flagkey"}) +} + +func TestLoginRejectsInvalidKeyWithoutSaving(t *testing.T) { + fakeAPI(t, respondStatus(401, `{"error":"Invalid API key."}`)) + + res := execute(t, "badkey\n", "login") + + errObj := wantError(t, res, 1, "api_error", "Invalid API key.") + if errObj["status"] != float64(401) { + t.Errorf("status = %v, want 401", errObj["status"]) + } + if key, _ := config.LoadAPIKey(); key != "" { + t.Error("an invalid key was saved") + } +} + +func TestLoginWithoutKey(t *testing.T) { + fakeAPI(t, nil) + wantError(t, execute(t, "\n", "login"), 2, "usage_error", "No API key given") +} + +func TestNoCommandPrintsHelpToStderr(t *testing.T) { + fakeAPI(t, nil) + + res := execute(t, "") + + if res.code != 2 { + t.Fatalf("exit code = %d, want 2", res.code) + } + if res.stdout != "" { + t.Errorf("stdout = %q, want empty", res.stdout) + } + help, last, _ := strings.Cut(strings.TrimSuffix(res.stderr, "\n"), "\n{") + if !strings.Contains(help, "Usage:") { + t.Errorf("stderr has no usage: %q", res.stderr) + } + if _, errObj := parseStderr(t, "{"+last); errObj["message"] != "No command given; see 'serpapi --help'" { + t.Errorf("error = %v", errObj) + } +} + +func TestHelpAndVersionGoToStdout(t *testing.T) { + fakeAPI(t, nil) + + for _, tt := range []struct { + args []string + want string + }{ + {[]string{"--help"}, "SERPAPI_KEY"}, + {[]string{"search", "--help"}, "--all-pages"}, + {[]string{"--version"}, "serpapi version "}, + } { + res := execute(t, "", tt.args...) + if res.code != 0 || res.stderr != "" || !strings.Contains(res.stdout, tt.want) { + t.Errorf("%v: exit %d, stdout %q, stderr %q; want %q on stdout", tt.args, res.code, res.stdout, res.stderr, tt.want) + } + } +} diff --git a/pkg/cmd/edge_test.go b/pkg/cmd/edge_test.go new file mode 100644 index 0000000..34ad9bf --- /dev/null +++ b/pkg/cmd/edge_test.go @@ -0,0 +1,314 @@ +package cmd + +import ( + "context" + "fmt" + "net" + "net/http" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + "time" + + "github.com/serpapi/serpapi-cli/pkg/config" +) + +// edgeCase is one edge case of the CLI. Cases are grouped by the one input +// that varies while the rest stays at its default (SERPAPI_KEY=envkey and an +// API that answers with one result), so each case has exactly one home: +// +// argv positional parameters: shape, encoding, reserved names +// flags flag values and combinations +// auth where the API key comes from +// response the API's answer: status, body shape, transport +// paging the serpapi_pagination.next chain of --all-pages +// output how results reach stdout +// +// Flows with several steps (login, key precedence, page merging, help) have +// their own tests in cmd_test.go. +type edgeCase struct { + name string + args []string + env map[string]string + setup func(t *testing.T) + api http.HandlerFunc // nil: any request fails the test + exit int + code string // error code, "" on success + msg string // substring of the error message + exact bool // msg is the whole error message + status int // error status, 0 for none + stdout string // exact stdout when set; errors without it must print nothing + reqs int // requests expected when api is set; 0 means 1 + query map[string]string + noQuery []string // parameters every request must omit + warn string // substring of a warning that must be printed + quiet bool // no warning may be printed +} + +const strayKey = "secretkey0123456789" + +var ( + oneResult = respond(`{"organic_results":[{"position":1}]}`) + noKey = map[string]string{"SERPAPI_KEY": "", "SERPAPI_API_KEY": ""} + search = []string{"search", "engine=google", "q=coffee"} +) + +func searchWith(args ...string) []string { return append(append([]string{}, search...), args...) } + +func edgeCases() []edgeCase { + invalidKey := `{"error":"Invalid API key. Your API key should be here: https://serpapi.com/manage-api-key"}` + pages := func(w http.ResponseWriter, r *http.Request) { fmt.Fprint(w, page(pageNumber(r), pageNumber(r) < 3)) } + return []edgeCase{ + {name: "argv/q and bare words", args: searchWith("tea"), exit: 2, code: "usage_error", msg: "Got both q=... and bare words"}, + {name: "argv/api_key parameter", args: searchWith("api_key=" + strayKey), exit: 2, code: "usage_error", msg: "not as the api_key parameter"}, + {name: "argv/dash word after --", args: []string{"search", "engine=google", "--", "coffee", "-starbucks"}, api: oneResult, + query: map[string]string{"q": "coffee -starbucks"}}, + {name: "argv/reserved URL characters", args: []string{"search", "engine=google", "q=a&b=c d+e%"}, api: oneResult, + query: map[string]string{"q": "a&b=c d+e%"}}, + {name: "argv/repeated identical key is quiet", args: searchWith("engine=google"), api: oneResult, quiet: true}, + {name: "argv/empty value is sent", args: []string{"search", "engine=google", "q="}, api: oneResult, query: map[string]string{"q": ""}}, + {name: "argv/unknown command", args: []string{"nope"}, exit: 2, code: "usage_error", msg: `unknown command "nope"`}, + {name: "argv/stray argument to account", args: []string{"account", strayKey}, exit: 2, code: "usage_error", msg: "account takes no arguments"}, + {name: "argv/stray argument to login", args: []string{"login", strayKey}, exit: 2, code: "usage_error", msg: "login takes no arguments; pipe the key instead"}, + {name: "argv/archive ID with a path", args: []string{"archive", "../bad"}, exit: 2, code: "usage_error", msg: "Invalid archive ID"}, + {name: "argv/archive with --jq reads the stored search", args: []string{"archive", "abc123", "--jq", ".place_results | {title, rating}"}, + api: respond(`{"search_metadata":{"id":"abc123"},"place_results":{"title":"Tartine","rating":4.6,"reviews":7351}}`), + stdout: `{"rating":4.6,"title":"Tartine"}` + "\n", query: map[string]string{"api_key": "envkey"}}, + {name: "argv/locations without query", args: []string{"locations"}, exit: 2, code: "usage_error", msg: "Pass a location query"}, + {name: "argv/locations blank query", args: []string{"locations", "q= "}, exit: 2, code: "usage_error", msg: "Pass a location query"}, + + {name: "flags/unknown flag outside search", args: []string{"account", "--bogus"}, exit: 2, code: "usage_error", msg: "unknown flag: --bogus", exact: true}, + {name: "flags/timeout in seconds", args: searchWith("--timeout", "30"), api: oneResult}, + {name: "flags/timeout as a duration", args: searchWith("--timeout", "2m"), api: oneResult}, + {name: "flags/timeout zero waits indefinitely", args: searchWith("--timeout", "0"), api: oneResult}, + {name: "flags/timeout negative", args: searchWith("--timeout=-5"), exit: 2, code: "usage_error", msg: `Invalid --timeout value "-5"`}, + {name: "flags/timeout infinity", args: searchWith("--timeout", "Inf"), exit: 2, code: "usage_error", msg: `Invalid --timeout value "Inf"`}, + {name: "flags/timeout negative duration", args: searchWith("--timeout=-1s"), exit: 2, code: "usage_error", msg: `Invalid --timeout value "-1s"`}, + {name: "flags/timeout NaN", args: searchWith("--timeout", "NaN"), exit: 2, code: "usage_error", msg: `Invalid --timeout value "NaN"`}, + {name: "flags/timeout past 31 years", args: searchWith("--timeout", "1e10"), exit: 2, code: "usage_error", msg: `Invalid --timeout value "1e10"`}, + {name: "flags/timeout under a nanosecond", args: searchWith("--timeout", "1e-10"), exit: 2, code: "usage_error", msg: `Invalid --timeout value "1e-10"`}, + {name: "flags/timeout duration under a nanosecond", args: searchWith("--timeout", "0.5ns"), exit: 2, code: "usage_error", + msg: `Invalid --timeout value "0.5ns"`}, + {name: "flags/timeout 0s waits indefinitely", args: searchWith("--timeout", "0s"), api: oneResult}, + {name: "flags/timeout word", args: searchWith("--timeout", "soon"), exit: 2, code: "usage_error", + msg: "expected seconds, such as 120, or a duration, such as 2m; 0 waits indefinitely"}, + {name: "flags/SERPAPI_TIMEOUT not a number", args: search, env: map[string]string{"SERPAPI_TIMEOUT": "abc"}, exit: 2, code: "usage_error", + msg: `Invalid SERPAPI_TIMEOUT value "abc"`}, + {name: "flags/--timeout wins over SERPAPI_TIMEOUT", args: searchWith("--timeout", "30"), env: map[string]string{"SERPAPI_TIMEOUT": "abc"}, api: oneResult}, + {name: "flags/fields outside search", args: []string{"account", "--fields", "plan_name"}, exit: 2, code: "usage_error", msg: "--fields works only with search"}, + {name: "flags/fields on locations", args: []string{"locations", "q=Austin", "--fields", "name"}, exit: 2, code: "usage_error", msg: "--fields works only with search"}, + {name: "flags/jq parse error", args: []string{"search", "--jq", ".[", "engine=google", "q=coffee"}, exit: 2, code: "usage_error", msg: "jq parse error"}, + {name: "flags/jq input builtin", args: searchWith("--jq", "input"), exit: 2, code: "usage_error", msg: "is not allowed"}, + {name: "flags/the suggested ? fix works", args: searchWith("--jq", "[.news_results[] | {title, source: (.source.name? // .source)}]"), + api: respond(`{"news_results":[{"title":"a","source":"Computer Weekly"},{"title":"b","source":{"name":"Wired"}}]}`), + stdout: `[{"source":"Computer Weekly","title":"a"},{"source":"Wired","title":"b"}]` + "\n"}, + {name: "flags/the one-item preview keeps every key", args: searchWith("--jq", `walk(if type == "array" then .[:1] else . end)`), + api: respond(`{"organic_results":[{"title":"a"},{"title":"b"}],"related_searches":[{"query":"x"},{"query":"y"}]}`), + stdout: `{"organic_results":[{"title":"a"}],"related_searches":[{"query":"x"}]}` + "\n"}, + {name: "flags/jq cannot read the environment", args: searchWith("--jq", "[$ENV, env] | map(length)"), api: oneResult, stdout: "[0,0]\n"}, + + {name: "auth/no key for account", args: []string{"account"}, env: noKey, exit: 2, code: "usage_error", msg: "No API key found"}, + {name: "auth/no key for archive", args: []string{"archive", "abc123"}, env: noKey, exit: 2, code: "usage_error", msg: "No API key found"}, + {name: "auth/no key for search", args: search, env: noKey, api: oneResult, noQuery: []string{"api_key"}}, + {name: "auth/no key for all pages", args: searchWith("--all-pages"), env: noKey, api: oneResult, noQuery: []string{"api_key"}}, + {name: "auth/invalid key keeps the API's message", args: search, env: map[string]string{"SERPAPI_KEY": "badkey"}, api: respondStatus(401, invalidKey), + exit: 1, code: "api_error", status: 401, msg: "Invalid API key. Your API key should be here: https://serpapi.com/manage-api-key", exact: true}, + {name: "auth/padded key is trimmed", args: []string{"account"}, env: map[string]string{"SERPAPI_KEY": " padded\n"}, + api: respond(`{"account_status":"Active"}`), query: map[string]string{"api_key": "padded"}}, + {name: "auth/config not valid TOML", args: []string{"account"}, env: noKey, setup: writeConfig("api_key = " + strayKey), + exit: 2, code: "usage_error", msg: "is not valid TOML (line 1); run 'serpapi login' to rewrite it"}, + {name: "auth/env key wins over a broken config", args: []string{"account"}, setup: writeConfig("api_key = " + strayKey), + api: respond(`{"account_status":"Active"}`), query: map[string]string{"api_key": "envkey"}}, + {name: "auth/search warns about a broken config", args: search, env: noKey, setup: writeConfig("api_key = " + strayKey), + api: oneResult, noQuery: []string{"api_key"}, warn: "is not valid TOML (line 1); run 'serpapi login' to rewrite it, or pass --api-key or SERPAPI_KEY; searching without a key"}, + {name: "auth/config unreadable", args: []string{"account"}, env: noKey, setup: unreadableConfig, + exit: 2, code: "usage_error", msg: "Cannot read the config file"}, + + {name: "response/invalid key", args: search, api: respondStatus(401, `{"error":"Invalid API key."}`), exit: 1, code: "api_error", status: 401, msg: "Invalid API key."}, + {name: "response/out of searches", args: search, api: respondStatus(429, `{"error":"Your account has run out of searches."}`), + exit: 1, code: "api_error", status: 429, msg: "run out of searches"}, + {name: "response/HTML error page", args: search, api: respondStatus(404, `Not Found`), exit: 1, code: "api_error", status: 404, msg: "HTTP 404: Not Found"}, + {name: "response/no results", args: search, api: respond(`{"error":"Google hasn't returned any results for this query."}`), + exit: 1, code: "api_error", status: 200, msg: "hasn't returned any results"}, + {name: "response/no results as markdown", args: searchWith("output=md"), api: respond(`{"error":"Google hasn't returned any results for this query."}`), + exit: 1, code: "api_error", status: 200, msg: "hasn't returned any results"}, + {name: "response/null body", args: search, api: respond(`null`), exit: 1, code: "api_error", msg: "not a JSON object"}, + {name: "response/array body", args: search, api: respond(`[{"position":1}]`), exit: 1, code: "api_error", msg: "not a JSON object"}, + {name: "response/redirect is not followed", args: search, api: redirectTo("http://127.0.0.1:1/steal"), exit: 1, code: "api_error", status: 302}, + {name: "response/connection closed", args: search, api: closeConnection, exit: 1, code: "network_error", msg: "Request to /search.json failed"}, + {name: "response/no answer in time", args: []string{"account", "--timeout", "50ms"}, api: hang, exit: 1, code: "network_error", + msg: "Request to /account.json timed out after 50ms"}, + {name: "response/SERPAPI_TIMEOUT applies", args: []string{"account"}, env: map[string]string{"SERPAPI_TIMEOUT": "0.05"}, api: hang, exit: 1, + code: "network_error", msg: "Request to /account.json timed out after 50ms. Slow engines, such as google_ai_mode"}, + + {name: "paging/jq runs on the merged pages", args: searchWith("--all-pages", "--jq", ".organic_results | length"), api: pages, reqs: 3, stdout: "3\n"}, + + {name: "output/small result is quiet", args: search, api: oneResult, quiet: true}, + {name: "output/large result with --jq is quiet", args: searchWith("--jq", ".organic_results | length"), + api: respond(bigResult("", 1500)), stdout: "1500\n", quiet: true}, + {name: "output/jq printing some values is quiet", args: searchWith("--jq", ".organic_results[0].position, .organic"), api: oneResult, + stdout: "1\n\n", quiet: true}, + {name: "output/jq printing nothing is quiet", args: searchWith("--jq", ".organic_results[] | select(.position > 5)"), api: oneResult, + stdout: "", quiet: true}, + {name: "output/jq streams results before a runtime error", args: searchWith("--jq", `.a[] | . + 1`), api: respond(`{"a":[1,2,"x"]}`), + exit: 2, code: "usage_error", msg: "cannot add", stdout: "2\n3\n"}, + } +} + +func TestEdgeCases(t *testing.T) { + for _, tc := range edgeCases() { + t.Run(tc.name, func(t *testing.T) { + reqs := fakeAPI(t, tc.api) + t.Setenv("SERPAPI_KEY", "envkey") + for k, v := range tc.env { + t.Setenv(k, v) + } + if tc.setup != nil { + tc.setup(t) + } + + res := execute(t, "", tc.args...) + + if tc.exit == 0 { + warnings := wantOK(t, res) + if tc.warn != "" && !strings.Contains(strings.Join(warnings, "\n"), tc.warn) { + t.Errorf("warnings = %q, want one containing %q", warnings, tc.warn) + } + if tc.quiet && len(warnings) > 0 { + t.Errorf("warnings = %q, want none", warnings) + } + } else { + errObj := wantError(t, res, tc.exit, tc.code, tc.msg) + if tc.exact && errObj["message"] != tc.msg { + t.Errorf("message = %q, want exactly %q", errObj["message"], tc.msg) + } + if got, _ := errObj["status"].(float64); int(got) != tc.status { + t.Errorf("status = %v, want %d", errObj["status"], tc.status) + } + } + if (tc.stdout != "" || tc.exit != 0) && res.stdout != tc.stdout { + t.Errorf("stdout = %q, want %q", res.stdout, tc.stdout) + } + if strings.Contains(res.stdout+res.stderr, strayKey) { + t.Errorf("output leaks the key: %s%s", res.stdout, res.stderr) + } + if tc.api == nil { + return + } + want := tc.reqs + if want == 0 { + want = 1 + } + urls := reqs.list() + if len(urls) != want { + t.Fatalf("got %d requests, want %d", len(urls), want) + } + for _, u := range urls { + q := u.Query() + for k, v := range tc.query { + if !q.Has(k) || q.Get(k) != v { + t.Errorf("%s: %s = %q, want %q", u.Path, k, q.Get(k), v) + } + } + for _, k := range tc.noQuery { + if q.Has(k) { + t.Errorf("%s sent %s", u.Path, k) + } + } + } + }) + } +} + +// bigResult is a response of n organic results with only a position each, +// after the given top-level members. +func bigResult(members string, n int) string { + var b strings.Builder + b.WriteString("{" + members + `"organic_results":[`) + for i := 1; i <= n; i++ { + if i > 1 { + b.WriteByte(',') + } + fmt.Fprintf(&b, `{"position":%d}`, i) + } + b.WriteString("]}") + return b.String() +} + +func writeConfig(content string) func(t *testing.T) { + return func(t *testing.T) { + if err := os.MkdirAll(filepath.Dir(config.Path()), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(config.Path(), []byte(content), 0o600); err != nil { + t.Fatal(err) + } + } +} + +func unreadableConfig(t *testing.T) { + if runtime.GOOS == "windows" || os.Geteuid() == 0 { + t.Skip("file permissions do not stop this user from reading") + } + writeConfig(`api_key = "savedkey"`)(t) + if err := os.Chmod(config.Path(), 0); err != nil { + t.Fatal(err) + } +} + +func redirectTo(location string) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, location, http.StatusFound) } +} + +func closeConnection(w http.ResponseWriter, _ *http.Request) { + conn, _, err := w.(http.Hijacker).Hijack() + if err == nil { + if tcp, ok := conn.(*net.TCPConn); ok { + _ = tcp.SetLinger(0) + } + _ = conn.Close() + } +} + +func hang(_ http.ResponseWriter, r *http.Request) { + select { + case <-r.Context().Done(): + case <-time.After(5 * time.Second): + } +} + +// failingWriter fails every write after the first limit bytes, like a pipe +// whose reader went away. +type failingWriter struct{ limit int } + +func (f *failingWriter) Write(p []byte) (int, error) { + if f.limit -= len(p); f.limit < 0 { + return 0, fmt.Errorf("broken pipe") + } + return len(p), nil +} + +func TestEndlessJQOutputStopsWhenStdoutFails(t *testing.T) { + fakeAPI(t, oneResult) + t.Setenv("SERPAPI_KEY", "envkey") + resetFlags(rootCmd) + rootCmd.SetOut(&failingWriter{limit: 1 << 16}) + var stderr strings.Builder + rootCmd.SetErr(&stderr) + rootCmd.SetIn(strings.NewReader("")) + rootCmd.SetArgs(searchWith("--jq", "repeat(1)")) + + done := make(chan int, 1) + go func() { done <- run(context.Background()) }() + select { + case code := <-done: + _, errObj := parseStderr(t, stderr.String()) + if code != 1 || errObj["code"] != "api_error" || !strings.Contains(errObj["message"].(string), "broken pipe") { + t.Errorf("exit %d, error %v; want api_error about the broken pipe", code, errObj) + } + case <-time.After(10 * time.Second): + t.Fatal("endless jq output kept running after stdout failed") + } +} diff --git a/pkg/cmd/fuzz_test.go b/pkg/cmd/fuzz_test.go new file mode 100644 index 0000000..6e94c63 --- /dev/null +++ b/pkg/cmd/fuzz_test.go @@ -0,0 +1,33 @@ +package cmd + +import ( + "encoding/json" + "testing" + "unicode/utf8" +) + +func FuzzMaskAPIKey(f *testing.F) { + f.Add("0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef") + f.Add("short") + f.Add(`quote"inside\backslash`) + f.Add("ключ-ключ-ключ") + f.Add("日本語のキーです日本語") + f.Add("<<<<<<<<<<&&&&") + f.Fuzz(func(t *testing.T, key string) { + quoted, _ := json.Marshal(key) + doc := `{"account_email":"dev@example.com","api_key":` + string(quoted) + `,"total_searches_left":1}` + out := maskAPIKey(json.RawMessage(doc)) + if !json.Valid(out) || utf8.ValidString(key) && !utf8.Valid(out) { + t.Fatalf("masking %q made invalid JSON or UTF-8: %s", key, out) + } + var got struct { + APIKey string `json:"api_key"` + } + if err := json.Unmarshal(out, &got); err != nil { + t.Fatal(err) + } + if r := []rune(key); len(r) >= 9 && got.APIKey != string(r[:4])+"…"+string(r[len(r)-4:]) { + t.Fatalf("key %q masked as %q", key, got.APIKey) + } + }) +} From 28cc8ddee6548b76d952938769a4c02285bc487c Mon Sep 17 00:00:00 2001 From: ilyazub Date: Mon, 12 Oct 2026 01:08:22 +0200 Subject: [PATCH 16/25] [CLI] Exit 130 with an interrupted error on Ctrl-C or SIGTERM The first Ctrl-C or SIGTERM cancels the command's context: an in-flight request stops, --jq keeps what it printed, and the CLI reports {"error":{"code":"interrupted",...}} and exits 130. A second signal kills the process, and if the command is still stuck 3 s after the first, as when stdout is a full pipe, the CLI exits 130 anyway. Before, a signal killed the process with no message. --all-pages prints the pages fetched so far once the paging rewrite later in this stack lands. Co-Authored-By: Claude Opus 5.5 --- pkg/cmd/edge_test.go | 89 ++++++++++++++++++++++++++++++++++++++++++++ pkg/cmd/root.go | 52 ++++++++++++++++++++++++-- 2 files changed, 138 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/edge_test.go b/pkg/cmd/edge_test.go index 34ad9bf..e56ec54 100644 --- a/pkg/cmd/edge_test.go +++ b/pkg/cmd/edge_test.go @@ -1,14 +1,17 @@ package cmd import ( + "bytes" "context" "fmt" + "io" "net" "net/http" "os" "path/filepath" "runtime" "strings" + "sync" "testing" "time" @@ -312,3 +315,89 @@ func TestEndlessJQOutputStopsWhenStdoutFails(t *testing.T) { t.Fatal("endless jq output kept running after stdout failed") } } + +func TestInterruptBeforeTheResponse(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + fakeAPI(t, func(_ http.ResponseWriter, r *http.Request) { + cancel() + <-r.Context().Done() + }) + t.Setenv("SERPAPI_KEY", "envkey") + + res := executeContext(t, ctx, "", search...) + + errObj := wantError(t, res, 130, "interrupted", "Stopped before the response arrived") + if _, ok := errObj["status"]; ok || res.stdout != "" { + t.Errorf("error %v, stdout %q; want no status and no output", errObj, res.stdout) + } +} + +func TestInterruptDuringJQKeepsWhatItPrinted(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + fakeAPI(t, func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprint(w, `{"organic_results":[{"position":1}]}`) + time.AfterFunc(50*time.Millisecond, cancel) + }) + t.Setenv("SERPAPI_KEY", "envkey") + + res := executeContext(t, ctx, "", searchWith("--jq", ".organic_results[0].position, (reduce range(1e12) as $i (0; . + 1))")...) + + wantError(t, res, 130, "interrupted", "Stopped while running --jq") + if res.stdout != "1\n" { + t.Errorf("stdout = %q, want the result printed before the signal", res.stdout) + } +} + +// lockedBuffer is a bytes.Buffer safe to write from another goroutine. +type lockedBuffer struct { + mu sync.Mutex + buf bytes.Buffer +} + +func (b *lockedBuffer) Write(p []byte) (int, error) { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.Write(p) +} + +func (b *lockedBuffer) String() string { + b.mu.Lock() + defer b.mu.Unlock() + return b.buf.String() +} + +func TestExitAfterGrace(t *testing.T) { + grace := interruptGrace + interruptGrace = 20 * time.Millisecond + t.Cleanup(func() { interruptGrace = grace }) + + t.Run("a command stuck after the signal exits 130", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + var stderr lockedBuffer + exited, stopped := make(chan int, 1), false + exitAfterGrace(ctx, make(chan struct{}), func() { stopped = true }, &stderr, func(code int) { exited <- code }) + + if code := <-exited; code != 130 || !stopped { + t.Errorf("exit(%d), stopped %v; want exit(130) after restoring the default handling", code, stopped) + } + if !strings.Contains(stderr.String(), `"code":"interrupted","message":"Stopped 20ms after the signal; the output may be cut off"`) { + t.Errorf("stderr = %q", stderr.String()) + } + }) + t.Run("a command that finishes in time is not cut off", func(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + done := make(chan struct{}) + close(done) + exitAfterGrace(ctx, done, func() {}, io.Discard, func(code int) { t.Errorf("exit(%d) after the command finished", code) }) + }) + t.Run("no signal, no exit", func(t *testing.T) { + done := make(chan struct{}) + close(done) + exitAfterGrace(context.Background(), done, func() { t.Error("stop called without a signal") }, io.Discard, + func(code int) { t.Errorf("exit(%d) without a signal", code) }) + }) +} diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index dff318a..4400d94 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -5,10 +5,14 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" + "io" "os" + "os/signal" "strconv" "strings" + "syscall" "time" "github.com/spf13/cobra" @@ -72,6 +76,8 @@ limited or out of searches; 5xx SerpApi failed. No status means no usable answer: a network_error, such as a timeout, or an unreadable response. Requests time out after 60s: --timeout or SERPAPI_TIMEOUT sets seconds, 0 waits indefinitely. +Exit 130, code interrupted: stopped by Ctrl-C or SIGTERM, after printing what +had arrived. Cost: each successful search counts against your monthly searches, including each page of --all-pages; cached, errored and failed ones don't, though errored @@ -94,18 +100,55 @@ are free.`, }, } -// Execute runs the root command and exits with the code run returns. +// interruptGrace is how long a command may take to wrap up after the first +// signal before the process exits anyway. +var interruptGrace = 3 * time.Second + +// Execute runs the root command. The first Ctrl-C or SIGTERM cancels the +// command's context so it can print what it has; a second one kills it. func Execute() { - os.Exit(run(context.Background())) + ctx, stop := signal.NotifyContext(context.Background(), os.Interrupt, syscall.SIGTERM) + done := make(chan struct{}) + go exitAfterGrace(ctx, done, stop, rootCmd.ErrOrStderr(), os.Exit) + code := run(ctx) + close(done) + stop() + os.Exit(code) +} + +// exitAfterGrace waits for the first signal, restores the default handling +// so that a second one kills the process, and exits 130 if the command is +// still running interruptGrace later, as it is when blocked on a full pipe. +func exitAfterGrace(ctx context.Context, done <-chan struct{}, stop func(), stderr io.Writer, exit func(int)) { + select { + case <-done: + return + case <-ctx.Done(): + } + stop() + select { + case <-done: + case <-time.After(interruptGrace): + // stderr may be the full pipe, so the message gets a moment, not a promise. + go clierrors.FprintError(stderr, &clierrors.InterruptedError{ + Message: fmt.Sprintf("Stopped %s after the signal; the output may be cut off", interruptGrace), + }) + time.Sleep(100 * time.Millisecond) + exit(130) + } } // run executes the root command, reports any error on its stderr as JSON and -// returns the exit code. +// returns the exit code. A command stopped by canceling ctx exits 130. func run(ctx context.Context) int { err := rootCmd.ExecuteContext(ctx) if err == nil { return 0 } + var interrupted *clierrors.InterruptedError + if ctx.Err() != nil && !errors.As(err, &interrupted) { + err = &clierrors.InterruptedError{Message: "Stopped before the response arrived"} + } clierrors.FprintError(rootCmd.ErrOrStderr(), err) return clierrors.ExitCode(err) } @@ -263,6 +306,9 @@ func printJQ(cmd *cobra.Command, v any) error { if writeErr != nil { return outputError(writeErr) } + if err != nil && ctx.Err() != nil { + return &clierrors.InterruptedError{Message: "Stopped while running --jq; what it printed before the signal is on stdout"} + } if err != nil { return &clierrors.UsageError{Message: err.Error()} } From b6233406e5aa76e3c3d9b4b5e178a31f0b3eab17 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Mon, 12 Oct 2026 00:43:44 +0200 Subject: [PATCH 17/25] [Output] Add WriteObject, which encodes an object as it writes it WriteObject writes a map as JSON indented by two spaces, byte for byte what json.Encoder with SetEscapeHTML(false) writes, but encodes array members one element at a time, so no encoded copy of the whole object is held in memory. It can report each member's encoded size. The next commit uses it to print --all-pages output; the tests fuzz it against encoding/json. Co-Authored-By: Claude Opus 5.5 --- pkg/output/stream.go | 94 ++++++++++++++++++++++++++++++++++ pkg/output/stream_test.go | 103 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 197 insertions(+) create mode 100644 pkg/output/stream.go create mode 100644 pkg/output/stream_test.go diff --git a/pkg/output/stream.go b/pkg/output/stream.go new file mode 100644 index 0000000..23169fb --- /dev/null +++ b/pkg/output/stream.go @@ -0,0 +1,94 @@ +package output + +import ( + "bytes" + "encoding/json" + "io" + "maps" + "slices" + "strings" +) + +// WriteObject writes m as JSON indented by two spaces per level, byte for +// byte what json.Encoder with SetEscapeHTML(false) and SetIndent("", " ") +// writes, but encodes array members one element at a time, so an encoded +// copy of the whole object is never held in memory. size, if not nil, gets +// the encoded length of each member's value. WriteObject returns the number +// of bytes written. +func WriteObject(w io.Writer, m map[string]any, size func(key string, n int)) (int64, error) { + cw := &countingWriter{w: w} + var buf bytes.Buffer + enc := json.NewEncoder(&buf) + enc.SetEscapeHTML(false) + // encode writes v as a value that starts at the given nesting depth. + encode := func(v any, depth int) error { + buf.Reset() + enc.SetIndent(indent(depth), " ") + if err := enc.Encode(v); err != nil { + return err + } + cw.Write(bytes.TrimSuffix(buf.Bytes(), []byte("\n"))) + return nil + } + + if len(m) == 0 { + cw.WriteString("{}\n") + return cw.n, cw.err + } + cw.WriteString("{") + for i, k := range slices.Sorted(maps.Keys(m)) { + if i > 0 { + cw.WriteString(",") + } + cw.WriteString("\n" + indent(1)) + if err := encode(k, 1); err != nil { + return cw.n, err + } + cw.WriteString(": ") + start := cw.n + if arr, ok := m[k].([]any); ok && len(arr) > 0 { + cw.WriteString("[") + for j, el := range arr { + if j > 0 { + cw.WriteString(",") + } + cw.WriteString("\n" + indent(2)) + if err := encode(el, 2); err != nil { + return cw.n, err + } + } + cw.WriteString("\n" + indent(1) + "]") + } else if err := encode(m[k], 1); err != nil { + return cw.n, err + } + if size != nil { + size(k, int(cw.n-start)) + } + if cw.err != nil { + return cw.n, cw.err + } + } + cw.WriteString("\n}\n") + return cw.n, cw.err +} + +func indent(depth int) string { return strings.Repeat(" ", depth) } + +// countingWriter counts the bytes written to w and keeps the first error, +// after which it writes nothing. +type countingWriter struct { + w io.Writer + n int64 + err error +} + +func (c *countingWriter) Write(p []byte) { + if c.err != nil { + return + } + n, err := c.w.Write(p) + c.n += int64(n) + c.err = err +} + +func (c *countingWriter) WriteString(s string) { c.Write([]byte(s)) } diff --git a/pkg/output/stream_test.go b/pkg/output/stream_test.go new file mode 100644 index 0000000..c2ec385 --- /dev/null +++ b/pkg/output/stream_test.go @@ -0,0 +1,103 @@ +package output + +import ( + "bytes" + "encoding/json" + "errors" + "testing" +) + +// encoded is what WriteObject must match: json.Encoder indenting by two +// spaces, without HTML escaping. +func encoded(t testing.TB, m map[string]any) []byte { + var buf bytes.Buffer + enc := json.NewEncoder(&buf) + enc.SetEscapeHTML(false) + enc.SetIndent("", " ") + if err := enc.Encode(m); err != nil { + t.Fatal(err) + } + return buf.Bytes() +} + +func decodeObject(data []byte) (map[string]any, bool) { + dec := json.NewDecoder(bytes.NewReader(data)) + dec.UseNumber() + var m map[string]any + return m, dec.Decode(&m) == nil && m != nil +} + +func TestWriteObjectMatchesTheEncoder(t *testing.T) { + inputs := []string{ + `{}`, + `{"organic_results":[{"position":1,"title":"Tom & Jerry","link":"https://e.com/?a=1&b=2"}],"search_metadata":{"id":"abc"}}`, + `{"b":[],"a":[[1,2],[]],"c":null,"d":[null,true,false,1.50,-0,1e400],"é":"\u2028\u2029 \ud83d\ude00 \u0000"}`, + `{"z":{"y":[{"x":[1]}]},"Z":"upper sorts first","":"empty key"}`, + } + for _, in := range inputs { + m, ok := decodeObject([]byte(in)) + if !ok { + t.Fatalf("bad input %s", in) + } + m["nil slice"] = []any(nil) + m["invalid utf-8"] = "a\xffb" + var got bytes.Buffer + n, err := WriteObject(&got, m, nil) + if want := encoded(t, m); err != nil || !bytes.Equal(got.Bytes(), want) || n != int64(len(want)) { + t.Errorf("WriteObject(%s) = %d, %v\n%s\nwant\n%s", in, n, err, got.Bytes(), want) + } + } +} + +func TestWriteObjectReportsEachValueSize(t *testing.T) { + m, _ := decodeObject([]byte(`{"organic_results":[{"position":1},{"position":2}],"search_metadata":{"id":"abc"},"x":"y"}`)) + sizes := map[string]int{} + if _, err := WriteObject(&bytes.Buffer{}, m, func(k string, n int) { sizes[k] = n }); err != nil { + t.Fatal(err) + } + want := map[string]int{ + "organic_results": len("[\n {\n \"position\": 1\n },\n {\n \"position\": 2\n }\n ]"), + "search_metadata": len("{\n \"id\": \"abc\"\n }"), + "x": len(`"y"`), + } + for k, n := range want { + if sizes[k] != n { + t.Errorf("size of %s = %d, want %d", k, sizes[k], n) + } + } +} + +type failAfter struct{ left int } + +func (f *failAfter) Write(p []byte) (int, error) { + if f.left -= len(p); f.left < 0 { + return 0, errors.New("broken pipe") + } + return len(p), nil +} + +func TestWriteObjectStopsAtTheFirstWriteError(t *testing.T) { + m, _ := decodeObject([]byte(`{"a":[1,2,3],"b":[4,5,6]}`)) + _, err := WriteObject(&failAfter{left: 5}, m, nil) + if err == nil || err.Error() != "broken pipe" { + t.Errorf("err = %v, want the write error", err) + } +} + +func FuzzWriteObjectMatchesTheEncoder(f *testing.F) { + f.Add([]byte(`{"a":[1,{"b":"<&>"}],"c":"\u2028"}`)) + f.Add([]byte(`{"organic_results":[],"x":[[[]]],"n":-1.5e-7}`)) + f.Fuzz(func(t *testing.T, data []byte) { + m, ok := decodeObject(data) + if !ok { + return + } + var got bytes.Buffer + if _, err := WriteObject(&got, m, nil); err != nil { + t.Fatal(err) + } + if want := encoded(t, m); !bytes.Equal(got.Bytes(), want) { + t.Errorf("WriteObject(%q)\n%s\nwant\n%s", data, got.Bytes(), want) + } + }) +} From f292b1475bb6245b1b2685b1ca58bb6a9635e272 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Mon, 12 Oct 2026 01:08:55 +0200 Subject: [PATCH 18/25] [Search] Check flags before billing, and page --all-pages safely - search with no parameters, --max-pages below 1, or --all-pages with async=true is a usage error before any request. - --fields keeps the error path, so an error inside the response still shows, and a --fields that matches nothing returns {} with a warning. - --all-pages carries json_restrictor, no_cache, zero_trace and output to every page, since serpapi_pagination.next drops them. It stops at a page --fields matches nothing on, at a repeated page and at --max-pages (default 100, with a warning when more pages exist), and treats a 200 error such as "no results" after the first page as the end. - When a later page fails or Ctrl-C stops the run, the pages fetched so far are printed and the error names them. - A pipe gets the merged pages written as they are encoded. 100 pages of 1 MB from a local fake API, median of 5 runs: peak RSS 722 -> 183 MB, and 669 -> 187 MB with --jq; wall time 0.63 -> 0.58 s. A terminal still gets the buffered, colored form. Co-Authored-By: Claude Opus 5.5 --- pkg/cmd/cmd_test.go | 172 ++++++++++++++++++++ pkg/cmd/edge_test.go | 41 +++++ pkg/cmd/fuzz_test.go | 29 ++++ pkg/cmd/search.go | 358 +++++++++++++++++++++++++++-------------- pkg/cmd/search_test.go | 34 ++-- 5 files changed, 504 insertions(+), 130 deletions(-) diff --git a/pkg/cmd/cmd_test.go b/pkg/cmd/cmd_test.go index 82f557b..49ea3d0 100644 --- a/pkg/cmd/cmd_test.go +++ b/pkg/cmd/cmd_test.go @@ -253,6 +253,22 @@ func TestAPIKeyPrecedence(t *testing.T) { } } +func TestFieldsSendsRestrictorAndWarnsWhenNothingMatches(t *testing.T) { + reqs := fakeAPI(t, respond(`{}`)) + t.Setenv("SERPAPI_KEY", "envkey") + + res := execute(t, "", "search", "engine=google", "q=coffee", "--fields", "organic_results[0:3].{title, link}") + + wantWarnings(t, wantOK(t, res), + "json_restrictor matched nothing or is malformed; compare its paths with the unfiltered response") + if res.stdout != "{}\n" { + t.Errorf("stdout = %q, want {}", res.stdout) + } + wantQuery(t, reqs.only(t), "/search.json", map[string]string{ + "json_restrictor": "organic_results[0:3].{title, link},error", + }) +} + func TestJQPrintsStringsRawAndKeepsNumbers(t *testing.T) { fakeAPI(t, respond(`{"organic_results":[{"link":"https://a.example/?x=1&y=<2>"},{"link":"https://b.example"}],`+ `"search_information":{"total_results":12345678901234567890}}`)) @@ -287,6 +303,162 @@ func pageNumber(r *http.Request) int { return start/10 + 1 } +func organicResults(t *testing.T, stdout string) []any { + t.Helper() + var merged map[string]any + if err := json.Unmarshal([]byte(stdout), &merged); err != nil { + t.Fatalf("stdout is not one JSON object: %v: %q", err, stdout) + } + if _, ok := merged["serpapi_pagination"]; ok { + t.Errorf("merged result keeps serpapi_pagination") + } + results, _ := merged["organic_results"].([]any) + return results +} + +func TestAllPagesMergesPagesAndCarriesParams(t *testing.T) { + reqs := fakeAPI(t, func(w http.ResponseWriter, r *http.Request) { + n := pageNumber(r) + fmt.Fprint(w, page(n, n < 3)) + }) + t.Setenv("SERPAPI_KEY", "envkey") + + res := execute(t, "", "search", "engine=google", "q=coffee", "no_cache=true", + "--fields", "organic_results", "--all-pages") + + wantWarnings(t, wantOK(t, res)) + if got := organicResults(t, res.stdout); len(got) != 3 { + t.Errorf("merged %d results, want 3", len(got)) + } + urls := reqs.list() + if len(urls) != 3 { + t.Fatalf("got %d requests, want 3", len(urls)) + } + for i, u := range urls { + start := "" + if i > 0 { + start = strconv.Itoa(i * 10) + } + wantQuery(t, u, "/search.json", map[string]string{ + "engine": "google", "q": "coffee", "start": start, "no_cache": "true", "api_key": "envkey", + "json_restrictor": "organic_results,error,serpapi_pagination.next", + }) + } +} + +func TestAllPagesStopsAtMaxPages(t *testing.T) { + reqs := fakeAPI(t, func(w http.ResponseWriter, r *http.Request) { + fmt.Fprint(w, page(pageNumber(r), true)) + }) + t.Setenv("SERPAPI_KEY", "envkey") + + res := execute(t, "", "search", "engine=google", "q=coffee", "--all-pages") + wantWarnings(t, wantOK(t, res), + "Stopped at 100 pages, but more exist; raise --max-pages to fetch them (one search each)") + if got := len(reqs.list()); got != 100 { + t.Errorf("default cap made %d requests, want 100", got) + } + + res = execute(t, "", "search", "engine=google", "q=coffee", "--all-pages", "--max-pages", "2") + wantWarnings(t, wantOK(t, res)) + if got := len(organicResults(t, res.stdout)); got != 2 { + t.Errorf("merged %d results, want 2", got) + } + if got := len(reqs.list()); got != 102 { + t.Errorf("--max-pages 2 made %d requests, want 2", got-100) + } +} + +func TestAllPagesStopsWhenNextPageRepeats(t *testing.T) { + reqs := fakeAPI(t, respond(page(1, true))) + t.Setenv("SERPAPI_KEY", "envkey") + + res := execute(t, "", "search", "engine=google", "q=coffee", "--all-pages") + + wantWarnings(t, wantOK(t, res), "Stopped after page 2: the next page repeats an earlier one") + if got := len(reqs.list()); got != 2 { + t.Errorf("got %d requests, want 2", got) + } +} + +func TestAllPagesTreatsNoResultsAsTheEnd(t *testing.T) { + fakeAPI(t, func(w http.ResponseWriter, r *http.Request) { + if pageNumber(r) == 2 { + fmt.Fprint(w, `{"error":"Google hasn't returned any results for this query."}`) + return + } + fmt.Fprint(w, page(1, true)) + }) + t.Setenv("SERPAPI_KEY", "envkey") + + res := execute(t, "", "search", "engine=google", "q=coffee", "--all-pages") + + wantWarnings(t, wantOK(t, res), "Stopped after page 1: Google hasn't returned any results for this query.") + if got := len(organicResults(t, res.stdout)); got != 1 { + t.Errorf("merged %d results, want 1", got) + } +} + +func TestAllPagesPrintsEarlierPagesWhenALaterOneFails(t *testing.T) { + fakeAPI(t, func(w http.ResponseWriter, r *http.Request) { + if pageNumber(r) == 3 { + respondStatus(429, `{"error":"Your account has run out of searches."}`)(w, r) + return + } + fmt.Fprint(w, page(pageNumber(r), true)) + }) + t.Setenv("SERPAPI_KEY", "envkey") + + res := execute(t, "", "search", "engine=google", "q=coffee", "--all-pages") + + errObj := wantError(t, res, 1, "api_error", + "Your account has run out of searches. (page 3 failed; pages 1-2 were printed)") + if errObj["status"] != float64(429) { + t.Errorf("status = %v, want 429", errObj["status"]) + } + if got := len(organicResults(t, res.stdout)); got != 2 { + t.Errorf("merged %d results, want 2", got) + } +} + +func TestAllPagesStopsWhenFieldsMatchNothing(t *testing.T) { + reqs := fakeAPI(t, func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintf(w, `{"serpapi_pagination":{"next":"https://serpapi.com/search.json?engine=google&q=coffee&start=%d"}}`, pageNumber(r)*10) + }) + t.Setenv("SERPAPI_KEY", "envkey") + + res := execute(t, "", "search", "engine=google", "q=coffee", "--fields", "organic_result", "--all-pages") + + wantWarnings(t, wantOK(t, res), + "json_restrictor matched nothing or is malformed; compare its paths with the unfiltered response") + if res.stdout != "{}\n" { + t.Errorf("stdout = %q, want {}", res.stdout) + } + reqs.only(t) +} + +func TestAllPagesStopsAtAPageFieldsMatchNothingOn(t *testing.T) { + reqs := fakeAPI(t, func(w http.ResponseWriter, r *http.Request) { + if n := pageNumber(r); n != 2 { + fmt.Fprint(w, page(n, true)) + return + } + fmt.Fprint(w, `{"serpapi_pagination":{"next":"https://serpapi.com/search.json?engine=google&q=coffee&start=20"}}`) + }) + t.Setenv("SERPAPI_KEY", "envkey") + + res := execute(t, "", "search", "engine=google", "q=coffee", "--fields", "organic_results", "--all-pages") + + wantWarnings(t, wantOK(t, res), + "Stopped after page 2: nothing on it matched json_restrictor; to fetch later pages anyway, filter with --jq instead of --fields") + if got := len(organicResults(t, res.stdout)); got != 1 { + t.Errorf("merged %d results, want 1", got) + } + if got := len(reqs.list()); got != 2 { + t.Errorf("got %d requests, want 2", got) + } +} + func TestMaxPagesWithoutAllPagesWarns(t *testing.T) { reqs := fakeAPI(t, respond(page(1, true))) t.Setenv("SERPAPI_KEY", "envkey") diff --git a/pkg/cmd/edge_test.go b/pkg/cmd/edge_test.go index e56ec54..b6e928d 100644 --- a/pkg/cmd/edge_test.go +++ b/pkg/cmd/edge_test.go @@ -64,6 +64,7 @@ func edgeCases() []edgeCase { invalidKey := `{"error":"Invalid API key. Your API key should be here: https://serpapi.com/manage-api-key"}` pages := func(w http.ResponseWriter, r *http.Request) { fmt.Fprint(w, page(pageNumber(r), pageNumber(r) < 3)) } return []edgeCase{ + {name: "argv/no parameters", args: []string{"search"}, exit: 2, code: "usage_error", msg: "No search parameters given"}, {name: "argv/q and bare words", args: searchWith("tea"), exit: 2, code: "usage_error", msg: "Got both q=... and bare words"}, {name: "argv/api_key parameter", args: searchWith("api_key=" + strayKey), exit: 2, code: "usage_error", msg: "not as the api_key parameter"}, {name: "argv/dash word after --", args: []string{"search", "engine=google", "--", "coffee", "-starbucks"}, api: oneResult, @@ -100,6 +101,8 @@ func edgeCases() []edgeCase { {name: "flags/SERPAPI_TIMEOUT not a number", args: search, env: map[string]string{"SERPAPI_TIMEOUT": "abc"}, exit: 2, code: "usage_error", msg: `Invalid SERPAPI_TIMEOUT value "abc"`}, {name: "flags/--timeout wins over SERPAPI_TIMEOUT", args: searchWith("--timeout", "30"), env: map[string]string{"SERPAPI_TIMEOUT": "abc"}, api: oneResult}, + {name: "flags/max-pages zero", args: searchWith("--all-pages", "--max-pages", "0"), exit: 2, code: "usage_error", msg: "--max-pages must be at least 1"}, + {name: "flags/all-pages with async", args: searchWith("async=true", "--all-pages"), exit: 2, code: "usage_error", msg: "drop async=true"}, {name: "flags/fields outside search", args: []string{"account", "--fields", "plan_name"}, exit: 2, code: "usage_error", msg: "--fields works only with search"}, {name: "flags/fields on locations", args: []string{"locations", "q=Austin", "--fields", "name"}, exit: 2, code: "usage_error", msg: "--fields works only with search"}, {name: "flags/jq parse error", args: []string{"search", "--jq", ".[", "engine=google", "q=coffee"}, exit: 2, code: "usage_error", msg: "jq parse error"}, @@ -146,6 +149,15 @@ func edgeCases() []edgeCase { {name: "response/SERPAPI_TIMEOUT applies", args: []string{"account"}, env: map[string]string{"SERPAPI_TIMEOUT": "0.05"}, api: hang, exit: 1, code: "network_error", msg: "Request to /account.json timed out after 50ms. Slow engines, such as google_ai_mode"}, + {name: "paging/foreign next host and key", args: searchWith("--all-pages"), api: func(w http.ResponseWriter, r *http.Request) { + if pageNumber(r) == 1 { + fmt.Fprint(w, `{"organic_results":[1],"serpapi_pagination":{"next":"https://evil.example/search.json?engine=google&q=coffee&start=10&api_key=stolen"}}`) + return + } + fmt.Fprint(w, `{"organic_results":[2]}`) + }, reqs: 2, stdout: "{\n \"organic_results\": [\n 1,\n 2\n ]\n}\n", query: map[string]string{"api_key": "envkey"}}, + {name: "paging/invalid next URL", args: searchWith("--all-pages"), api: respond(`{"organic_results":[1],"serpapi_pagination":{"next":"http://[::1"}}`), + exit: 1, code: "api_error", msg: "Invalid pagination URL in response: http://[::1 (page 2 failed; page 1 was printed)", stdout: "{\n \"organic_results\": [\n 1\n ]\n}\n"}, {name: "paging/jq runs on the merged pages", args: searchWith("--all-pages", "--jq", ".organic_results | length"), api: pages, reqs: 3, stdout: "3\n"}, {name: "output/small result is quiet", args: search, api: oneResult, quiet: true}, @@ -316,6 +328,35 @@ func TestEndlessJQOutputStopsWhenStdoutFails(t *testing.T) { } } +func TestInterruptDuringAllPagesPrintsFetchedPages(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + reached := make(chan struct{}) + reqs := fakeAPI(t, func(w http.ResponseWriter, r *http.Request) { + if n := pageNumber(r); n < 3 { + fmt.Fprint(w, page(n, true)) + return + } + close(reached) + <-r.Context().Done() + }) + t.Setenv("SERPAPI_KEY", "envkey") + go func() { + <-reached + cancel() + }() + + res := executeContext(t, ctx, "", searchWith("--all-pages")...) + + wantError(t, res, 130, "interrupted", "Stopped while fetching page 3; pages 1-2 were printed") + if got := len(organicResults(t, res.stdout)); got != 2 { + t.Errorf("printed %d results, want the 2 fetched pages", got) + } + if got := len(reqs.list()); got != 3 { + t.Errorf("got %d requests, want 3", got) + } +} + func TestInterruptBeforeTheResponse(t *testing.T) { ctx, cancel := context.WithCancel(context.Background()) defer cancel() diff --git a/pkg/cmd/fuzz_test.go b/pkg/cmd/fuzz_test.go index 6e94c63..9e46aa1 100644 --- a/pkg/cmd/fuzz_test.go +++ b/pkg/cmd/fuzz_test.go @@ -2,6 +2,7 @@ package cmd import ( "encoding/json" + "net/url" "testing" "unicode/utf8" ) @@ -31,3 +32,31 @@ func FuzzMaskAPIKey(f *testing.F) { } }) } + +func FuzzParseNextParams(f *testing.F) { + f.Add("https://serpapi.com/search.json?engine=google&q=coffee&start=10", "organic_results") + f.Add("http://[::1", "") + f.Add("/search.json?q=%zz&q=a&json_restrictor=x", "error") + f.Fuzz(func(t *testing.T, next, restrictor string) { + carried := map[string]string{} + if restrictor != "" { + carried["json_restrictor"] = restrictor + } + got, err := parseNextParams(next, carried) + if err != nil { + return + } + if restrictor != "" && got["json_restrictor"] != restrictor { + t.Fatalf("carried json_restrictor %q lost: %v", restrictor, got) + } + back, err := url.ParseQuery(canonicalParamsKey(got)) + if err != nil || len(back) != len(got) { + t.Fatalf("canonical key %q does not round-trip %v", canonicalParamsKey(got), got) + } + for k, v := range got { + if back.Get(k) != v { + t.Fatalf("canonical key %q changed %s=%q to %q", canonicalParamsKey(got), k, v, back.Get(k)) + } + } + }) +} diff --git a/pkg/cmd/search.go b/pkg/cmd/search.go index 2bf6b84..7d407b8 100644 --- a/pkg/cmd/search.go +++ b/pkg/cmd/search.go @@ -1,17 +1,20 @@ package cmd import ( + "bufio" "bytes" "encoding/json" "errors" "fmt" + "net/http" "net/url" - "sort" "github.com/spf13/cobra" + "github.com/serpapi/serpapi-cli/pkg/api" "github.com/serpapi/serpapi-cli/pkg/config" clierrors "github.com/serpapi/serpapi-cli/pkg/errors" + "github.com/serpapi/serpapi-cli/pkg/output" "github.com/serpapi/serpapi-cli/pkg/params" ) @@ -20,37 +23,87 @@ var ( maxPagesFlag int ) -const ( - defaultMaxPages = 100 - paginationRestrictorPath = "serpapi_pagination.next" -) +const defaultMaxPages = 100 + +// carriedParams are dropped from serpapi_pagination.next but still apply to +// every page. +var carriedParams = []string{"json_restrictor", "no_cache", "zero_trace", "output"} var searchCmd = &cobra.Command{ - Use: "search [PARAMS...]", - Short: "Perform a search with any supported SerpApi engine", + Use: "search PARAMS...", + Short: "Run a search with any SerpApi engine", + Long: `Run a search. Parameters are key=value pairs, starting with engine=. +Words without "=" are joined into q. Some engines name the query differently: +youtube search_query, amazon k, ebay _nkw, walmart and naver query, yahoo p, +yandex text. Parameters of every engine: https://serpapi.com/search-engine-apis + +--fields (the json_restrictor parameter) makes the server return only the +listed paths: https://serpapi.com/json-restrictor + organic_results[0:3] first three results + organic_results[].{title, link} two fields of every result + search_metadata.id,local_results[0] several paths, comma-separated +Unknown paths are dropped silently; if nothing matches you get {} and a warning. +The server also drops null, false and empty values. --fields is part of the +cache key, so a different --fields can miss the cache and count as a new +search; --jq filters the response locally without changing the request. + +Responses are often 20-300 KB, and one key can be large too: Maps +place_results holds reviews and photos. To see the shape of an unfamiliar +response, print a sample with one item per array; nested arrays are cut too, +so it is not the answer: + --jq 'walk(if type == "array" then .[:1] else . end)' +Then take what you need from the stored search, which is free: + serpapi archive SEARCH_METADATA_ID --jq '.place_results | {title, rating, reviews}' + +--all-pages follows serpapi_pagination.next and merges the arrays of every page +into one JSON object, up to --max-pages pages, one search each. It stops at a +page that --fields matches nothing on; use --jq to filter such pages instead. +If a later page fails, or Ctrl-C stops the run, the pages fetched so far are +printed and the error says how many. + +async=true returns at once with search_metadata.id; collect the results later +with 'serpapi archive ID'.`, Example: ` serpapi search engine=google q=coffee - serpapi search engine=google_light q="weather in Tokyo" - serpapi search engine=google_maps q="pizza" ll="@40.7455096,-74.0083012,14z" - serpapi search engine=google q=coffee --jq ".organic_results[:3]" - serpapi search engine=google q=coffee --all-pages --max-pages 3`, + serpapi search engine=google coffee shops --fields "organic_results[].{title, link}" + serpapi search engine=youtube search_query="lofi beats" --jq ".video_results[:3][].link" + serpapi search engine=google_maps q=pizza ll="@40.7455096,-74.0083012,14z" --fields "local_results[0:5]" + serpapi search engine=google q=coffee --all-pages --max-pages 3 --jq ".organic_results | length"`, Args: cobra.ArbitraryArgs, RunE: runSearch, } func init() { - searchCmd.Flags().BoolVar(&allPagesFlag, "all-pages", false, "Fetch all pages and merge array results") - searchCmd.Flags().IntVar(&maxPagesFlag, "max-pages", 0, "Maximum number of pages to fetch when paginating with --all-pages") + searchCmd.Flags().BoolVar(&allPagesFlag, "all-pages", false, "follow pagination and merge the arrays of every page") + searchCmd.Flags().IntVar(&maxPagesFlag, "max-pages", defaultMaxPages, "page limit for --all-pages; each page is one search") rootCmd.AddCommand(searchCmd) } func runSearch(cmd *cobra.Command, args []string) error { + if len(args) == 0 { + return &clierrors.UsageError{Message: "No search parameters given, e.g. serpapi search engine=google q=coffee"} + } parsed, err := params.ParseAll(args) if err != nil { return err } + p := params.ParamsToMap(parsed) - paramsMap := params.ParamsToMap(parsed) - params.ApplyFields(paramsMap, fieldsFlag) + maxPagesSet := cmd.Flags().Changed("max-pages") + if maxPagesSet && maxPagesFlag < 1 { + return &clierrors.UsageError{Message: "--max-pages must be at least 1"} + } + if a := p["async"]; allPagesFlag && a != "" && a != "false" && a != "0" { + return &clierrors.UsageError{Message: "--all-pages cannot follow an async search; drop async=" + a} + } + if maxPagesSet && !allPagesFlag { + warn(cmd, "--max-pages has no effect without --all-pages") + } + + keep := []string{"error"} + if allPagesFlag { + keep = append(keep, "serpapi_pagination.next") + } + restrictor := params.ApplyFields(p, fieldsFlag, keep...) // Search runs without a key, as in v0.2: the API answers a few demo // queries without one and returns 401 for the rest. @@ -59,170 +112,241 @@ func runSearch(cmd *cobra.Command, args []string) error { if keyErr != config.ErrNoAPIKey && errors.As(keyErr, &cfgErr) { warn(cmd, "%s; searching without a key", cfgErr.Message) } - client := newClient(apiKey) - - hasMaxPages := cmd.Flags().Changed("max-pages") + return sendSearch(cmd, newClient(apiKey), p, restrictor != "") +} - if hasMaxPages && !allPagesFlag { - warn(cmd, "--max-pages has no effect without --all-pages") +func sendSearch(cmd *cobra.Command, client *api.Client, p map[string]string, restricted bool) error { + ctx := cmd.Context() + if allPagesFlag { + return searchAllPages(cmd, client, p, restricted) } - if !allPagesFlag { - raw, err := withSpinner("Searching...", func() (json.RawMessage, error) { return client.Search(cmd.Context(), paramsMap) }) - if err != nil { - return err - } - return handleOutput(cmd, raw) + result, err := withSpinner("Searching...", func() (json.RawMessage, error) { return client.Search(ctx, p) }) + if err != nil { + return err } - - // Pagination mode - maxPages := defaultMaxPages - if hasMaxPages { - maxPages = maxPagesFlag + if restricted && isEmptyObject(result) { + warnNothingMatched(cmd) } + return handleOutput(cmd, result) +} - paginationRestrictor := withPaginationRestrictor(paramsMap["json_restrictor"]) - if paginationRestrictor != "" { - paramsMap["json_restrictor"] = paginationRestrictor - } +func isEmptyObject(raw []byte) bool { + var m map[string]json.RawMessage + return json.Unmarshal(raw, &m) == nil && m != nil && len(m) == 0 +} + +func warnNothingMatched(cmd *cobra.Command) { + warn(cmd, "json_restrictor matched nothing or is malformed; compare its paths with the unfiltered response") +} - currentParams := paramsMap - var accumulated map[string]any - visitedPages := make(map[string]bool) - pagesFetched := 0 - visitedPages[canonicalParamsKey(currentParams)] = true +// searchAllPages fetches page after page and prints the merged result. Pages +// already fetched are printed even when a later page fails. +func searchAllPages(cmd *cobra.Command, client *api.Client, first map[string]string, restricted bool) error { + carried := make(map[string]string) + for _, k := range carriedParams { + if v, ok := first[k]; ok { + carried[k] = v + } + } + ctx := cmd.Context() + pageParams := first + seen := map[string]bool{canonicalParamsKey(first): true} + var merged map[string]any + pages := 0 + var pageErr error for { - raw, err := withSpinner(fmt.Sprintf("Fetching page %d...", pagesFetched+1), func() (json.RawMessage, error) { - return client.Search(cmd.Context(), currentParams) + raw, err := withSpinner(fmt.Sprintf("Fetching page %d...", pages+1), func() (json.RawMessage, error) { + return client.Search(ctx, pageParams) }) - if err != nil { - return err + var page map[string]any + if err == nil { + dec := json.NewDecoder(bytes.NewReader(raw)) + dec.UseNumber() + if decErr := dec.Decode(&page); decErr != nil { + err = &clierrors.APIError{Message: "Failed to parse response: " + decErr.Error()} + } } - pagesFetched++ - - var result map[string]any - dec := json.NewDecoder(bytes.NewReader(raw)) - dec.UseNumber() - if err := dec.Decode(&result); err != nil { - return &clierrors.APIError{Message: "Failed to parse response: " + err.Error()} + if err != nil { + if pages == 0 { + return err + } + pageErr = err + break } + pages++ - nextURL := extractNextURL(result) - - if accumulated == nil { - accumulated = result + next := extractNextURL(page) + if merged == nil { + merged = page } else { - mergeArrayFields(accumulated, result) + mergeArrayFields(merged, page) } - - if nextURL == "" { + if next == "" { break } - - if pagesFetched >= maxPages { + // Following next anyway would bill a search for every remaining page + // of a mistyped restrictor. + if restricted && !hasData(page) { + if pages > 1 { + warn(cmd, "Stopped after page %d: nothing on it matched json_restrictor; to fetch later pages anyway, filter with --jq instead of --fields", pages) + } break } - - nextParams, err := parseNextParams(nextURL, paginationRestrictor) + if pages >= maxPagesFlag { + if !cmd.Flags().Changed("max-pages") { + warn(cmd, "Stopped at %d pages, but more exist; raise --max-pages to fetch them (one search each)", pages) + } + break + } + nextParams, err := parseNextParams(next, carried) if err != nil { - return err + pageErr = err + break } - - canonical := canonicalParamsKey(nextParams) - if visitedPages[canonical] { + key := canonicalParamsKey(nextParams) + if seen[key] { + warn(cmd, "Stopped after page %d: the next page repeats an earlier one", pages) break } - visitedPages[canonical] = true - currentParams = nextParams + seen[key] = true + pageParams = nextParams } - if accumulated == nil { - return &clierrors.APIError{Message: "No results returned"} + if merged == nil { + merged = map[string]any{} + } + delete(merged, "serpapi_pagination") + if restricted && len(merged) == 0 { + warnNothingMatched(cmd) } - delete(accumulated, "serpapi_pagination") + if err := printMerged(cmd, merged); err != nil { + return err + } + if pageErr == nil { + return nil + } + if ctx.Err() != nil { + return &clierrors.InterruptedError{Message: fmt.Sprintf("Stopped while fetching page %d; %s printed", pages+1, printedPages(pages))} + } - var buf bytes.Buffer - enc := json.NewEncoder(&buf) - enc.SetEscapeHTML(false) - if err := enc.Encode(accumulated); err != nil { - return &clierrors.APIError{Message: "Failed to encode result: " + err.Error()} + // A 200 with an error body, such as "no results", usually means the + // previous page was the last one. + var apiErr *clierrors.APIError + if errors.As(pageErr, &apiErr) && apiErr.Status == http.StatusOK { + warn(cmd, "Stopped after page %d: %s", pages, apiErr.Message) + return nil } - return handleOutput(cmd, json.RawMessage(bytes.TrimRight(buf.Bytes(), "\n"))) + return annotatePageError(pageErr, pages) } -// withPaginationRestrictor adds the pagination field needed internally while -// preserving the caller's server-side field projection. -func withPaginationRestrictor(restrictor string) string { - if restrictor == "" { - return "" +// printMerged prints the merged pages. --jq runs on them directly, and a +// pipe gets them encoded as they are written, so no encoded copy of every +// page is ever held in memory. +func printMerged(cmd *cobra.Command, merged map[string]any) error { + if jqFlag != "" { + return printJQ(cmd, merged) } - return restrictor + "," + paginationRestrictorPath + w := cmd.OutOrStdout() + if output.IsTerminal(w) { + var buf bytes.Buffer + enc := json.NewEncoder(&buf) + enc.SetEscapeHTML(false) + if err := enc.Encode(merged); err != nil { + return &clierrors.APIError{Message: "Failed to encode result: " + err.Error()} + } + return outputError(output.PrintJSON(w, buf.Bytes())) + } + bw := bufio.NewWriterSize(w, 64<<10) + if _, err := output.WriteObject(bw, merged, nil); err != nil { + return outputError(err) + } + return outputError(bw.Flush()) } -// extractNextURL pulls the next pagination URL from a search result. -func extractNextURL(result map[string]any) string { - pag, ok := result["serpapi_pagination"] - if !ok { - return "" +func printedPages(pages int) string { + if pages > 1 { + return fmt.Sprintf("pages 1-%d were", pages) } - pagMap, ok := pag.(map[string]any) - if !ok { - return "" + return "page 1 was" +} + +// annotatePageError adds which page failed and what was printed, keeping the +// error's type, status and cause. +func annotatePageError(err error, pages int) error { + note := fmt.Sprintf(" (page %d failed; %s printed)", pages+1, printedPages(pages)) + + var apiErr *clierrors.APIError + var netErr *clierrors.NetworkError + switch { + case errors.As(err, &apiErr): + return &clierrors.APIError{Message: apiErr.Message + note, Status: apiErr.Status, Cause: apiErr.Cause} + case errors.As(err, &netErr): + return &clierrors.NetworkError{Message: netErr.Message + note, Cause: netErr.Cause} + } + return fmt.Errorf("%w%s", err, note) +} + +// hasData reports whether a page has anything besides pagination links. +func hasData(page map[string]any) bool { + for k := range page { + if k != "serpapi_pagination" { + return true + } } - next, ok := pagMap["next"] + return false +} + +// extractNextURL pulls the next pagination URL from a search result. +func extractNextURL(result map[string]any) string { + pag, ok := result["serpapi_pagination"].(map[string]any) if !ok { return "" } - s, _ := next.(string) - return s + next, _ := pag["next"].(string) + return next } -// mergeArrayFields appends array values from src into dst; scalar fields are overwritten. +// mergeArrayFields appends array values from src into dst; other fields are overwritten. func mergeArrayFields(dst, src map[string]any) { for key, val := range src { if newItems, ok := val.([]any); ok { - if existing, ok := dst[key]; ok { - if existingArr, ok := existing.([]any); ok { - dst[key] = append(existingArr, newItems...) - continue - } + if existing, ok := dst[key].([]any); ok { + dst[key] = append(existing, newItems...) + continue } } dst[key] = val } } -// parseNextParams extracts query parameters from a full URL and reapplies the -// internal restrictor because pagination URLs may omit response-shaping params. -func parseNextParams(nextURL, jsonRestrictor string) (map[string]string, error) { - parsed, err := url.Parse(nextURL) - if err != nil { - return nil, &clierrors.NetworkError{Message: "Invalid pagination URL: " + err.Error()} +// parseNextParams returns the query of a serpapi_pagination.next URL plus the +// carried parameters. The URL's host and path are ignored, so requests stay on +// the configured server and endpoint. +func parseNextParams(next string, carried map[string]string) (map[string]string, error) { + u, err := url.Parse(next) + if err != nil || u.RawQuery == "" { + return nil, &clierrors.APIError{Message: "Invalid pagination URL in response: " + next} } result := make(map[string]string) - for k, v := range parsed.Query() { + for k, v := range u.Query() { if len(v) > 0 { result[k] = v[0] } } - if jsonRestrictor != "" { - result["json_restrictor"] = jsonRestrictor + for k, v := range carried { + result[k] = v } return result, nil } // canonicalParamsKey produces a stable key from query params regardless of order. func canonicalParamsKey(p map[string]string) string { - keys := make([]string, 0, len(p)) - for k := range p { - keys = append(keys, k) - } - sort.Strings(keys) - vals := url.Values{} - for _, k := range keys { - vals.Set(k, p[k]) + vals := make(url.Values, len(p)) + for k, v := range p { + vals.Set(k, v) } return vals.Encode() } diff --git a/pkg/cmd/search_test.go b/pkg/cmd/search_test.go index 258539d..d1d833f 100644 --- a/pkg/cmd/search_test.go +++ b/pkg/cmd/search_test.go @@ -20,23 +20,11 @@ func TestCanonicalParamsKeyIsOrderIndependent(t *testing.T) { } } -func TestWithPaginationRestrictor(t *testing.T) { - if got := withPaginationRestrictor(""); got != "" { - t.Errorf("expected empty restrictor, got %q", got) - } - - const fields = "organic_results[].{title,link}" - want := fields + ",serpapi_pagination.next" - if got := withPaginationRestrictor(fields); got != want { - t.Errorf("expected %q, got %q", want, got) - } -} - func TestParseNextParams(t *testing.T) { const restrictor = "organic_results[].{title,link},serpapi_pagination.next" params, err := parseNextParams( "https://serpapi.com/search.json?q=coffee&start=10&engine=google", - restrictor, + map[string]string{"json_restrictor": restrictor}, ) if err != nil { t.Fatal(err) @@ -54,3 +42,23 @@ func TestParseNextParams(t *testing.T) { t.Errorf("expected json_restrictor=%q, got %q", restrictor, params["json_restrictor"]) } } + +func TestParseNextParamsCarriedParamsWin(t *testing.T) { + next := "https://serpapi.com/search.json?engine=google&q=coffee&start=10&output=html" + params, err := parseNextParams(next, map[string]string{"output": "json", "no_cache": "true"}) + if err != nil { + t.Fatal(err) + } + want := map[string]string{"engine": "google", "q": "coffee", "start": "10", "output": "json", "no_cache": "true"} + if canonicalParamsKey(params) != canonicalParamsKey(want) { + t.Errorf("got %v, want %v", params, want) + } +} + +func TestParseNextParamsRejectsURLWithoutQuery(t *testing.T) { + for _, next := range []string{"", "https://serpapi.com/search.json", "%zz"} { + if _, err := parseNextParams(next, nil); err == nil { + t.Errorf("parseNextParams(%q) succeeded", next) + } + } +} From 2c0ffa669a1be7d3c4f55ad95809df17bdb1579b Mon Sep 17 00:00:00 2001 From: ilyazub Date: Mon, 12 Oct 2026 01:09:18 +0200 Subject: [PATCH 19/25] [Output] Indent piped JSON the way --all-pages streams it A pipe, such as an agent reading stdout, now gets JSON indented by json.Indent: the bytes json.Encoder with SetIndent and WriteObject write, so a single search and merged --all-pages pages are formatted alike. tidwall/pretty, used for pipes until now, kept a short array on one line; the two agree on most responses. A body that isn't JSON comes back as is, and terminal output is unchanged. Co-Authored-By: Claude Opus 5.5 --- pkg/output/output.go | 19 +++++++++++--- pkg/output/output_test.go | 54 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 4 deletions(-) diff --git a/pkg/output/output.go b/pkg/output/output.go index e380cee..bd5c479 100644 --- a/pkg/output/output.go +++ b/pkg/output/output.go @@ -48,9 +48,10 @@ func PrintJSON(w io.Writer, data []byte) error { } // FormatJSON returns a JSON document as it is shown on w, preserving key -// order. A terminal gets it indented, through ForTerminal, and colored -// (unless NO_COLOR is set); anything else, such as a pipe, gets it indented as -// tidwall/pretty formats it. +// order. A terminal gets it indented and colored (unless NO_COLOR is set); +// anything else, such as a pipe or an agent reading stdout, gets it indented +// by two spaces per level, so that head, grep and line-based file viewers show +// part of it. Input that isn't JSON comes back as is. func FormatJSON(w io.Writer, data []byte) []byte { if IsTerminal(w) { out := ForTerminal(pretty.Pretty(data)) @@ -59,7 +60,17 @@ func FormatJSON(w io.Writer, data []byte) []byte { } return out } - return pretty.Pretty(data) + + var buf bytes.Buffer + buf.Grow(len(data) + len(data)/4 + 1) + // Only JSON's own whitespace: json.Indent keeps trailing spaces, and + // trimming Unicode spaces too would accept a body that isn't JSON. + if err := json.Indent(&buf, bytes.TrimRight(data, " \t\r\n"), "", " "); err != nil { + buf.Reset() + buf.Write(bytes.TrimSpace(data)) + } + buf.WriteByte('\n') + return buf.Bytes() } // PrintRaw writes a non-JSON response body (Markdown, HTML, CSV) to w as is, diff --git a/pkg/output/output_test.go b/pkg/output/output_test.go index f8b6dc1..c14b3aa 100644 --- a/pkg/output/output_test.go +++ b/pkg/output/output_test.go @@ -2,6 +2,8 @@ package output import ( "bytes" + "encoding/json" + "reflect" "strings" "testing" ) @@ -76,6 +78,16 @@ func TestPrintJSONIndentsWhenNotATerminal(t *testing.T) { } } +func TestPrintJSONKeepsInvalidInput(t *testing.T) { + var buf bytes.Buffer + if err := PrintJSON(&buf, []byte("not json\n")); err != nil { + t.Fatal(err) + } + if buf.String() != "not json\n" { + t.Errorf("got %q", buf.String()) + } +} + func TestPrintRaw(t *testing.T) { for in, want := range map[string]string{"": "", "# Title": "# Title\n", "a\n": "a\n"} { var buf bytes.Buffer @@ -87,3 +99,45 @@ func TestPrintRaw(t *testing.T) { } } } + +func FuzzPrintJSONPiped(f *testing.F) { + f.Add([]byte(`{"a": [1, 2.50, 1e400, 12345678901234567890], "b": "<&> café \u2028"}`)) + f.Add([]byte(` null `)) + f.Add([]byte(`{"truncated":`)) + f.Add([]byte("{\"\":0}\n\u0085")) + f.Add([]byte("{\"a\":1}\u00a0")) + f.Fuzz(func(t *testing.T, data []byte) { + var buf bytes.Buffer + if err := PrintJSON(&buf, data); err != nil { + t.Fatal(err) + } + out := buf.Bytes() + if !json.Valid(data) { + if want := append(bytes.TrimSpace(data), '\n'); !bytes.Equal(out, want) { + t.Fatalf("invalid JSON %q changed to %q", data, out) + } + return + } + var want bytes.Buffer + if err := json.Indent(&want, bytes.TrimRight(data, " \t\r\n"), "", " "); err != nil { + t.Fatal(err) + } + if !bytes.Equal(out, append(want.Bytes(), '\n')) { + t.Fatalf("piped output is not the indented input: %q -> %q", data, out) + } + if a, b := decodeNumbers(t, data), decodeNumbers(t, out); !reflect.DeepEqual(a, b) { + t.Fatalf("indenting changed the value: %q -> %q", data, out) + } + }) +} + +func decodeNumbers(t *testing.T, data []byte) any { + t.Helper() + dec := json.NewDecoder(bytes.NewReader(data)) + dec.UseNumber() + var v any + if err := dec.Decode(&v); err != nil { + t.Fatal(err) + } + return v +} From 720ec774f30344e338bc71efd909cb974c511108 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Mon, 12 Oct 2026 00:45:25 +0200 Subject: [PATCH 20/25] [CLI] Point agents at the fix, print output=md, add serpapi version - A flag guessed for a parameter, such as --num 10 or -e, gets "API parameters are key=value pairs, e.g. num=VALUE"; a search word starting with - is told to go inside q="..." or after --. - jq errors add the fix: ? for a value of another type, []? for a missing array, and the response's top-level keys for a path that isn't there. A filter that prints only null gets a warning listing those keys. - A piped result over 16 KB, which agent tools stop showing inline, comes with a warning that lists its 12 largest top-level keys by size and the free serpapi archive ID --jq to take parts of it, unless stderr is the same pipe (2>&1), where it would break a JSON parser. - A parameter given twice keeps its last value, with a warning; a 401 for a search sent without a key says how to give one. - output=md, html, raw_html, plain, csv and rank:DOMAIN print as is in search and archive. --jq or --all-pages with them, and unknown values such as output=xml, are usage errors before the search runs. - serpapi version prints what --version does. Co-Authored-By: Claude Opus 5.5 --- pkg/cmd/account.go | 2 +- pkg/cmd/archive.go | 41 +++++++++++-- pkg/cmd/cmd_test.go | 36 +++++++++++ pkg/cmd/edge_test.go | 54 ++++++++++++++++ pkg/cmd/hint.go | 143 +++++++++++++++++++++++++++++++++++++++++++ pkg/cmd/hint_test.go | 104 +++++++++++++++++++++++++++++++ pkg/cmd/locations.go | 3 +- pkg/cmd/root.go | 76 +++++++++++++++++++++-- pkg/cmd/search.go | 90 ++++++++++++++++++++++++--- pkg/cmd/version.go | 27 ++++++++ 10 files changed, 556 insertions(+), 20 deletions(-) create mode 100644 pkg/cmd/hint.go create mode 100644 pkg/cmd/hint_test.go create mode 100644 pkg/cmd/version.go diff --git a/pkg/cmd/account.go b/pkg/cmd/account.go index 6c8d4a4..ff42659 100644 --- a/pkg/cmd/account.go +++ b/pkg/cmd/account.go @@ -34,7 +34,7 @@ func runAccount(cmd *cobra.Command, _ []string) error { if err != nil { return err } - return handleOutput(cmd, maskAPIKey(result)) + return handleOutput(cmd, maskAPIKey(result), nil) } var reAPIKey = regexp.MustCompile(`("api_key"\s*:\s*)("(?:[^"\\]|\\.)*")`) diff --git a/pkg/cmd/archive.go b/pkg/cmd/archive.go index 288d5c1..9cbcb08 100644 --- a/pkg/cmd/archive.go +++ b/pkg/cmd/archive.go @@ -3,23 +3,27 @@ package cmd import ( "encoding/json" "regexp" + "strings" "github.com/spf13/cobra" clierrors "github.com/serpapi/serpapi-cli/pkg/errors" + "github.com/serpapi/serpapi-cli/pkg/output" ) var reArchiveID = regexp.MustCompile(`^[a-zA-Z0-9_-]+$`) var archiveCmd = &cobra.Command{ - Use: "archive ID", + Use: "archive ID [output=FORMAT]", Short: "Fetch an earlier search by its search_metadata.id (free)", Long: `Fetch an earlier search by the ID in its search_metadata.id. This request is free, and it is how you collect the results of a search made with async=true: -until search_metadata.status is Success it is Queued or Processing, so poll.`, +until search_metadata.status is Success it is Queued or Processing, so poll. +output= takes the same values as in search, such as md for Markdown.`, Example: ` serpapi archive 6a2b3c4d5e6f7a8b9c0d1e2f + serpapi archive 6a2b3c4d5e6f7a8b9c0d1e2f output=md serpapi archive 6a2b3c4d5e6f7a8b9c0d1e2f --jq ".organic_results[0]"`, - Args: cobra.ExactArgs(1), + Args: cobra.RangeArgs(1, 2), RunE: runArchive, } @@ -32,15 +36,42 @@ func runArchive(cmd *cobra.Command, args []string) error { if !reArchiveID.MatchString(id) { return &clierrors.UsageError{Message: "Invalid archive ID: must contain only alphanumeric characters, dashes, and underscores"} } + format := "json" + if len(args) == 2 { + v, ok := strings.CutPrefix(args[1], "output=") + if !ok { + return &clierrors.UsageError{Message: "Unexpected argument " + args[1] + "; archive takes an ID and optionally output=FORMAT"} + } + format = v + } + raw, err := isRawOutput(format) + if err != nil { + return err + } + if raw && jqFlag != "" { + return &clierrors.UsageError{Message: "--jq needs JSON, but output=" + format + " is not JSON"} + } apiKey, err := resolveAPIKey() if err != nil { return err } client := newClient(apiKey) - result, err := withSpinner("Fetching archive...", func() (json.RawMessage, error) { return client.Archive(cmd.Context(), id) }) + ctx := cmd.Context() + + if format == "json" { + result, err := withSpinner("Fetching archive...", func() (json.RawMessage, error) { return client.Archive(ctx, id) }) + if err != nil { + return err + } + return handleOutput(cmd, result, archiveHint) + } + body, err := withSpinner("Fetching archive...", func() ([]byte, error) { return client.ArchiveRaw(ctx, id, format) }) if err != nil { return err } - return handleOutput(cmd, result) + if raw { + return outputError(output.PrintRaw(cmd.OutOrStdout(), body)) + } + return handleOutput(cmd, body, archiveHint) } diff --git a/pkg/cmd/cmd_test.go b/pkg/cmd/cmd_test.go index 49ea3d0..1435c90 100644 --- a/pkg/cmd/cmd_test.go +++ b/pkg/cmd/cmd_test.go @@ -487,6 +487,42 @@ func TestAccountMasksAPIKey(t *testing.T) { } } +func TestArchiveOutputFormats(t *testing.T) { + reqs := fakeAPI(t, func(w http.ResponseWriter, r *http.Request) { + switch r.URL.Query().Get("output") { + case "md": + fmt.Fprint(w, "# Coffee") + case "rank:example.com": + fmt.Fprint(w, "3") + default: + fmt.Fprint(w, `{"search_metadata":{"id":"abc_123-X"},"organic_results":[{"link":"https://example.com"}]}`) + } + }) + t.Setenv("SERPAPI_KEY", "envkey") + + tests := []struct { + args []string + output, stdout string + }{ + {nil, "", "{\n \"search_metadata\": {\n \"id\": \"abc_123-X\"\n },\n \"organic_results\": [\n {\n \"link\": \"https://example.com\"\n }\n ]\n}\n"}, + {[]string{"output=md"}, "md", "# Coffee\n"}, + {[]string{"output=rank:example.com"}, "rank:example.com", "3\n"}, + {[]string{"output=json_with_images", "--jq", ".organic_results[0].link"}, "json_with_images", "https://example.com\n"}, + } + for i, tt := range tests { + res := execute(t, "", append([]string{"archive", "abc_123-X"}, tt.args...)...) + wantOK(t, res) + if res.stdout != tt.stdout { + t.Errorf("%v: stdout = %q, want %q", tt.args, res.stdout, tt.stdout) + } + urls := reqs.list() + if len(urls) != i+1 { + t.Fatalf("%v: got %d requests in total, want %d", tt.args, len(urls), i+1) + } + wantQuery(t, urls[i], "/searches/abc_123-X.json", map[string]string{"output": tt.output, "api_key": "envkey"}) + } +} + func TestLocationsSendsNoAPIKey(t *testing.T) { reqs := fakeAPI(t, respond(`[{"canonical_name":"Austin,TX,Texas,United States"}]`)) t.Setenv("SERPAPI_KEY", "envkey") diff --git a/pkg/cmd/edge_test.go b/pkg/cmd/edge_test.go index b6e928d..a344ca9 100644 --- a/pkg/cmd/edge_test.go +++ b/pkg/cmd/edge_test.go @@ -16,6 +16,7 @@ import ( "time" "github.com/serpapi/serpapi-cli/pkg/config" + "github.com/serpapi/serpapi-cli/pkg/version" ) // edgeCase is one edge case of the CLI. Cases are grouped by the one input @@ -67,22 +68,37 @@ func edgeCases() []edgeCase { {name: "argv/no parameters", args: []string{"search"}, exit: 2, code: "usage_error", msg: "No search parameters given"}, {name: "argv/q and bare words", args: searchWith("tea"), exit: 2, code: "usage_error", msg: "Got both q=... and bare words"}, {name: "argv/api_key parameter", args: searchWith("api_key=" + strayKey), exit: 2, code: "usage_error", msg: "not as the api_key parameter"}, + {name: "argv/word starting with a dash", args: []string{"search", "engine=google", "coffee", "-starbucks"}, exit: 2, code: "usage_error", + msg: `unknown shorthand flag: 's' in -starbucks; to search for a word starting with "-", put it inside q="..." or after --`}, {name: "argv/dash word after --", args: []string{"search", "engine=google", "--", "coffee", "-starbucks"}, api: oneResult, query: map[string]string{"q": "coffee -starbucks"}}, {name: "argv/reserved URL characters", args: []string{"search", "engine=google", "q=a&b=c d+e%"}, api: oneResult, query: map[string]string{"q": "a&b=c d+e%"}}, + {name: "argv/repeated key keeps the last", args: searchWith("q=tea"), api: oneResult, query: map[string]string{"q": "tea"}, + warn: `q given more than once; using the last value, "tea"`}, {name: "argv/repeated identical key is quiet", args: searchWith("engine=google"), api: oneResult, quiet: true}, + {name: "argv/locations repeated key", args: []string{"locations", "q=Austin", "q=Dallas"}, api: respond(`[]`), + query: map[string]string{"q": "Dallas"}, warn: "q given more than once"}, {name: "argv/empty value is sent", args: []string{"search", "engine=google", "q="}, api: oneResult, query: map[string]string{"q": ""}}, {name: "argv/unknown command", args: []string{"nope"}, exit: 2, code: "usage_error", msg: `unknown command "nope"`}, {name: "argv/stray argument to account", args: []string{"account", strayKey}, exit: 2, code: "usage_error", msg: "account takes no arguments"}, {name: "argv/stray argument to login", args: []string{"login", strayKey}, exit: 2, code: "usage_error", msg: "login takes no arguments; pipe the key instead"}, {name: "argv/archive ID with a path", args: []string{"archive", "../bad"}, exit: 2, code: "usage_error", msg: "Invalid archive ID"}, + {name: "argv/archive extra word", args: []string{"archive", "abc123", "md"}, exit: 2, code: "usage_error", msg: "Unexpected argument md"}, {name: "argv/archive with --jq reads the stored search", args: []string{"archive", "abc123", "--jq", ".place_results | {title, rating}"}, api: respond(`{"search_metadata":{"id":"abc123"},"place_results":{"title":"Tartine","rating":4.6,"reviews":7351}}`), stdout: `{"rating":4.6,"title":"Tartine"}` + "\n", query: map[string]string{"api_key": "envkey"}}, + {name: "argv/version", args: []string{"version"}, stdout: "serpapi version " + version.Version + "\n", quiet: true}, + {name: "argv/version extra word", args: []string{"version", "1"}, exit: 2, code: "usage_error", msg: "version takes no arguments"}, + {name: "flags/jq on version", args: []string{"version", "--jq", ".version"}, exit: 2, code: "usage_error", msg: "version prints text, not JSON"}, {name: "argv/locations without query", args: []string{"locations"}, exit: 2, code: "usage_error", msg: "Pass a location query"}, {name: "argv/locations blank query", args: []string{"locations", "q= "}, exit: 2, code: "usage_error", msg: "Pass a location query"}, + {name: "flags/unknown flag", args: searchWith("--bogus"), exit: 2, code: "usage_error", + msg: "unknown flag: --bogus; API parameters are key=value pairs, e.g. bogus=VALUE"}, + {name: "flags/API parameter as a flag", args: searchWith("--num", "10"), exit: 2, code: "usage_error", msg: "e.g. num=VALUE"}, + {name: "flags/one-letter flag guess", args: []string{"search", "-e", "youtube", "--search_query", "Rick"}, exit: 2, code: "usage_error", + msg: "unknown shorthand flag: 'e' in -e; API parameters are key=value pairs, e.g. engine=google q=coffee", exact: true}, {name: "flags/unknown flag outside search", args: []string{"account", "--bogus"}, exit: 2, code: "usage_error", msg: "unknown flag: --bogus", exact: true}, {name: "flags/timeout in seconds", args: searchWith("--timeout", "30"), api: oneResult}, {name: "flags/timeout as a duration", args: searchWith("--timeout", "2m"), api: oneResult}, @@ -107,6 +123,14 @@ func edgeCases() []edgeCase { {name: "flags/fields on locations", args: []string{"locations", "q=Austin", "--fields", "name"}, exit: 2, code: "usage_error", msg: "--fields works only with search"}, {name: "flags/jq parse error", args: []string{"search", "--jq", ".[", "engine=google", "q=coffee"}, exit: 2, code: "usage_error", msg: "jq parse error"}, {name: "flags/jq input builtin", args: searchWith("--jq", "input"), exit: 2, code: "usage_error", msg: "is not allowed"}, + {name: "flags/jq iterates a missing array", args: searchWith("--jq", ".missing[].link"), api: oneResult, exit: 2, code: "usage_error", + msg: "use []? to skip a missing array, e.g. .organic_results[]?; the response has: organic_results"}, + {name: "flags/jq on a missing object lists the keys", args: searchWith("--jq", ".place_results | keys"), + api: respond(`{"search_metadata":{"id":"1"},"local_results":[{"title":"Blue Bottle"}]}`), exit: 2, code: "usage_error", + msg: "jq runtime error: keys cannot be applied to: null; the response has: local_results, search_metadata", exact: true}, + {name: "flags/jq type mismatch suggests ?", args: searchWith("--jq", "[.news_results[] | {title, source: (.source.name // .source)}]"), + api: respond(`{"news_results":[{"title":"a","source":"Computer Weekly"}]}`), exit: 2, code: "usage_error", + msg: "; add ? to skip values of another type, e.g. .source.name? // .source"}, {name: "flags/the suggested ? fix works", args: searchWith("--jq", "[.news_results[] | {title, source: (.source.name? // .source)}]"), api: respond(`{"news_results":[{"title":"a","source":"Computer Weekly"},{"title":"b","source":{"name":"Wired"}}]}`), stdout: `[{"source":"Computer Weekly","title":"a"},{"source":"Wired","title":"b"}]` + "\n"}, @@ -114,11 +138,24 @@ func edgeCases() []edgeCase { api: respond(`{"organic_results":[{"title":"a"},{"title":"b"}],"related_searches":[{"query":"x"},{"query":"y"}]}`), stdout: `{"organic_results":[{"title":"a"}],"related_searches":[{"query":"x"}]}` + "\n"}, {name: "flags/jq cannot read the environment", args: searchWith("--jq", "[$ENV, env] | map(length)"), api: oneResult, stdout: "[0,0]\n"}, + {name: "flags/output md", args: searchWith("output=md"), api: respond("# Coffee\n\n1. [Blue Bottle](https://bluebottlecoffee.com)"), + stdout: "# Coffee\n\n1. [Blue Bottle](https://bluebottlecoffee.com)\n", query: map[string]string{"output": "md"}}, + {name: "flags/output in capitals", args: searchWith("output=MD"), api: respond("# Coffee\n"), stdout: "# Coffee\n", + query: map[string]string{"output": "MD"}}, + {name: "flags/unknown output", args: searchWith("output=xml"), exit: 2, code: "usage_error", msg: "Unknown output=xml"}, + {name: "flags/rank output without domain", args: searchWith("output=rank:"), exit: 2, code: "usage_error", msg: "Unknown output=rank:"}, + {name: "flags/jq with markdown", args: searchWith("output=md", "--jq", ".x"), exit: 2, code: "usage_error", msg: "--jq needs JSON, but output=md is not JSON"}, + {name: "flags/all-pages with markdown", args: searchWith("output=md", "--all-pages"), exit: 2, code: "usage_error", msg: "--all-pages needs JSON"}, + {name: "flags/archive unknown output", args: []string{"archive", "abc123", "output=xml"}, exit: 2, code: "usage_error", msg: "Unknown output=xml"}, + {name: "flags/archive jq with markdown", args: []string{"archive", "abc123", "output=md", "--jq", ".x"}, exit: 2, code: "usage_error", msg: "--jq needs JSON"}, {name: "auth/no key for account", args: []string{"account"}, env: noKey, exit: 2, code: "usage_error", msg: "No API key found"}, {name: "auth/no key for archive", args: []string{"archive", "abc123"}, env: noKey, exit: 2, code: "usage_error", msg: "No API key found"}, {name: "auth/no key for search", args: search, env: noKey, api: oneResult, noQuery: []string{"api_key"}}, {name: "auth/no key for all pages", args: searchWith("--all-pages"), env: noKey, api: oneResult, noQuery: []string{"api_key"}}, + {name: "auth/no key for markdown", args: searchWith("output=md"), env: noKey, api: respond("# Coffee\n"), noQuery: []string{"api_key"}}, + {name: "auth/no key and a metered query", args: search, env: noKey, api: respondStatus(401, invalidKey), exit: 1, code: "api_error", status: 401, + msg: "manage-api-key (no API key found; pass --api-key, set SERPAPI_KEY or run 'serpapi login')"}, {name: "auth/invalid key keeps the API's message", args: search, env: map[string]string{"SERPAPI_KEY": "badkey"}, api: respondStatus(401, invalidKey), exit: 1, code: "api_error", status: 401, msg: "Invalid API key. Your API key should be here: https://serpapi.com/manage-api-key", exact: true}, {name: "auth/padded key is trimmed", args: []string{"account"}, env: map[string]string{"SERPAPI_KEY": " padded\n"}, @@ -161,8 +198,25 @@ func edgeCases() []edgeCase { {name: "paging/jq runs on the merged pages", args: searchWith("--all-pages", "--jq", ".organic_results | length"), api: pages, reqs: 3, stdout: "3\n"}, {name: "output/small result is quiet", args: search, api: oneResult, quiet: true}, + {name: "output/large result names its keys and the archive", args: search, api: respond(bigResult(`"search_metadata":{"id":"abc123"},`, 1500)), + warn: "keys by size: organic_results 52 KB, search_metadata 24 B. To print only what you need, read the stored search for free: serpapi archive abc123 --jq '.KEY'"}, + {name: "output/large result without an id", args: search, api: respond(bigResult("", 1500)), + warn: "repeat the command with --jq '.KEY': the same search within an hour comes from the cache"}, {name: "output/large result with --jq is quiet", args: searchWith("--jq", ".organic_results | length"), api: respond(bigResult("", 1500)), stdout: "1500\n", quiet: true}, + {name: "output/large result with --fields still hints", args: searchWith("--fields", "organic_results"), + api: respond(bigResult("", 1500)), warn: "Large result: 52 KB"}, + {name: "output/large merged pages", args: searchWith("--all-pages"), reqs: 2, api: func(w http.ResponseWriter, r *http.Request) { + next := "" + if pageNumber(r) == 1 { + next = `"serpapi_pagination":{"next":"https://serpapi.com/search.json?engine=google&q=coffee&start=10"},` + } + fmt.Fprint(w, bigResult(next+`"inline_images":[{"position":1}],`, 700)) + }, warn: "Large result: 48 KB; top-level keys by size: organic_results 48 KB, inline_images 70 B. To print only what you need, repeat the command"}, + {name: "output/large archive", args: []string{"archive", "abc123"}, api: respond(bigResult(`"search_metadata":{"id":"abc123"},`, 1500)), + warn: "add --jq '.KEY'; reading the archive is free"}, + {name: "output/jq printing only null lists the keys", args: searchWith("--jq", ".organic"), api: oneResult, stdout: "\n", + warn: "--jq printed only null; check the path against the response's top-level keys: organic_results"}, {name: "output/jq printing some values is quiet", args: searchWith("--jq", ".organic_results[0].position, .organic"), api: oneResult, stdout: "1\n\n", quiet: true}, {name: "output/jq printing nothing is quiet", args: searchWith("--jq", ".organic_results[] | select(.position > 5)"), api: oneResult, diff --git a/pkg/cmd/hint.go b/pkg/cmd/hint.go new file mode 100644 index 0000000..3ca433d --- /dev/null +++ b/pkg/cmd/hint.go @@ -0,0 +1,143 @@ +package cmd + +import ( + "encoding/json" + "fmt" + "slices" + "strings" + + "github.com/spf13/cobra" + + "github.com/serpapi/serpapi-cli/pkg/output" + "github.com/serpapi/serpapi-cli/pkg/params" +) + +// largeResult is the size above which an unfiltered result sent to a pipe +// gets a hint on stderr. Agent tools keep only about 20 KB of a command's +// output inline: Copilot CLI saves anything longer to a file to page through. +const largeResult = 16 << 10 + +type keySize struct { + key string + size int +} + +// valueSize records the encoded length of a JSON value. encoding/json hands +// UnmarshalJSON a slice of the input, so measuring copies nothing. +type valueSize int + +func (s *valueSize) UnmarshalJSON(b []byte) error { + *s = valueSize(len(b)) + return nil +} + +// topLevelSizes returns the top-level keys of a JSON object with the +// encoded size of each value, largest first, and search_metadata.id. +func topLevelSizes(raw []byte) ([]keySize, string) { + var top map[string]valueSize + if json.Unmarshal(raw, &top) != nil { + return nil, "" + } + keys := make([]keySize, 0, len(top)) + for k, v := range top { + keys = append(keys, keySize{k, int(v)}) + } + sortKeySizes(keys) + var meta struct { + SearchMetadata struct { + ID string `json:"id"` + } `json:"search_metadata"` + } + _ = json.Unmarshal(raw, &meta) + return keys, meta.SearchMetadata.ID +} + +func sortKeySizes(keys []keySize) { + slices.SortFunc(keys, func(a, b keySize) int { + if a.size != b.size { + return b.size - a.size + } + return strings.Compare(a.key, b.key) + }) +} + +// hintLargeResult warns that a result about to be printed to a pipe is +// large, listing its top-level keys by size and how to print only part of +// it. It is printed before the result because agents read stdout and stderr +// together and may see only the start. A terminal or a --jq run gets none. +func hintLargeResult(cmd *cobra.Command, size int, keys []keySize, next string) { + if !wantsLargeHint(cmd, size) || len(keys) == 0 { + return + } + const shown = 12 + parts := make([]string, 0, shown) + for _, k := range keys[:min(len(keys), shown)] { + parts = append(parts, k.key+" "+sizeText(k.size)) + } + list := strings.Join(parts, ", ") + if len(keys) > shown { + list += fmt.Sprintf(" and %d more", len(keys)-shown) + } + warn(cmd, "Large result: %s; top-level keys by size: %s. To print only what you need, %s", + sizeText(size), list, next) +} + +// wantsLargeHint reports whether a result of size bytes gets the large +// result hint, before any work is spent on its keys. A stream merged with +// 2>&1 gets none: agents often pipe it into a JSON parser, which the extra +// line would break. +func wantsLargeHint(cmd *cobra.Command, size int) bool { + w := cmd.OutOrStdout() + return jqFlag == "" && size > largeResult && !output.IsTerminal(w) && !output.SameStream(w, cmd.ErrOrStderr()) +} + +// hintLargeJSON is hintLargeResult for JSON as it is about to be printed; +// next turns its search_metadata.id into the suggested next step. +func hintLargeJSON(cmd *cobra.Command, raw []byte, next func(id string) string) { + if !wantsLargeHint(cmd, len(raw)) { + return + } + keys, id := topLevelSizes(raw) + hintLargeResult(cmd, len(raw), keys, next(id)) +} + +// warnRepeated names each parameter given twice with different values: the +// server, like the CLI, keeps only the last one. +func warnRepeated(cmd *cobra.Command, parsed []params.Param) { + for _, p := range params.Repeated(parsed) { + warn(cmd, "%s given more than once; using the last value, %q", p.Key, p.Value) + } +} + +// archiveHint is the next step for a large stored search. +func archiveHint(string) string { return "add --jq '.KEY'; reading the archive is free" } + +// searchHint is the next step for a large search result: re-reading the +// stored search is free, and so is the same search within the cache hour. +func searchHint(id string) string { + if id != "" { + return fmt.Sprintf("read the stored search for free: serpapi archive %s --jq '.KEY'", id) + } + return "repeat the command with --jq '.KEY': the same search within an hour comes from the cache" +} + +func sizeText(n int) string { + if n < 1024 { + return fmt.Sprintf("%d B", n) + } + return fmt.Sprintf("%d KB", (n+512)/1024) +} + +// objectKeys lists the top-level keys of v, sorted, when v is an object. +func objectKeys(v any) string { + obj, ok := v.(map[string]any) + if !ok || len(obj) == 0 { + return "" + } + keys := make([]string, 0, len(obj)) + for k := range obj { + keys = append(keys, k) + } + slices.Sort(keys) + return strings.Join(keys, ", ") +} diff --git a/pkg/cmd/hint_test.go b/pkg/cmd/hint_test.go new file mode 100644 index 0000000..daa1264 --- /dev/null +++ b/pkg/cmd/hint_test.go @@ -0,0 +1,104 @@ +package cmd + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "os" + "path/filepath" + "strings" + "testing" + + "github.com/spf13/cobra" + + "github.com/serpapi/serpapi-cli/pkg/output" +) + +func TestLargeResultHintListsTwelveKeysLargestFirst(t *testing.T) { + members := make([]string, 0, 14) + for i := 1; i <= 14; i++ { + members = append(members, fmt.Sprintf(`"k%02d":"%s"`, i, strings.Repeat("x", i*300))) + } + raw := []byte(`{"search_metadata":{"id":"abc123"},` + strings.Join(members, ",") + "}") + + resetFlags(rootCmd) + var stdout, stderr bytes.Buffer + cmd := &cobra.Command{} + cmd.SetOut(&stdout) + cmd.SetErr(&stderr) + hintLargeJSON(cmd, raw, searchHint) + + var got struct { + Warning struct { + Message string `json:"message"` + } `json:"warning"` + } + if err := json.Unmarshal(stderr.Bytes(), &got); err != nil { + t.Fatalf("stderr is not one JSON warning: %v: %q", err, stderr.String()) + } + want := "Large result: 31 KB; top-level keys by size: k14 4 KB, k13 4 KB, k12 4 KB, k11 3 KB, k10 3 KB, " + + "k09 3 KB, k08 2 KB, k07 2 KB, k06 2 KB, k05 1 KB, k04 1 KB, k03 902 B and 3 more. " + + "To print only what you need, read the stored search for free: serpapi archive abc123 --jq '.KEY'" + if got.Warning.Message != want { + t.Errorf("warning =\n%s\nwant\n%s", got.Warning.Message, want) + } +} + +func TestLargeResultHintSkipsATerminal(t *testing.T) { + isTerminal := output.IsTerminal + output.IsTerminal = func(io.Writer) bool { return true } + t.Cleanup(func() { output.IsTerminal = isTerminal }) + fakeAPI(t, respond(bigResult(`"search_metadata":{"id":"abc123"},`, 1500))) + t.Setenv("SERPAPI_KEY", "envkey") + + res := execute(t, "", search...) + + if warnings := wantOK(t, res); len(warnings) > 0 { + t.Errorf("warnings = %q, want none on a terminal", warnings) + } +} + +// Agents often run `serpapi search ... 2>&1 | python3 -c 'json.load(...)'`. +// In an agent study, the warning in that merged stream broke 5 of 6 such +// parses, so a merged stream must stay one JSON document. +func TestLargeResultHintSkipsAMergedStream(t *testing.T) { + fakeAPI(t, respond(bigResult(`"search_metadata":{"id":"abc123"},`, 1500))) + t.Setenv("SERPAPI_KEY", "envkey") + runTo := func(stdout, stderr *os.File) { + t.Helper() + resetFlags(rootCmd) + rootCmd.SetOut(stdout) + rootCmd.SetErr(stderr) + rootCmd.SetArgs(append([]string{}, search...)) + if code := run(context.Background()); code != 0 { + t.Fatalf("exit code %d", code) + } + } + create := func(name string) *os.File { + f, err := os.Create(filepath.Join(t.TempDir(), name)) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { f.Close() }) + return f + } + + merged := create("merged") + runTo(merged, merged) + out, err := os.ReadFile(merged.Name()) + if err != nil { + t.Fatal(err) + } + var v map[string]any + if err := json.Unmarshal(out, &v); err != nil { + t.Errorf("the merged stream is not one JSON document: %v: %.200s", err, out) + } + + stdout, stderr := create("stdout"), create("stderr") + runTo(stdout, stderr) + if warning, _ := os.ReadFile(stderr.Name()); !strings.Contains(string(warning), "Large result: 52 KB") { + t.Errorf("separate stderr = %q, want the large-result warning", warning) + } +} diff --git a/pkg/cmd/locations.go b/pkg/cmd/locations.go index b1fceae..3a60e73 100644 --- a/pkg/cmd/locations.go +++ b/pkg/cmd/locations.go @@ -30,6 +30,7 @@ func runLocations(cmd *cobra.Command, args []string) error { if err != nil { return err } + warnRepeated(cmd, parsed) p := params.ParamsToMap(parsed) if blank(p["q"]) && blank(p["location"]) && blank(p["limit"]) { return &clierrors.UsageError{Message: "Pass a location query, e.g. serpapi locations q=Austin limit=5; without one the server returns every location (66 MB)"} @@ -41,7 +42,7 @@ func runLocations(cmd *cobra.Command, args []string) error { if err != nil { return err } - return handleOutput(cmd, result) + return handleOutput(cmd, result, nil) } func blank(s string) bool { return strings.TrimSpace(s) == "" } diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index 4400d94..648cac3 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -8,8 +8,10 @@ import ( "errors" "fmt" "io" + "maps" "os" "os/signal" + "slices" "strconv" "strings" "syscall" @@ -64,6 +66,7 @@ characters from results escaped, so a page cannot drive it; piped output keeps them. Keep results small: --fields "organic_results[].{title, link}" server-side filter (search only) --jq ".organic_results[].link" jq filter; strings print unquoted + output=md Markdown instead of JSON Errors and warnings go to stderr as one JSON object per line: {"error":{"code":"api_error","message":"Invalid API key. ...","status":401}} @@ -88,6 +91,7 @@ hour old, so add it only when that is too old. account, locations and archive are free.`, Example: ` serpapi search engine=google q=coffee --fields "organic_results[0:3]" serpapi search engine=google coffee shops --jq ".organic_results[].link" + serpapi search engine=google q=coffee output=md serpapi locations q=Austin limit=3 serpapi account --jq .total_searches_left`, Version: version.Version, @@ -165,11 +169,36 @@ func init() { rootCmd.PersistentPreRunE = validateGlobalFlags // Wrap cobra flag-parsing errors as UsageError so they get exit code 2. - rootCmd.SetFlagErrorFunc(func(_ *cobra.Command, err error) error { - return &clierrors.UsageError{Message: err.Error()} + rootCmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error { + return &clierrors.UsageError{Message: flagErrorHint(cmd, err.Error())} }) } +// flagErrorHint explains the two flag errors agents hit most: a search word +// starting with "-", such as -site:example.com, and an API parameter guessed +// as a flag, such as --num 10. +func flagErrorHint(cmd *cobra.Command, msg string) string { + if cmd != searchCmd && cmd != locationsCmd { + return msg + } + params := "API parameters are key=value pairs, e.g. engine=google q=coffee" + if cmd == locationsCmd { + params = "API parameters are key=value pairs, e.g. q=Austin limit=3" + } + if strings.HasPrefix(msg, "unknown shorthand flag") { + // pflag reports "unknown shorthand flag: 'e' in -e". A one-letter flag + // is a guessed parameter; a longer one may be a word like -site:x.com. + if _, arg, _ := strings.Cut(msg, " in "); len(arg) == 2 { + return msg + "; " + params + } + return msg + `; to search for a word starting with "-", put it inside q="..." or after --; ` + params + } + if name, ok := strings.CutPrefix(msg, "unknown flag: --"); ok { + return msg + "; API parameters are key=value pairs, e.g. " + name + "=VALUE" + } + return msg +} + // validateGlobalFlags rejects bad flags before any request, so a typo never // costs a search. func validateGlobalFlags(cmd *cobra.Command, _ []string) error { @@ -269,9 +298,16 @@ func noArgs(hint string) cobra.PositionalArgs { } // handleOutput applies --jq, if set, and prints the JSON result to stdout. -func handleOutput(cmd *cobra.Command, raw []byte) error { +// next, if not nil, gives the large-result hint its next step. +func handleOutput(cmd *cobra.Command, raw []byte, next func(id string) string) error { if jqFlag == "" { - return outputError(output.PrintJSON(cmd.OutOrStdout(), raw)) + w := cmd.OutOrStdout() + out := output.FormatJSON(w, raw) + if next != nil { + hintLargeJSON(cmd, out, next) + } + _, err := w.Write(out) + return outputError(err) } var v any dec := json.NewDecoder(bytes.NewReader(raw)) @@ -282,7 +318,8 @@ func handleOutput(cmd *cobra.Command, raw []byte) error { return printJQ(cmd, v) } -// printJQ runs --jq on v and prints each result as it is produced. +// printJQ runs --jq on v and prints each result as it is produced. A filter +// that prints only null gets a warning listing the keys v has. func printJQ(cmd *cobra.Command, v any) error { code, err := jq.Compile(jqFlag) if err != nil { @@ -292,11 +329,16 @@ func printJQ(cmd *cobra.Command, v any) error { tty := output.IsTerminal(w) bw := bufio.NewWriter(w) var writeErr error + printed, nulls := 0, 0 ctx := cmd.Context() if ctx == nil { ctx = context.Background() } err = jq.Run(ctx, code, v, func(val any) error { + printed++ + if val == nil { + nulls++ + } writeErr = output.PrintJQValue(val, bw, tty) return writeErr }) @@ -310,11 +352,33 @@ func printJQ(cmd *cobra.Command, v any) error { return &clierrors.InterruptedError{Message: "Stopped while running --jq; what it printed before the signal is on stdout"} } if err != nil { - return &clierrors.UsageError{Message: err.Error()} + return &clierrors.UsageError{Message: jqErrorHint(err.Error(), v)} + } + if keys := objectKeys(v); printed > 0 && nulls == printed && keys != "" { + warn(cmd, "--jq printed only null; check the path against the response's top-level keys: %s", keys) } return nil } +// jqErrorHint adds the fix for the jq errors agents hit most: a value of +// another type than the filter expects, and a path missing from this +// response, for which it lists the keys the response does have. +func jqErrorHint(msg string, v any) string { + if strings.Contains(msg, "expected an object but got") { + return msg + "; add ? to skip values of another type, e.g. .source.name? // .source" + } + if !strings.Contains(msg, ": null") { + return msg + } + if strings.Contains(msg, "cannot iterate over: null") { + msg += "; use []? to skip a missing array, e.g. .organic_results[]?" + } + if obj, ok := v.(map[string]any); ok && len(obj) > 0 { + msg += "; the response has: " + strings.Join(slices.Sorted(maps.Keys(obj)), ", ") + } + return msg +} + func outputError(err error) error { if err == nil { return nil diff --git a/pkg/cmd/search.go b/pkg/cmd/search.go index 7d407b8..6514da6 100644 --- a/pkg/cmd/search.go +++ b/pkg/cmd/search.go @@ -6,8 +6,10 @@ import ( "encoding/json" "errors" "fmt" + "io" "net/http" "net/url" + "strings" "github.com/spf13/cobra" @@ -25,6 +27,12 @@ var ( const defaultMaxPages = 100 +// SerpApi rejects any other output value, but only after the search has run. +var ( + jsonOutputs = map[string]bool{"json": true, "json_with_images": true, "json_with_pixel_position": true} + rawOutputs = map[string]bool{"md": true, "html": true, "raw_html": true, "rawhtml": true, "plain": true, "csv": true} +) + // carriedParams are dropped from serpapi_pagination.next but still apply to // every page. var carriedParams = []string{"json_restrictor", "no_cache", "zero_trace", "output"} @@ -33,7 +41,8 @@ var searchCmd = &cobra.Command{ Use: "search PARAMS...", Short: "Run a search with any SerpApi engine", Long: `Run a search. Parameters are key=value pairs, starting with engine=. -Words without "=" are joined into q. Some engines name the query differently: +Words without "=" are joined into q. A parameter given twice keeps its last +value, with a warning. Some engines name the query differently: youtube search_query, amazon k, ebay _nkw, walmart and naver query, yahoo p, yandex text. Parameters of every engine: https://serpapi.com/search-engine-apis @@ -48,13 +57,18 @@ cache key, so a different --fields can miss the cache and count as a new search; --jq filters the response locally without changing the request. Responses are often 20-300 KB, and one key can be large too: Maps -place_results holds reviews and photos. To see the shape of an unfamiliar -response, print a sample with one item per array; nested arrays are cut too, -so it is not the answer: +place_results holds reviews and photos. Piped without --jq, a result over +16 KB comes with a warning on stderr that lists its top-level keys by size, +unless stderr goes into the same pipe (2>&1), where it would break a JSON +parser. To see the shape of an unfamiliar response, print a sample with one +item per array; nested arrays are cut too, so it is not the answer: --jq 'walk(if type == "array" then .[:1] else . end)' Then take what you need from the stored search, which is free: serpapi archive SEARCH_METADATA_ID --jq '.place_results | {title, rating, reviews}' +output=md returns Markdown; html, plain and csv are also available. These are +not JSON, so --jq and --all-pages do not apply. + --all-pages follows serpapi_pagination.next and merges the arrays of every page into one JSON object, up to --max-pages pages, one search each. It stops at a page that --fields matches nothing on; use --jq to filter such pages instead. @@ -67,6 +81,7 @@ with 'serpapi archive ID'.`, serpapi search engine=google coffee shops --fields "organic_results[].{title, link}" serpapi search engine=youtube search_query="lofi beats" --jq ".video_results[:3][].link" serpapi search engine=google_maps q=pizza ll="@40.7455096,-74.0083012,14z" --fields "local_results[0:5]" + serpapi search engine=google q=coffee output=md serpapi search engine=google q=coffee --all-pages --max-pages 3 --jq ".organic_results | length"`, Args: cobra.ArbitraryArgs, RunE: runSearch, @@ -86,12 +101,25 @@ func runSearch(cmd *cobra.Command, args []string) error { if err != nil { return err } + warnRepeated(cmd, parsed) p := params.ParamsToMap(parsed) maxPagesSet := cmd.Flags().Changed("max-pages") if maxPagesSet && maxPagesFlag < 1 { return &clierrors.UsageError{Message: "--max-pages must be at least 1"} } + raw := false + if o, ok := p["output"]; ok { + if raw, err = isRawOutput(o); err != nil { + return err + } + } + if raw && jqFlag != "" { + return &clierrors.UsageError{Message: "--jq needs JSON, but output=" + p["output"] + " is not JSON"} + } + if raw && allPagesFlag { + return &clierrors.UsageError{Message: "--all-pages needs JSON, but output=" + p["output"] + " is not JSON"} + } if a := p["async"]; allPagesFlag && a != "" && a != "false" && a != "0" { return &clierrors.UsageError{Message: "--all-pages cannot follow an async search; drop async=" + a} } @@ -112,11 +140,22 @@ func runSearch(cmd *cobra.Command, args []string) error { if keyErr != config.ErrNoAPIKey && errors.As(keyErr, &cfgErr) { warn(cmd, "%s; searching without a key", cfgErr.Message) } - return sendSearch(cmd, newClient(apiKey), p, restrictor != "") + err = sendSearch(cmd, newClient(apiKey), p, raw, restrictor != "") + if apiKey == "" { + err = explainMissingKey(err) + } + return err } -func sendSearch(cmd *cobra.Command, client *api.Client, p map[string]string, restricted bool) error { +func sendSearch(cmd *cobra.Command, client *api.Client, p map[string]string, raw, restricted bool) error { ctx := cmd.Context() + if raw { + body, err := withSpinner("Searching...", func() ([]byte, error) { return client.SearchRaw(ctx, p) }) + if err != nil { + return err + } + return outputError(output.PrintRaw(cmd.OutOrStdout(), body)) + } if allPagesFlag { return searchAllPages(cmd, client, p, restricted) } @@ -128,7 +167,36 @@ func sendSearch(cmd *cobra.Command, client *api.Client, p map[string]string, res if restricted && isEmptyObject(result) { warnNothingMatched(cmd) } - return handleOutput(cmd, result) + return handleOutput(cmd, result, searchHint) +} + +// explainMissingKey adds to a 401 for a search sent without a key how to give +// one, keeping the API's message, type and status. +func explainMissingKey(err error) error { + var apiErr *clierrors.APIError + if !errors.As(err, &apiErr) || apiErr.Status != http.StatusUnauthorized { + return err + } + return &clierrors.APIError{ + Message: apiErr.Message + " (no API key found; pass --api-key, set SERPAPI_KEY or run 'serpapi login')", + Status: apiErr.Status, + Cause: apiErr.Cause, + } +} + +func isRawOutput(o string) (bool, error) { + // The API lowercases output, so MD works like md. + switch lower := strings.ToLower(o); { + case jsonOutputs[lower]: + return false, nil + case rawOutputs[lower]: + return true, nil + case strings.HasPrefix(lower, "rank:") && len(lower) > len("rank:"), + strings.HasPrefix(lower, "ad_rank:") && len(lower) > len("ad_rank:"): + return true, nil + } + return false, &clierrors.UsageError{Message: "Unknown output=" + o + + "; use json, json_with_images, json_with_pixel_position, md, html, raw_html, plain, csv, rank:DOMAIN or ad_rank:DOMAIN"} } func isEmptyObject(raw []byte) bool { @@ -259,6 +327,14 @@ func printMerged(cmd *cobra.Command, merged map[string]any) error { } return outputError(output.PrintJSON(w, buf.Bytes())) } + // Measuring takes an extra encoding pass, but holds no encoded copy. + var keys []keySize + size, err := output.WriteObject(io.Discard, merged, func(k string, n int) { keys = append(keys, keySize{k, n}) }) + if err != nil { + return &clierrors.APIError{Message: "Failed to encode result: " + err.Error()} + } + sortKeySizes(keys) + hintLargeResult(cmd, int(size), keys, searchHint("")) bw := bufio.NewWriterSize(w, 64<<10) if _, err := output.WriteObject(bw, merged, nil); err != nil { return outputError(err) diff --git a/pkg/cmd/version.go b/pkg/cmd/version.go new file mode 100644 index 0000000..78303be --- /dev/null +++ b/pkg/cmd/version.go @@ -0,0 +1,27 @@ +package cmd + +import ( + "fmt" + + "github.com/spf13/cobra" + + clierrors "github.com/serpapi/serpapi-cli/pkg/errors" + "github.com/serpapi/serpapi-cli/pkg/version" +) + +var versionCmd = &cobra.Command{ + Use: "version", + Short: "Print the CLI version, as --version does", + Args: noArgs("run serpapi version"), + RunE: func(cmd *cobra.Command, _ []string) error { + if jqFlag != "" { + return &clierrors.UsageError{Message: "version prints text, not JSON; drop --jq"} + } + _, err := fmt.Fprintf(cmd.OutOrStdout(), "serpapi version %s\n", version.Version) + return outputError(err) + }, +} + +func init() { + rootCmd.AddCommand(versionCmd) +} From 602d17d7062bad5401899590ba27e4570e968172 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Mon, 12 Oct 2026 00:46:09 +0200 Subject: [PATCH 21/25] [Docs] Document the CLI for people, scripts and AI agents The README covers search parameters and per-engine query keys, --fields and its cache key, output=md and the other formats, --all-pages limits and cost, async with archive polling, locations, login, version, key precedence, the config file, and what counts against the plan. "For AI Agents" states the stdout and stderr contract, the exit codes and the large result warning. Co-Authored-By: Claude Opus 5.5 --- README.md | 155 +++++++++++++++++++++++++----------------------------- 1 file changed, 72 insertions(+), 83 deletions(-) diff --git a/README.md b/README.md index 5826397..d24bd7f 100644 --- a/README.md +++ b/README.md @@ -1,6 +1,6 @@ # serpapi-cli -> HTTP client for structured web search data via SerpApi +> Search engine results as JSON from [SerpApi](https://serpapi.com), for people, scripts and AI agents ## Installation @@ -24,102 +24,90 @@ go install github.com/serpapi/serpapi-cli/cmd/serpapi@latest ## Quick Start ```bash -# Authenticate -serpapi login - -# Perform a search -serpapi search engine=google q=coffee +serpapi login # check and save your API key +serpapi search engine=google q=coffee # JSON results +serpapi search engine=google coffee --fields "organic_results[].{title, link}" +serpapi search engine=google q=coffee output=md # Markdown ``` +`serpapi --help` and each command's `--help` document parameters, errors and costs. + ## Commands ### search -Perform a search with any supported SerpApi engine. Parameters are passed as bare `key=value` pairs. +Parameters are `key=value` pairs sent to the API as-is; words without `=` are joined into `q`, and a parameter given twice keeps its last value, with a warning. Each engine's parameters are listed at . Some engines name the query differently: `search_query` (YouTube), `k` (Amazon), `_nkw` (eBay), `query` (Walmart, Naver), `p` (Yahoo), `text` (Yandex). ```bash -# Basic search -serpapi search engine=google q=coffee - -# Multiple parameters -serpapi search engine=google q="coffee shops" location="Austin,TX" - -# Use a different engine -serpapi search engine=google_maps q="pizza" ll="@40.7455096,-74.0083012,14z" +serpapi search engine=google q="coffee shops" location="Austin, Texas, United States" +serpapi search engine=youtube search_query="lofi beats" --jq '.video_results[:3][].link' +serpapi search engine=google_maps q=pizza ll="@40.7455096,-74.0083012,14z" --fields "local_results[0:5]" +``` -# With server-side field filtering (reduces response size at API level) -serpapi search --fields "organic_results[].{title,link}" engine=google q=coffee +`--fields` sets SerpApi's [`json_restrictor`](https://serpapi.com/json-restrictor), so the server returns only the listed paths: -# With client-side jq filtering (like gh --jq) -serpapi search --jq ".organic_results[0:3] | [.[] | {title, link}]" engine=google q=coffee +| `--fields` | Returns | +|---|---| +| `organic_results[0:3]` | the first three results | +| `organic_results[].{title, link}` | two fields of every result | +| `search_metadata.id,local_results[0]` | several paths, comma-separated | -# Both: server-side reduces payload, then client-side refines -serpapi search --fields "organic_results" --jq ".organic_results[0:3] | [.[] | {title, link}]" engine=google q=coffee -``` +Unknown paths are dropped silently; if nothing matches, you get `{}` and a warning on stderr. The server also drops `null`, `false` and empty values. `--fields` is part of the cache key, so a different `--fields` can miss the cache and count as a new search; `--jq` filters the response locally without changing the request. -#### Pagination Flags +`output=md` returns Markdown; `html`, `plain` and `csv` are also available. They are not JSON, so `--jq` and `--all-pages` don't apply. -- `--all-pages` — Fetch all result pages and merge array fields across pages -- `--max-pages ` — Maximum number of pages to fetch when using `--all-pages` +`--all-pages` follows `serpapi_pagination.next` and merges the arrays of every page into one object, up to `--max-pages` pages (default 100). Each page is one search. It stops at a page that `--fields` matches nothing on, so a typo costs one search; to page past such pages, filter with `--jq` instead. If a later page fails, or Ctrl-C stops the run, the pages fetched so far are printed and the error says which page it stopped at. ```bash -# Fetch all pages and merge array results -serpapi search engine=google q=coffee --all-pages - -# Limit to first 3 pages -serpapi search engine=google q=coffee --all-pages --max-pages 3 +serpapi search engine=google q=coffee --all-pages --max-pages 3 --jq '.organic_results | length' ``` -### account +`async=true` returns at once with `search_metadata.id`; collect the results with `serpapi archive`. -Retrieve account information and usage statistics. +### archive + +Fetches an earlier search by its `search_metadata.id`, for free. A search made with `async=true` has `search_metadata.status` `Queued` or `Processing` until it is `Success`, so poll. `output=` takes the same values as in search. ```bash -serpapi account +serpapi archive 6a2b3c4d5e6f7a8b9c0d1e2f +serpapi archive 6a2b3c4d5e6f7a8b9c0d1e2f output=md ``` ### locations -Lookup available locations for search queries (no API key required). +Finds values for the `location` search parameter: pass a result's `canonical_name`. Free, and needs no API key. `limit` is at most 10. A query is required, because without one the API returns every location (66 MB). ```bash -# Find locations matching "austin" -serpapi locations q=austin num=5 +serpapi locations q=Austin limit=3 --jq '.[0].canonical_name' ``` -### archive +### account -Retrieve a previously cached search by ID. +Shows the plan, usage and searches left, for free. The API key in the output is masked. ```bash -serpapi archive +serpapi account --jq .total_searches_left ``` ### login -Interactive authentication flow to save API key to config file. +Checks an API key with a free account request, then saves it. The key comes from `--api-key`, a hidden prompt on a terminal, or else the first line of stdin, which `login` waits for. ```bash serpapi login +printf %s "$KEY" | serpapi login ``` -## Global Flags +### version -- `--fields ` — Server-side field filtering (maps to SerpApi's `json_restrictor` parameter). Note: The `--fields` filter uses SerpApi's server-side field restrictor syntax—see [SerpApi docs](https://serpapi.com) for supported expressions. -- `--jq ` — Client-side [jq](https://jqlang.github.io/jq/) filter applied to the JSON response, same as `gh --jq` (see the [jq tutorial](https://jqlang.github.io/jq/tutorial/) for syntax). Runs locally after the API call; the jq engine is embedded so no external `jq` install is required. Use `--fields` to shrink the payload server-side, then `--jq` to reshape it client-side. Examples: - ```bash - # Pick the first 3 organic results, keeping only title and link - serpapi search --jq '.organic_results[0:3] | [.[] | {title, link}]' engine=google q=coffee +Prints the CLI version, as `--version` does. - # Print just the link of each organic result - serpapi search --jq '.organic_results[].link' engine=google q=coffee +## Global Flags - # Count organic results - serpapi search --jq '.organic_results | length' engine=google q=coffee - ``` - Quote the expression with single quotes in bash/zsh to avoid shell interpretation of `$`, `|`, and `"`. -- `--api-key ` — Override API key (takes priority over environment and config file) -- `--timeout ` — HTTP request timeout (default `60`, env: `SERPAPI_TIMEOUT`). Use `0` to wait indefinitely. Slow engines such as `google_ai_mode`, or searches with `no_cache=true`, can take longer than the default; raise this if you see a `network_error` mentioning `Client.Timeout exceeded`. Note that a timed-out search may still complete on SerpApi's side and be billed and retrievable with `serpapi archive `. +- `--jq `: [jq](https://jqlang.github.io/jq/manual/) filter applied to the JSON result, like `gh --jq`; strings print unquoted and `null` as an empty line, and results stream as they are produced. A filter that prints only `null` gets a warning listing the response's top-level keys. jq is built in, so nothing else needs installing. In bash and zsh, quote the expression with single quotes. +- `--fields `: search only; see [search](#search). +- `--api-key `: API key. Prefer `SERPAPI_KEY`, since flags show up in `ps` and shell history. +- `--timeout `: HTTP request timeout in seconds, such as `120`, or a duration, such as `2m` (default `60`; env `SERPAPI_TIMEOUT`). `0` waits indefinitely. Slow engines such as `google_ai_mode`, and searches with `no_cache=true`, can take longer than the default; raise it when a `network_error` says the request timed out. A timed-out search may still finish on SerpApi's side, count against your searches and be available in your search archive. ```bash # Give an AI Mode query up to two minutes serpapi search --timeout 120 engine=google_ai_mode q="..." @@ -130,45 +118,46 @@ serpapi login ## Configuration -### Authentication Priority Chain - -The CLI checks for API keys in this order: +The API key comes from the first of: -1. `--api-key` flag -2. `SERPAPI_KEY` environment variable -3. Config file: `~/.config/serpapi/config.toml` +1. `--api-key` +2. `SERPAPI_KEY` +3. `SERPAPI_API_KEY` +4. the key saved by `serpapi login` -If no API key is found, run `serpapi login` to authenticate interactively. +Without a key, `search` still runs, but the API answers only a few demo queries such as `q=coffee` and returns 401 for the rest. `account` and `archive` need a key. -> **Security note:** For security, prefer setting `SERPAPI_KEY` as an environment variable over -> passing `--api-key` on the command line (command-line arguments are visible in process listings). - -### Config File Format +`serpapi login` saves the key, readable only by you, to `serpapi/config.toml` in the OS config directory: `$XDG_CONFIG_HOME` or `~/.config` on Linux, `~/Library/Application Support` on macOS, `%AppData%` on Windows. ```toml api_key = "your_serpapi_key_here" ``` +A config file that can't be read or isn't valid TOML is reported by path and line, never by content: `search` warns and runs without a key, while `account` and `archive` stop. `serpapi login` rewrites it. + ## For AI Agents -This CLI is optimized for consumption by AI agents (Claude, Codex, etc.): - -- **Use `--fields` for server-side filtering** to reduce token usage: - - Example: `--fields "organic_results[0:3]"` returns only first 3 results - - Filtering happens at the API level, saving bandwidth and context window tokens - - Syntax follows SerpApi's `json_restrictor` parameter -- **Use `--jq` for client-side filtering** (same as `gh --jq`): - - Example: `--jq ".organic_results | length"` counts results locally - - Full jq expression support: pipes, array slicing, object construction, `select`, `map`, etc. - - Runs after API response is received -- **Combine both** for maximum efficiency: - - `--fields` reduces the API response size (less bandwidth) - - `--jq` refines the result further (less context window tokens) -- **Exit codes**: - - `0` = success - - `1` = API error (invalid key, rate limit, etc.) - - `2` = usage error (missing arguments, invalid flags) -- **Errors are always JSON** on stderr for structured parsing +`serpapi --help` is the full contract. In short: + +- **stdout carries only results**: JSON indented by two spaces, colored on a terminal. Piped, it is indented too, so `head`, `grep` and file viewers show whole lines; `head` cuts the JSON short, so take parts with `--jq`. A terminal gets control characters from results escaped, so a page can't drive it; piped output keeps them. Shrink them on the server with `--fields`, then locally with `--jq`, or ask for `output=md`. +- **Unfamiliar engine?** Responses are often 20–300 KB, and one key can be large too (Maps `place_results` holds reviews and photos). Piped without `--jq`, a result over 16 KB comes with a warning on stderr that lists its top-level keys by size, unless stderr goes into the same pipe (`2>&1`), where it would break a JSON parser. Print a sample with one item per array, `--jq 'walk(if type == "array" then .[:1] else . end)'`; nested arrays are cut too, so it shows the shape, not the answer. Then take what you need from the stored search for free: `serpapi archive SEARCH_METADATA_ID --jq '...'`. +- **Errors and warnings go to stderr**, one JSON object per line: + ``` + {"error":{"code":"api_error","message":"Invalid API key. ...","status":401}} + {"warning":{"message":"..."}} + ``` +- **The exit code says what to do next**: + + | Exit | `code` | Meaning | + |---|---|---| + | 0 | | Success; warnings may still be on stderr. | + | 2 | `usage_error` | The command is wrong; fix it before retrying. | + | 1 | `api_error` | `status` is the HTTP status: 200 the search ran but reported a problem, such as no results; 400 invalid parameters; 401 invalid API key; 403 account not allowed; 429 rate limited or out of searches; 5xx SerpApi failed. No `status`: the response was unreadable. | + | 1 | `network_error` | No answer, such as a timeout. | + | 130 | `interrupted` | Stopped by Ctrl-C or SIGTERM after printing what had arrived, such as the `--all-pages` pages fetched so far. | + +- **Cost**: each successful search counts against your monthly searches, including each `--all-pages` page; cached, errored and failed ones don't, though errored ones count toward the hourly limit ([pricing FAQ](https://serpapi.com/pricing)). An identical search, `--fields` included, repeated within an hour comes from the cache. `no_cache=true` forces a new, counted search; cached results are at most an hour old, so add it only when that is too old. `account`, `locations` and `archive` are free. +- **No prompts without a terminal.** Only `serpapi login` reads stdin, and it waits for the first line, so pipe the key in (`printf %s "$KEY" | serpapi login`) or, simpler, set `SERPAPI_KEY`. ## Links From 8755effdf0860ebf24aeb74b7c0a356948a07041 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Mon, 12 Oct 2026 00:46:09 +0200 Subject: [PATCH 22/25] [Tests] Check the binary's contract without an API key, on every PR Offline tests run the compiled binary for usage errors, help and version, in an isolated HOME with no key in the environment. Tests that reach the real API, which can use searches, run only when SERPAPI_KEY is set; on main, the locations and invalid-key tests reached it without one. CI's e2e job now runs on fork pull requests too, where only the offline tests run. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 7 +- tests/e2e_test.go | 296 +++++++++++++++++++++++++-------------- 2 files changed, 194 insertions(+), 109 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 3d00907..6f2d9c2 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -38,11 +38,8 @@ jobs: with: go-version: stable check-latest: true + # Fork PRs get no secrets, so only the offline contract tests run there. - name: Run E2E tests - if: github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == github.repository env: SERPAPI_KEY: ${{ secrets.SERPAPI_KEY }} - run: go test ./tests/... - - name: Skip E2E (fork PR) - if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name != github.repository - run: echo "Skipping E2E tests - SERPAPI_KEY unavailable in fork PRs" + run: go test -v ./tests/... diff --git a/tests/e2e_test.go b/tests/e2e_test.go index ac8d910..27b8ea8 100644 --- a/tests/e2e_test.go +++ b/tests/e2e_test.go @@ -1,7 +1,9 @@ package e2e import ( + "bytes" "encoding/json" + "errors" "os" "os/exec" "path/filepath" @@ -12,6 +14,12 @@ import ( var binaryPath string func TestMain(m *testing.M) { + os.Exit(run(m)) +} + +// run builds the binary into a temp dir and runs the tests. It is separate +// from TestMain because os.Exit skips deferred calls, which leaked the dir. +func run(m *testing.M) int { tmpDir, err := os.MkdirTemp("", "serpapi-e2e-*") if err != nil { panic("failed to create temp dir: " + err.Error()) @@ -25,7 +33,7 @@ func TestMain(m *testing.M) { panic("failed to build binary: " + err.Error() + "\n" + string(out)) } binaryPath = bin - os.Exit(m.Run()) + return m.Run() } func findRepoRoot() string { @@ -42,7 +50,9 @@ func findRepoRoot() string { } } -func requireKey(t *testing.T) string { +// requireLive returns SERPAPI_KEY and skips the test without it. Live tests +// reach the real API and can use searches. +func requireLive(t *testing.T) string { t.Helper() key := os.Getenv("SERPAPI_KEY") if key == "" { @@ -51,118 +61,206 @@ func requireKey(t *testing.T) string { return key } -func TestSearchBasic(t *testing.T) { - key := requireKey(t) - cmd := exec.Command(binaryPath, "--api-key", key, "search", "engine=google", "q=coffee") - out, err := cmd.Output() - if err != nil { - t.Fatalf("search failed: %v", err) +type result struct { + code int + stdout, stderr string +} + +// serpapi runs the binary with a fresh config directory and no key in the environment. +func serpapi(t *testing.T, stdin string, args ...string) result { + t.Helper() + return serpapiIn(t, t.TempDir(), stdin, args...) +} + +func serpapiIn(t *testing.T, home, stdin string, args ...string) result { + t.Helper() + cmd := exec.Command(binaryPath, args...) + cmd.Env = append(os.Environ(), + "HOME="+home, "XDG_CONFIG_HOME="+home, "AppData="+home, + "SERPAPI_KEY=", "SERPAPI_API_KEY=", "NO_COLOR=1") + cmd.Stdin = strings.NewReader(stdin) + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + var res result + if err := cmd.Run(); err != nil { + var exitErr *exec.ExitError + if !errors.As(err, &exitErr) { + t.Fatalf("running serpapi: %v", err) + } + res.code = exitErr.ExitCode() + } + res.stdout, res.stderr = stdout.String(), stderr.String() + return res +} + +func wantSuccess(t *testing.T, res result) { + t.Helper() + if res.code != 0 { + t.Fatalf("exit code %d, stderr: %s", res.code, res.stderr) + } +} + +// errorJSON parses the error that failed commands print as the last stderr line. +func errorJSON(t *testing.T, res result) map[string]any { + t.Helper() + lines := strings.Split(strings.TrimSpace(res.stderr), "\n") + var v struct { + Error map[string]any `json:"error"` + } + if err := json.Unmarshal([]byte(lines[len(lines)-1]), &v); err != nil || v.Error == nil { + t.Fatalf("last stderr line is not a JSON error: %q", res.stderr) + } + return v.Error +} + +func TestUsageErrorsWithoutNetwork(t *testing.T) { + tests := []struct { + args []string + msg string + }{ + {nil, "No command given"}, + {[]string{"account"}, "No API key found"}, + {[]string{"archive", "abc123"}, "No API key found"}, + {[]string{"--api-key", "x", "archive", "../bad"}, "Invalid archive ID"}, + {[]string{"--api-key", "x", "search", "engine=google", "q=coffee", "output=xml"}, "Unknown output=xml"}, + {[]string{"locations"}, "Pass a location query"}, + } + for _, tt := range tests { + res := serpapi(t, "", tt.args...) + if res.code != 2 { + t.Errorf("%v: exit code %d, want 2; stderr: %s", tt.args, res.code, res.stderr) + continue + } + e := errorJSON(t, res) + if msg, _ := e["message"].(string); e["code"] != "usage_error" || !strings.Contains(msg, tt.msg) { + t.Errorf("%v: error %v, want usage_error containing %q", tt.args, e, tt.msg) + } + if res.stdout != "" { + t.Errorf("%v: stdout %q, want empty", tt.args, res.stdout) + } } - if !strings.Contains(string(out), "search_metadata") { +} + +func TestHelpAndVersion(t *testing.T) { + res := serpapi(t, "", "--help") + wantSuccess(t, res) + if !strings.Contains(res.stdout, "search") { + t.Errorf("help does not mention search: %s", res.stdout) + } + + res = serpapi(t, "", "--version") + wantSuccess(t, res) + if !strings.HasPrefix(res.stdout, "serpapi version ") { + t.Errorf("version output %q", res.stdout) + } +} + +func TestSearchBasic(t *testing.T) { + key := requireLive(t) + res := serpapi(t, "", "--api-key", key, "search", "engine=google", "q=coffee") + wantSuccess(t, res) + if !strings.Contains(res.stdout, "search_metadata") { t.Error("expected output to contain search_metadata") } } func TestSearchWithFields(t *testing.T) { - key := requireKey(t) - cmd := exec.Command(binaryPath, "--api-key", key, "--fields", "organic_results[0:2]", "search", "engine=google", "q=coffee") - out, err := cmd.Output() - if err != nil { - t.Fatalf("search with fields failed: %v", err) + key := requireLive(t) + res := serpapi(t, "", "--api-key", key, "--fields", "organic_results[0:2]", "search", "engine=google", "q=coffee") + wantSuccess(t, res) + var v struct { + OrganicResults []any `json:"organic_results"` + Metadata json.RawMessage `json:"search_metadata"` + } + if err := json.Unmarshal([]byte(res.stdout), &v); err != nil { + t.Fatalf("output is not JSON: %v", err) } - output := string(out) - if !strings.Contains(output, "[") && !strings.Contains(output, "{") { - t.Error("expected JSON output") + if n := len(v.OrganicResults); n == 0 || n > 2 { + t.Errorf("got %d organic results, want 1-2", n) + } + if v.Metadata != nil { + t.Error("--fields kept search_metadata") } } func TestSearchWithJQ(t *testing.T) { - key := requireKey(t) - cmd := exec.Command(binaryPath, "--api-key", key, "--jq", ".search_metadata.id", "search", "engine=google", "q=coffee") - out, err := cmd.Output() - if err != nil { - t.Fatalf("search with jq failed: %v", err) - } - if len(strings.TrimSpace(string(out))) == 0 { - t.Error("expected non-empty jq output") + key := requireLive(t) + res := serpapi(t, "", "--api-key", key, "--jq", ".search_metadata.id", "search", "engine=google", "q=coffee") + wantSuccess(t, res) + if id := strings.TrimSpace(res.stdout); id == "" || strings.Contains(id, `"`) { + t.Errorf("want one unquoted search ID, got %q", res.stdout) } } func TestAccount(t *testing.T) { - key := requireKey(t) - cmd := exec.Command(binaryPath, "--api-key", key, "account") - out, err := cmd.Output() - if err != nil { - t.Fatalf("account failed: %v", err) + key := requireLive(t) + res := serpapi(t, "", "--api-key", key, "account") + wantSuccess(t, res) + if !strings.Contains(res.stdout, "account_email") { + t.Error("expected account_email in output") } - output := string(out) - if !strings.Contains(output, "account_email") && !strings.Contains(output, "plan") { - t.Error("expected account info in output") + if strings.Contains(res.stdout, key) { + t.Error("account output contains the full API key") } } func TestLocations(t *testing.T) { - // Locations don't require API key but need network - cmd := exec.Command(binaryPath, "locations", "q=austin", "num=3") - out, err := cmd.Output() - if err != nil { - t.Skip("locations endpoint unreachable") + requireLive(t) + res := serpapi(t, "", "locations", "q=austin", "limit=3") + wantSuccess(t, res) + var locations []map[string]any + if err := json.Unmarshal([]byte(res.stdout), &locations); err != nil { + t.Fatalf("output is not a JSON array: %v", err) + } + if n := len(locations); n == 0 || n > 3 { + t.Fatalf("got %d locations, want 1-3", n) } - output := string(out) - if !strings.Contains(output, "[") && !strings.Contains(output, "canonical_name") { - t.Error("expected locations JSON output") + if locations[0]["canonical_name"] == nil { + t.Errorf("location has no canonical_name: %v", locations[0]) } } func TestArchive(t *testing.T) { - key := requireKey(t) + key := requireLive(t) // Search without no_cache so the result is immediately archived. - searchCmd := exec.Command(binaryPath, "--api-key", key, "search", "engine=google", "q=coffee") - searchOut, err := searchCmd.Output() - if err != nil { - t.Fatalf("search failed: %v", err) - } + res := serpapi(t, "", "--api-key", key, "--jq", ".search_metadata.id", "search", "engine=google", "q=coffee") + wantSuccess(t, res) + searchID := strings.TrimSpace(res.stdout) - var result map[string]any - if err := json.Unmarshal(searchOut, &result); err != nil { - t.Fatalf("failed to parse search output: %v", err) + res = serpapi(t, "", "--api-key", key, "archive", searchID) + wantSuccess(t, res) + if !strings.Contains(res.stdout, searchID) { + t.Error("expected the search ID in the archived result") } - meta, ok := result["search_metadata"].(map[string]any) - if !ok { - t.Fatal("no search_metadata in result") - } - searchID, ok := meta["id"].(string) - if !ok || searchID == "" { - t.Fatal("no search ID in search_metadata") - } - - cmd := exec.Command(binaryPath, "--api-key", key, "archive", searchID) - out, err := cmd.CombinedOutput() - if err != nil { - t.Fatalf("archive failed: %v\n%s", err, out) - } - output := string(out) - if !strings.Contains(output, "search_metadata") { - t.Error("expected search_metadata in archive result") + res = serpapi(t, "", "--api-key", key, "archive", searchID, "output=md") + wantSuccess(t, res) + if strings.TrimSpace(res.stdout) == "" { + t.Error("expected Markdown output for the archived search") } } func TestSearchAllPages(t *testing.T) { - key := requireKey(t) - cmd := exec.Command(binaryPath, "--api-key", key, "search", "engine=google", "q=coffee", "num=1", "--all-pages", "--max-pages", "2") - out, err := cmd.Output() - if err != nil { - t.Fatalf("search all pages failed: %v", err) - } - if !strings.Contains(string(out), "organic_results") { + key := requireLive(t) + res := serpapi(t, "", "--api-key", key, "search", "engine=google", "q=coffee", + "--fields", "organic_results", "--all-pages", "--max-pages", "2") + wantSuccess(t, res) + var v map[string]json.RawMessage + if err := json.Unmarshal([]byte(res.stdout), &v); err != nil { + t.Fatalf("output is not JSON: %v", err) + } + if v["organic_results"] == nil { t.Error("expected organic_results in merged output") } + if v["serpapi_pagination"] != nil { + t.Error("merged output keeps serpapi_pagination") + } } func TestSearchAllPagesWithFields(t *testing.T) { - key := requireKey(t) + key := requireLive(t) cmd := exec.Command( binaryPath, "--api-key", key, @@ -218,39 +316,29 @@ func TestSearchAllPagesWithFields(t *testing.T) { } func TestInvalidAPIKey(t *testing.T) { - cmd := exec.Command(binaryPath, "--api-key", "invalid", "search", "engine=google", "q=test") - err := cmd.Run() - if err == nil { - t.Fatal("expected failure with invalid API key") - } - if exitErr, ok := err.(*exec.ExitError); ok { - if exitErr.ExitCode() != 1 { - t.Errorf("expected exit code 1, got %d", exitErr.ExitCode()) - } + requireLive(t) + res := serpapi(t, "", "--api-key", "invalid", "search", "engine=google", "q=test") + if res.code != 1 { + t.Fatalf("exit code %d, want 1; stderr: %s", res.code, res.stderr) } -} - -func TestNoArgs(t *testing.T) { - cmd := exec.Command(binaryPath) - err := cmd.Run() - if err == nil { - t.Fatal("expected failure with no args") - } - if exitErr, ok := err.(*exec.ExitError); ok { - if exitErr.ExitCode() != 2 { - t.Errorf("expected exit code 2, got %d", exitErr.ExitCode()) - } + if e := errorJSON(t, res); e["code"] != "api_error" || e["status"] != float64(401) { + t.Errorf("error %v, want api_error with status 401", e) } } func TestLoginFlow(t *testing.T) { - key := requireKey(t) - // Run login in an isolated HOME so it doesn't touch the real config. - tmpHome := t.TempDir() - cmd := exec.Command(binaryPath, "login") - cmd.Stdin = strings.NewReader(key + "\n") - cmd.Env = append(os.Environ(), "HOME="+tmpHome, "XDG_CONFIG_HOME="+tmpHome) - if err := cmd.Run(); err != nil { - t.Fatalf("login failed: %v", err) + key := requireLive(t) + home := t.TempDir() + + res := serpapiIn(t, home, key+"\n", "login") + wantSuccess(t, res) + if res.stderr != "" { + t.Errorf("login without a terminal should succeed silently, stderr: %s", res.stderr) + } + + res = serpapiIn(t, home, "", "account") + wantSuccess(t, res) + if !strings.Contains(res.stdout, "account_email") { + t.Error("the saved key did not work") } } From 469b0b1975f729ece40b5f56223a5bf017dfcf77 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Mon, 12 Oct 2026 14:00:36 +0200 Subject: [PATCH 23/25] [Login] Escape the account email before printing it to a terminal login printed the account's email to the terminal as the API sent it: in a real PTY, an email holding ESC ] 52 (set the clipboard), ESC [ 2J and a bidi override reached the terminal in both "Logged in as" and "Already logged in as". The email now goes through ForTerminal, as search results do. Co-Authored-By: Claude Opus 5.5 --- pkg/cmd/cmd_test.go | 18 ++++++++++++++++++ pkg/cmd/login.go | 3 ++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/pkg/cmd/cmd_test.go b/pkg/cmd/cmd_test.go index 1435c90..18c8342 100644 --- a/pkg/cmd/cmd_test.go +++ b/pkg/cmd/cmd_test.go @@ -604,6 +604,24 @@ func TestLoginWithoutKey(t *testing.T) { wantError(t, execute(t, "\n", "login"), 2, "usage_error", "No API key given") } +// login shows the account's email on a terminal, so it must be as safe there +// as a search result: a real PTY run printed ESC ] 52 and ESC [ 2J from it. +func TestAccountEmailIsSafeOnATerminal(t *testing.T) { + fakeAPI(t, respond(`{"account_email":"dev\u001b]52;c;ZXZpbA==\u0007\u001b[2J\u202e@example.com"}`)) + + email, err := accountEmail(context.Background(), "testkey") + + if err != nil { + t.Fatal(err) + } + if strings.ContainsAny(email, "\x1b\x07\u202e") { + t.Errorf("accountEmail = %q, want control and bidi characters escaped", email) + } + if !strings.Contains(email, `dev\u001b]52`) { + t.Errorf("accountEmail = %q, want ESC written as \\u001b", email) + } +} + func TestNoCommandPrintsHelpToStderr(t *testing.T) { fakeAPI(t, nil) diff --git a/pkg/cmd/login.go b/pkg/cmd/login.go index fc6122b..648c613 100644 --- a/pkg/cmd/login.go +++ b/pkg/cmd/login.go @@ -15,6 +15,7 @@ import ( "github.com/serpapi/serpapi-cli/pkg/config" clierrors "github.com/serpapi/serpapi-cli/pkg/errors" + "github.com/serpapi/serpapi-cli/pkg/output" ) var loginCmd = &cobra.Command{ @@ -112,5 +113,5 @@ func accountEmail(ctx context.Context, apiKey string) (string, error) { if json.Unmarshal(raw, &account) != nil || account.AccountEmail == "" { return "unknown", nil } - return account.AccountEmail, nil + return string(output.ForTerminal([]byte(account.AccountEmail))), nil } From d03f846ab90a67547adf8b6a0877a521c33367c3 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Mon, 12 Oct 2026 14:04:06 +0200 Subject: [PATCH 24/25] [Docs] Say how to tag a release safely and where to get an API key AGENTS.md now tags the merge commit after its CI passed with no skipped live test, checks the tag points there, never moves a published tag, says how to publish written release notes, and pulls a bad version for go install users with retract. README's Quick Start links the API key page and the keyless demo, and the Go section says go install needs Go 1.25. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 12 ++++++++++-- README.md | 4 ++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 9d32d45..28c7aa5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -6,8 +6,16 @@ - Only a pushed `v*` tag releases. `.github/workflows/release.yml` then: 1. runs GoReleaser (`.goreleaser.yml`), which publishes a GitHub release with 5 archives (darwin and linux on amd64 and arm64, windows on amd64) and `serpapi__checksums.txt`, and stamps the version into `serpapi --version`; 2. writes `Formula/serpapi-cli.rb` in `serpapi/homebrew-tap` from those checksums and pushes it as a `serpapi-cli ` commit. -- A tag reaches every Homebrew and `go install` user, so tagging is a maintainer's call. Tags here are lightweight: `git tag v0.3.0 origin/main && git push origin v0.3.0`. +- A tag reaches every Homebrew and `go install` user, so tagging is a maintainer's call. Tag the merge commit on `main` once its CI has passed, live e2e included: the live tests skip without `SERPAPI_KEY`, so the e2e log must have no `--- SKIP` line. Tags here are lightweight: + ```bash + git fetch origin + git tag v0.3.0 + git rev-list -n1 v0.3.0 # must print that sha + git push origin v0.3.0 + ``` +- Never move, delete or reuse a published tag: Go's module proxy and checksum database keep the first content they saw for each version, so a moved tag never reaches most `go install` users and fails checksum verification for the rest. +- The release body is GoReleaser's list of commits. To publish written notes instead, run `gh release edit v0.3.0 -R serpapi/serpapi-cli --notes-file ` after the Release run. - Release binaries build with the latest stable Go (`go-version: stable`); the `go` line in `go.mod` is the oldest Go that `go install` users need. - The formula's `test do` block in `release.yml` runs the released binary, e.g. `serpapi account` without a key must exit 2 with "No API key". Change it together with the behavior it checks. - Check a release: the Release run is green, the release has the 5 archives and the checksums file, the tap has the `serpapi-cli ` commit, and `brew upgrade serpapi-cli && brew test serpapi-cli` passes. -- To pull a bad release, revert the formula commit in the tap so new installs get the previous version, and fix forward with a new tag: `go install ...@latest` takes the highest tag. +- To pull a bad release, revert the formula commit in the tap so new installs get the previous version, and fix forward: release the next patch version with the bad one retracted in its `go.mod` (`retract v0.3.0` in v0.3.1), since `go install ...@latest` takes the highest tag that isn't retracted. diff --git a/README.md b/README.md index d24bd7f..3c87292 100644 --- a/README.md +++ b/README.md @@ -21,8 +21,12 @@ Download directly from [GitHub Releases](https://github.com/serpapi/serpapi-cli/ go install github.com/serpapi/serpapi-cli/cmd/serpapi@latest ``` +This needs Go 1.25 or newer; with the default `GOTOOLCHAIN=auto`, Go 1.21 and later download it on their own. + ## Quick Start +Your API key is at . Without one, `search` still answers a few demo queries such as `q=coffee`. + ```bash serpapi login # check and save your API key serpapi search engine=google q=coffee # JSON results From d70ee5f41b08d83cfe8b3d5c75a6b4e2b5826e29 Mon Sep 17 00:00:00 2001 From: ilyazub Date: Mon, 12 Oct 2026 14:20:10 +0200 Subject: [PATCH 25/25] [CLI] Keep stderr to JSON lines when serpapi runs with no command serpapi with no command printed its 58-line usage to stderr before the JSON error, even when stderr wasn't a terminal: the one case of the 120 in the behavior matrix whose stderr wasn't JSON lines, which README promises scripts. The usage now shows only on a terminal, as login's messages do; off one, stderr is the error line alone, and it names --help. Co-Authored-By: Claude Opus 5.5 --- pkg/cmd/cmd_test.go | 33 ++++++++++++++++++++++++++++++++- pkg/cmd/root.go | 7 +++++-- 2 files changed, 37 insertions(+), 3 deletions(-) diff --git a/pkg/cmd/cmd_test.go b/pkg/cmd/cmd_test.go index 18c8342..eada884 100644 --- a/pkg/cmd/cmd_test.go +++ b/pkg/cmd/cmd_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "fmt" + "io" "net/http" "net/http/httptest" "net/url" @@ -18,6 +19,7 @@ import ( "github.com/serpapi/serpapi-cli/pkg/api" "github.com/serpapi/serpapi-cli/pkg/config" + "github.com/serpapi/serpapi-cli/pkg/output" ) type result struct { @@ -622,7 +624,36 @@ func TestAccountEmailIsSafeOnATerminal(t *testing.T) { } } -func TestNoCommandPrintsHelpToStderr(t *testing.T) { +// Off a terminal, stderr carries only JSON lines, so a script reading it line +// by line gets the error and nothing else. +func TestNoCommandOffATerminalPrintsOnlyTheError(t *testing.T) { + fakeAPI(t, nil) + + res := execute(t, "") + + if res.code != 2 { + t.Fatalf("exit code = %d, want 2", res.code) + } + if res.stdout != "" { + t.Errorf("stdout = %q, want empty", res.stdout) + } + dec := json.NewDecoder(strings.NewReader(res.stderr)) + var got struct{ Error map[string]any } + if err := dec.Decode(&got); err != nil { + t.Fatalf("stderr = %q, want a JSON error line first: %v", res.stderr, err) + } + if got.Error["code"] != "usage_error" || got.Error["message"] != "No command given; see 'serpapi --help'" { + t.Errorf("error = %v", got.Error) + } + if err := dec.Decode(&struct{}{}); err != io.EOF { + t.Errorf("stderr = %q, want nothing after the error line", res.stderr) + } +} + +func TestNoCommandOnATerminalPrintsUsage(t *testing.T) { + isTerminal := output.IsTerminal + output.IsTerminal = func(io.Writer) bool { return true } + t.Cleanup(func() { output.IsTerminal = isTerminal }) fakeAPI(t, nil) res := execute(t, "") diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index 648cac3..373697b 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -98,8 +98,11 @@ are free.`, SilenceUsage: true, SilenceErrors: true, RunE: func(cmd *cobra.Command, args []string) error { - cmd.SetOut(cmd.ErrOrStderr()) - _ = cmd.Help() + // Off a terminal, stderr carries only JSON lines; the error names --help. + if output.IsTerminal(cmd.ErrOrStderr()) { + cmd.SetOut(cmd.ErrOrStderr()) + _ = cmd.Help() + } return &clierrors.UsageError{Message: "No command given; see 'serpapi --help'"} }, }