diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f085081..6f2d9c2 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,12 +36,10 @@ 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 + # 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/.github/workflows/release.yml b/.github/workflows/release.yml index c78980f..30d63e7 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 @@ -16,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 diff --git a/AGENTS.md b/AGENTS.md new file mode 100644 index 0000000..28c7aa5 --- /dev/null +++ b/AGENTS.md @@ -0,0 +1,21 @@ +# 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. 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: 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 bcbef14..db9f2bd 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 @@ -21,105 +21,97 @@ 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 -```bash -# Authenticate -serpapi login +Your API key is at . Without one, `search` still answers a few demo queries such as `q=coffee`. -# Perform a search -serpapi search engine=google q=coffee +```bash +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`. + +### archive -Retrieve account information and usage statistics. +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 +122,46 @@ serpapi login ## Configuration -### Authentication priority chain +The API key comes from the first of: -The CLI checks for API keys in this order: +1. `--api-key` +2. `SERPAPI_KEY` +3. `SERPAPI_API_KEY` +4. the key saved by `serpapi login` -1. `--api-key` flag -2. `SERPAPI_KEY` environment variable -3. Config file: `~/.config/serpapi/config.toml` +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. -If no API key is found, run `serpapi login` to authenticate interactively. - -> **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 diff --git a/go.mod b/go.mod index afc3781..052d7d3 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/spf13/pflag v1.0.10 + 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 + 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= 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) + } + } +} diff --git a/pkg/cmd/account.go b/pkg/cmd/account.go index 9aa4843..ff42659 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), nil) } -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..9cbcb08 100644 --- a/pkg/cmd/archive.go +++ b/pkg/cmd/archive.go @@ -1,20 +1,30 @@ 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 ", - Short: "Retrieve a previously cached search by ID", - Args: cobra.ExactArgs(1), - RunE: runArchive, + 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. +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.RangeArgs(1, 2), + RunE: runArchive, } func init() { @@ -26,24 +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"} } - - apiKey, err := resolveAPIKey() + 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"} + } - client, err := newClient(apiKey) + apiKey, err := resolveAPIKey() if err != nil { return err } + client := newClient(apiKey) + ctx := cmd.Context() - sp := newSpinner("Fetching archive...") - sp.Start() - defer sp.Stop() - result, err := client.Archive(cmd.Context(), id) + 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(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 new file mode 100644 index 0000000..eada884 --- /dev/null +++ b/pkg/cmd/cmd_test.go @@ -0,0 +1,692 @@ +package cmd + +import ( + "bytes" + "context" + "encoding/json" + "fmt" + "io" + "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" + "github.com/serpapi/serpapi-cli/pkg/output" +) + +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 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}}`)) + 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 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") + + 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 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") + + 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") +} + +// 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) + } +} + +// 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, "") + + 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..a344ca9 --- /dev/null +++ b/pkg/cmd/edge_test.go @@ -0,0 +1,498 @@ +package cmd + +import ( + "bytes" + "context" + "fmt" + "io" + "net" + "net/http" + "os" + "path/filepath" + "runtime" + "strings" + "sync" + "testing" + "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 +// 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/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}, + {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/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"}, + {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"}, + {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: "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"}, + 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/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}, + {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, + 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") + } +} + +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() + 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/fuzz_test.go b/pkg/cmd/fuzz_test.go new file mode 100644 index 0000000..9e46aa1 --- /dev/null +++ b/pkg/cmd/fuzz_test.go @@ -0,0 +1,62 @@ +package cmd + +import ( + "encoding/json" + "net/url" + "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) + } + }) +} + +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/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 741e554..3a60e73 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,19 @@ 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 + 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)"} } - - 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, nil) } + +func blank(s string) bool { return strings.TrimSpace(s) == "" } diff --git a/pkg/cmd/login.go b/pkg/cmd/login.go index 39d6e3f..648c613 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" @@ -13,97 +15,103 @@ 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{ 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, ok := config.LoadAPIKey(); ok { - 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 - } - raw, err := client.Account(cmd.Context()) - if err != nil { - return err + 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 string(output.ForTerminal([]byte(account.AccountEmail))), nil } - diff --git a/pkg/cmd/root.go b/pkg/cmd/root.go index 41eb197..373697b 100644 --- a/pkg/cmd/root.go +++ b/pkg/cmd/root.go @@ -1,11 +1,20 @@ package cmd import ( + "bufio" "bytes" + "context" "encoding/json" + "errors" "fmt" + "io" + "maps" "os" + "os/signal" + "slices" "strconv" + "strings" + "syscall" "time" "github.com/spf13/cobra" @@ -24,76 +33,200 @@ 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`, + Use: "serpapi", + 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 + 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}} + {"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. +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 +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 search engine=google q=coffee output=md + 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.Help() - return &clierrors.UsageError{Message: "No command specified. See usage above."} + // 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'"} }, } -// Execute runs the root command. +// 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() { - if err := rootCmd.Execute(); err != nil { - clierrors.PrintError(err) - os.Exit(clierrors.ExitCode(err)) + 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. 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) +} + 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 { - return &clierrors.UsageError{Message: err.Error()} + rootCmd.SetFlagErrorFunc(func(cmd *cobra.Command, err error) error { + return &clierrors.UsageError{Message: flagErrorHint(cmd, err.Error())} }) } -// 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") +// 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 } - return config.ResolveAPIKey(key) + if name, ok := strings.CutPrefix(msg, "unknown flag: --"); ok { + return msg + "; API parameters are key=value pairs, e.g. " + name + "=VALUE" + } + return msg } -// 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, ok := config.LoadAPIKey(); ok { - 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 +234,157 @@ 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 } - return api.NewWithTimeout(apiKey, timeout), nil + for _, name := range apiKeyEnvVars { + if key := strings.TrimSpace(os.Getenv(name)); key != "" { + return key, 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()} +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} } - results, err := jq.Apply(jqFlag, v) - if err != nil { - return &clierrors.UsageError{Message: err.Error()} + return nil + } +} + +// handleOutput applies --jq, if set, and prints the JSON result to stdout. +// 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 == "" { + w := cmd.OutOrStdout() + out := output.FormatJSON(w, raw) + if next != nil { + hintLargeJSON(cmd, out, next) } - for _, val := range results { - if err := output.PrintJQValue(val, os.Stdout); err != nil { - return &clierrors.APIError{Message: fmt.Sprintf("Output error: %s", err)} - } + _, err := w.Write(out) + return outputError(err) + } + 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. 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 { + return &clierrors.UsageError{Message: err.Error()} + } + w := cmd.OutOrStdout() + 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++ } - return nil + 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 && 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: jqErrorHint(err.Error(), v)} } - if err := output.PrintJSON(raw); err != nil { - return &clierrors.APIError{Message: fmt.Sprintf("Output error: %s", err)} + 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 } -// newSpinner creates a spinner with the given label. -func newSpinner(label string) *spinner.Spinner { - return spinner.New(label) +// 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 + } + return &clierrors.APIError{Message: "Output error: " + err.Error()} } diff --git a/pkg/cmd/search.go b/pkg/cmd/search.go index 2470a3f..6514da6 100644 --- a/pkg/cmd/search.go +++ b/pkg/cmd/search.go @@ -1,16 +1,22 @@ package cmd import ( + "bufio" "bytes" "encoding/json" + "errors" "fmt" + "io" + "net/http" "net/url" - "os" - "sort" + "strings" "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" ) @@ -19,210 +25,404 @@ var ( maxPagesFlag int ) -const ( - defaultMaxPages = 100 - paginationRestrictorPath = "serpapi_pagination.next" +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"} + 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. 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 + +--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. 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. +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 output=md + 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 } + warnRepeated(cmd, parsed) + p := params.ParamsToMap(parsed) - apiKey := resolveAPIKeyOptional() - paramsMap := params.ParamsToMap(parsed) - params.ApplyFields(paramsMap, fieldsFlag) - - client, err := newClient(apiKey) - if err != nil { - return err + 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} + } + if maxPagesSet && !allPagesFlag { + warn(cmd, "--max-pages has no effect without --all-pages") } - hasMaxPages := cmd.Flags().Changed("max-pages") - - if hasMaxPages && !allPagesFlag { - fmt.Fprintln(os.Stderr, "Warning: --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. + apiKey, keyErr := resolveAPIKey() + var cfgErr *clierrors.UsageError + if keyErr != config.ErrNoAPIKey && errors.As(keyErr, &cfgErr) { + warn(cmd, "%s; searching without a key", cfgErr.Message) } + err = sendSearch(cmd, newClient(apiKey), p, raw, restrictor != "") + if apiKey == "" { + err = explainMissingKey(err) + } + return err +} - if !allPagesFlag { - sp := newSpinner("Searching...") - sp.Start() - defer sp.Stop() - raw, err := client.Search(cmd.Context(), paramsMap) +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 handleOutput(raw) + return outputError(output.PrintRaw(cmd.OutOrStdout(), body)) + } + if allPagesFlag { + return searchAllPages(cmd, client, p, restricted) } - // Pagination mode - maxPages := defaultMaxPages - if hasMaxPages { - maxPages = maxPagesFlag + result, err := withSpinner("Searching...", func() (json.RawMessage, error) { return client.Search(ctx, p) }) + if err != nil { + return err } + if restricted && isEmptyObject(result) { + warnNothingMatched(cmd) + } + return handleOutput(cmd, result, searchHint) +} - paginationRestrictor := withPaginationRestrictor(paramsMap["json_restrictor"]) - if paginationRestrictor != "" { - paramsMap["json_restrictor"] = paginationRestrictor +// 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, + } +} - currentParams := paramsMap - var accumulated map[string]any - visitedPages := make(map[string]bool) - pagesFetched := 0 - visitedPages[canonicalParamsKey(currentParams)] = true +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"} +} - for { - sp := newSpinner(fmt.Sprintf("Fetching page %d...", pagesFetched+1)) - sp.Start() - raw, err := client.Search(cmd.Context(), currentParams) - sp.Stop() - if err != nil { - return err - } - pagesFetched++ +func isEmptyObject(raw []byte) bool { + var m map[string]json.RawMessage + return json.Unmarshal(raw, &m) == nil && m != nil && len(m) == 0 +} - 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()} +func warnNothingMatched(cmd *cobra.Command) { + warn(cmd, "json_restrictor matched nothing or is malformed; compare its paths with the unfiltered response") +} + +// 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 } + } - nextURL := extractNextURL(result) + 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...", pages+1), func() (json.RawMessage, error) { + return client.Search(ctx, pageParams) + }) + 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()} + } + } + if err != nil { + if pages == 0 { + return err + } + pageErr = err + break + } + pages++ - 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 merged == nil { + merged = map[string]any{} + } + delete(merged, "serpapi_pagination") + if restricted && len(merged) == 0 { + warnNothingMatched(cmd) } - if accumulated == nil { - return &clierrors.APIError{Message: "No results returned"} + 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))} } - delete(accumulated, "serpapi_pagination") + // 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 annotatePageError(pageErr, pages) +} - var buf bytes.Buffer - enc := json.NewEncoder(&buf) - enc.SetEscapeHTML(false) - if err := enc.Encode(accumulated); err != nil { +// 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) + } + 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())) + } + // 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()} } - return handleOutput(json.RawMessage(bytes.TrimRight(buf.Bytes(), "\n"))) + 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) + } + return outputError(bw.Flush()) } -// withPaginationRestrictor adds the pagination field needed internally while -// preserving the caller's server-side field projection. -func withPaginationRestrictor(restrictor string) string { - if restrictor == "" { - return "" +func printedPages(pages int) string { + if pages > 1 { + return fmt.Sprintf("pages 1-%d were", pages) } - return restrictor + "," + paginationRestrictorPath + return "page 1 was" } -// 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 "" +// 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} } - pagMap, ok := pag.(map[string]any) - if !ok { - return "" + 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) + } + } +} 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) +} diff --git a/pkg/config/config.go b/pkg/config/config.go index 4deb20c..8d8c306 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 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 +// 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) } } 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") 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) + } +} diff --git a/pkg/output/output.go b/pkg/output/output.go index 6817e79..bd5c479 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" @@ -13,40 +14,142 @@ 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. -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 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)) + if os.Getenv("NO_COLOR") == "" { + out = pretty.Color(out, darkStyle) + } + return out + } + + 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, +// 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 +174,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 } } @@ -82,4 +189,3 @@ func formatNumber(f float64) string { } return fmt.Sprintf("%g", f) } - diff --git a/pkg/output/output_test.go b/pkg/output/output_test.go index e4b5e52..c14b3aa 100644 --- a/pkg/output/output_test.go +++ b/pkg/output/output_test.go @@ -2,13 +2,15 @@ package output import ( "bytes" + "encoding/json" + "reflect" "strings" "testing" ) 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 +21,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 +32,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 +43,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 +56,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 +65,79 @@ 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 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 + 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) + } + } +} + +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 +} 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) + } + }) +} 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) + } + }) +} 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) + } + }) +} 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") + } + } +} 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") } }