fix: harden pagination and exit-code classification #3
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -23,6 +23,13 @@ type ListResult struct { | |
| Requests int `json:"requests"` | ||
| } | ||
|
|
||
| // MaxListRequests bounds the `--all` pagination loop. It is deliberately far | ||
| // above any real result set: at the largest page size any endpoint accepts | ||
| // (2000, live chat) it allows 20,000,000 items, and every other endpoint caps | ||
| // at 50 or 100 per page. A legitimate `--all` cannot reach it, so hitting it | ||
| // means the server is not terminating. | ||
| const MaxListRequests = 10000 | ||
|
|
||
| func (c *Client) List(ctx context.Context, resource string, params url.Values, options PageOptions) (ListResult, error) { | ||
| if options.PageSize > 0 { | ||
| params.Set("maxResults", fmt.Sprint(options.PageSize)) | ||
|
|
@@ -31,6 +38,12 @@ func (c *Client) List(ctx context.Context, resource string, params url.Values, o | |
| params.Set("pageToken", options.PageToken) | ||
| } | ||
| result := ListResult{Items: make([]map[string]any, 0)} | ||
| // Seed with the caller's starting token: a server echoing it back is the | ||
| // same loop as any other repeated token and must not re-fetch the page. | ||
| seenTokens := make(map[string]struct{}) | ||
| if options.PageToken != "" { | ||
| seenTokens[options.PageToken] = struct{}{} | ||
| } | ||
| for { | ||
| response, err := c.Get(ctx, resource, params) | ||
| if err != nil { | ||
|
|
@@ -47,15 +60,38 @@ func (c *Client) List(ctx context.Context, resource string, params url.Values, o | |
| } | ||
| items = filtered | ||
| } | ||
| truncated := false | ||
| if options.Limit > 0 && len(result.Items)+len(items) > options.Limit { | ||
| items = items[:options.Limit-len(result.Items)] | ||
| truncated = true | ||
| } | ||
| result.Items = append(result.Items, items...) | ||
| // A page from which items were discarded reports no resume token: | ||
| // the server's token points past the discarded tail, so resuming from | ||
| // it would silently skip data. A page trimmed to exactly its own | ||
| // length is not truncated and keeps its token. | ||
| if truncated { | ||
| result.NextPageToken = "" | ||
| } else { | ||
| result.NextPageToken = response.NextPageToken | ||
| } | ||
| if !options.All || result.NextPageToken == "" || (options.Limit > 0 && len(result.Items) >= options.Limit) { | ||
| break | ||
| } | ||
| // A token we have already followed can only return the same page | ||
| // again. Stop and report "", which correctly says "no valid resume | ||
| // point" rather than handing back a token that loops. | ||
| if _, seen := seenTokens[result.NextPageToken]; seen { | ||
|
Comment on lines
+82
to
+84
There was a problem hiding this comment.
When Prompt To Fix With AIThis is a comment left during a code review.
Path: internal/youtube/list.go
Line: 82-84
Comment:
**Request ceiling rejects valid pagination**
When `--all` uses `--page-size 1` for more than 10,000 results, this ceiling rejects a traversal whose tokens are still advancing, causing the command to discard all accumulated output and return an error.
How can I resolve this? If you propose a fix, please make it concise.
Contributor
Author
There was a problem hiding this comment. Intentionally skipping this one. Reaching the ceiling requires 10,000 requests, and every paginated request costs ≥1 unit of the default 10,000-unit daily quota — so the API's own There was a problem hiding this comment. That's a sound argument — the API quota acts as a natural ceiling below the guard for 1-unit endpoints, high-page-size endpoints reject Tip: You can customize Greptile's behavior for this repo with |
||
| result.NextPageToken = "" | ||
| break | ||
| } | ||
| seenTokens[result.NextPageToken] = struct{}{} | ||
| // Backstop for a server that emits distinct tokens forever, which the | ||
| // loop check cannot catch. | ||
| if result.Requests >= MaxListRequests { | ||
| return result, fmt.Errorf("pagination did not terminate after %d requests (the server kept returning a nextPageToken); re-run with --limit to bound the result", MaxListRequests) | ||
| } | ||
| params.Set("pageToken", result.NextPageToken) | ||
| } | ||
| return result, nil | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.