Repository navigation
Conversation
`serpapi extract URL [key=value...]` sends an engine=extract search, so --fields, --jq, output=md, --debug and --timeout work as in search. `serpapi map URL` and `serpapi crawl URL` are shortcuts for `serpapi extract URL mode=map` and `mode=crawl`: same request, parameters and flags. The URL may be given bare or as url=, and must be one absolute http or https URL; anything else is a usage error before any request, as are engine= other than extract, a mode= that contradicts map or crawl, a word without "=", which search would take as q=, and search's --all-pages and --image. Unlike search, the commands need an API key up front, since the Extract API answers nothing without one. A timed-out extraction no longer blames slow engines such as google_ai_mode: extract and map say only how to retry, and crawl suggests --timeout 300 and that each page counts. The README documents the three commands, says map and crawl are shortcuts for mode=map and mode=crawl, and lists crawl among the requests that can outlast the default --timeout. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
trusche
left a comment
There was a problem hiding this comment.
Thanks @strzibny I don't know Go, so this is a fully automated AI review with my usual coding agents. Let me know if that's of any use or you'd rather not get reviews like this.
Everything below and inline is verbatim from Opus 5.5. High
Claude Opus 5.5 AI-assisted review: read the code, ran the tests and checked the CLI by hand.
Approve with comments. Nothing blocks the merge. Please add the release:minor label before you merge: this PR adds three new commands, and without a label the merge releases only a patch version (see AGENTS.md, "Releases").
QA: go test ./pkg/..., go vet and the offline e2e tests pass. I ran extract, map and crawl against the live API (3 searches), and 14 usage-error and timeout cases offline. The help text matches the Extract API docs. Details are in the inline comments; all of them are non-blocking.
| if raw { | ||
| body, err := withSpinner(label, func() ([]byte, error) { return client.SearchRaw(ctx, p) }) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| return outputError(output.PrintRaw(cmd.OutOrStdout(), body)) | ||
| } | ||
| result, err := withSpinner(label, 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(p)) |
There was a problem hiding this comment.
These lines are a copy of sendSearch (search.go:180-201). Only the spinner text is different. The extract commands do not have --all-pages, so allPagesFlag is always false here. Add a label argument to sendSearch and call it from here. That removes about 15 lines.
The output= and --jq check at lines 117-125 is now in 3 places (also search.go:130-138 and archive.go:47-53). Maybe move it into one helper.
| case strings.EqualFold(query.Get("engine"), "extract") && strings.EqualFold(query.Get("mode"), "crawl"): | ||
| message = timedOut + ". A crawl extracts every page it visits, so it takes longer than one page: " + | ||
| retry(max(suggestedTimeout(c.http.Timeout), 300)) + | ||
| ". The crawl may still finish on SerpApi's side, count one search per page and be available in your search archive" | ||
| case strings.EqualFold(query.Get("engine"), "extract"): | ||
| message = timedOut + ": " + retry(suggestedTimeout(c.http.Timeout)) + ". The extraction" + onSerpApi | ||
| default: |
There was a problem hiding this comment.
The HTTP client now reads engine=extract and mode=crawl from the request URL, and 300 is a fixed number here. The /image and google_ai_mode cases already do something similar, so this is OK. But this is the second engine-specific branch in the client. Maybe give 300 a name.
A question: when a crawl times out on the CLI side, it can still finish on the server and charge one search per page. This advice arrives only after that cost. Should crawl default to a 300s timeout when the user does not set --timeout or SERPAPI_TIMEOUT? Then this branch may not be needed.
| mode=crawl every page of the website it visits; same as 'serpapi crawl' | ||
| include_tags=, exclude_tags= CSS selectors to keep or drop | ||
| include_markdown=true add the main content as Markdown to JSON | ||
| output=md only the main content, as Markdown |
There was a problem hiding this comment.
"only the main content" is not quite correct: the Markdown starts with a YAML block with the search metadata (live: serpapi extract https://example.com/ output=md). Maybe: "the main content as Markdown, after the search metadata".
| continue | ||
| } | ||
| // search takes a bare word as q=; here it is a parameter missing its "=". | ||
| if !strings.Contains(a, "=") { |
There was a problem hiding this comment.
Good. But the root help at root.go:60-61 still says "words without "=" are joined into q" for all commands. That is not true for extract, map and crawl now. Maybe add "in search" there.
| // as the URL, so it is reported as an invalid one. | ||
| func isURLArg(arg string) bool { | ||
| key, _, _ := strings.Cut(arg, "=") | ||
| return arg == "" || strings.ContainsAny(key, ":/.?") |
There was a problem hiding this comment.
An empty argument counts as the URL. When it comes with a real URL, the error has an empty first item:
$ serpapi extract "" https://example.com
extract takes one URL, got 2: , https://example.com; ...
Maybe report the empty URL first, before the CLI counts the URLs.
| } | ||
| msg := fmt.Sprintf("Invalid URL %q: the Extract API takes an absolute http or https URL", raw) | ||
| if !strings.Contains(raw, "://") { | ||
| if u, err := url.Parse("https://" + raw); err == nil && strings.Contains(u.Hostname(), ".") { |
There was a problem hiding this comment.
Small: serpapi extract mailto:me@x.com suggests e.g. https://mailto:me@x.com. url.Parse reads mailto:me as a user and password, so the hostname is x.com. Maybe suggest only when u.User == nil.
| if isExtractCommand(cmd) { | ||
| // search's --all-pages and --image have no Extract API parameter; | ||
| // its --max-pages does, as crawl's max_pages. | ||
| if name != "max-pages" && searchCmd.Flags().Lookup(name) != nil { | ||
| return msg + "; --" + name + " works only with search; " + params | ||
| } | ||
| // Extract API parameters use underscores, as in max_pages. | ||
| name = strings.ReplaceAll(name, "-", "_") | ||
| } |
There was a problem hiding this comment.
The change from - to _ applies only to the extract commands. search --no-cache still suggests no-cache=VALUE. That bug is not from this PR, but the same fix applies there. Maybe apply it to all commands, or leave it for a separate PR.
Also, serpapi extract URL --max-pages 3 suggests max_pages=VALUE, but only crawl uses that parameter.
| serpapi extract https://example.com --jq '.inline_links[].link' | ||
| ``` | ||
|
|
||
| It counts one search. The Extract API runs no JavaScript and follows each site's `robots.txt`. Search pages that have their own engine, such as Google or Bing, are redirected to it and return that engine's JSON. Unlike `search`, `extract`, `map` and `crawl` need an API key. |
There was a problem hiding this comment.
"It counts one search" comes right after the table row for mode=crawl, which costs one search per page. The help at extract.go:37-38 has the correct sentence: "scrape and map count one search each; crawl counts one per page it extracts".
|
|
||
| `serpapi crawl URL` is a shortcut for `serpapi extract URL mode=crawl`: it sends the same request and takes the same parameters and flags. | ||
|
|
||
| Starts at the URL, follows links on the same website and extracts every page it visits, one entry each in `pages`. `max_pages` caps the pages, the first included (default 10, at most 100), and `max_depth` how many links deep it goes (default 2, at most 3; `0` is only the first page). |
There was a problem hiding this comment.
The help (extract.go:81) and the API docs list include_subdomains=true for crawl, but this section does not mention it.
| func TestExtractCommandsTimeOut(t *testing.T) { | ||
| const extraction = "Request to /search.json timed out after 50ms: retry with a higher --timeout, such as --timeout 120" | ||
| const crawl = "Request to /search.json timed out after 50ms. A crawl extracts every page it visits, so it takes longer than one page: " + | ||
| "retry with a higher --timeout, such as --timeout 300" | ||
| for _, tc := range []struct { | ||
| name, env, want string | ||
| args []string | ||
| }{ | ||
| {"extract --timeout", "", extraction, []string{"--timeout", "0.05", "extract"}}, | ||
| {"map --timeout", "", extraction, []string{"--timeout", "50ms", "map"}}, | ||
| {"extract mode=crawl", "", crawl, []string{"--timeout", "50ms", "extract", "mode=crawl"}}, | ||
| {"crawl --timeout", "", crawl, []string{"--timeout", "0.05", "crawl"}}, | ||
| {"crawl SERPAPI_TIMEOUT", "0.05", crawl, []string{"crawl"}}, | ||
| } { | ||
| t.Run(tc.name, func(t *testing.T) { | ||
| fakeAPI(t, hang) | ||
| t.Setenv("SERPAPI_TIMEOUT", tc.env) | ||
|
|
||
| start := time.Now() | ||
| res := execute(t, "", append(append([]string{"--api-key", extractKey}, tc.args...), "https://example.com/")...) | ||
|
|
||
| wantError(t, res, 1, "network_error", tc.want) | ||
| if strings.Contains(res.stdout+res.stderr, "Slow engines") { | ||
| t.Errorf("advice blames slow engines:\n%s%s", res.stdout, res.stderr) | ||
| } | ||
| if elapsed := time.Since(start); elapsed > 2*time.Second { | ||
| t.Errorf("took %v, want the 50ms timeout to stop it", elapsed) | ||
| } | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Some tests check the same thing more than once:
- The timeout messages are checked here (5 cases) and also in
client_test.goTestExtractTimeoutAdvice. - The debug trace runs 3 times (lines 209-245).
- "Needs an API key" is in lines 118-124 and 197-204, and in 2 rows of
tests/e2e_test.go.
One case for each is enough. Not blocking.
This PR adds Extract API support. I wanted to add specific
extractsubcommand but though it's actually useful for CLI to also addmapandcrawlaliases for developer experience.Done with Opus 5.5 High and Max. Some futher notes:
engine=extracton/search.json, so these commands reuse search's request and output code:--fields,--jq,output=md,--debug,--timeoutandarchivework as in search.search, they need an API key up front; the API returns 401 without one.engine=other than extract, amode=that contradictsmap/crawl, a parameter missing its=, and search-only flags such as--all-pages. An argument counts as the URL when it has:,/,.or?before any=.crawlsuggests--timeout 300. This changes shared code:networkErrorinpkg/api/client.gonow picks the advice from the request'sengineandmode.crawlcosts one search per extracted page (defaultmax_pages=10). The help and README say so.Note Extract API can already be used by passing
engine=extract, but this makes things nicer.