Skip to content

fix(cli): propagate paginated response read errors - #3983

Merged
lyingbug merged 1 commit into
Tencent:mainfrom
dvd233:fix/cli-pagination-read-errors
Oct 7, 2026
Merged

lyingbug merged 1 commit into
Tencent:mainfrom
dvd233:fix/cli-pagination-read-errors

Conversation

@dvd233

@dvd233 dvd233 commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Description

api --paginate discards errors from io.ReadAll. An interrupted first response can therefore exit successfully with truncated output. A later interruption can be reported as invalid input, and complete-looking JSON can be accepted despite a failed transfer.

The four-line production fix checks the read error after closing each response body and returns the existing network.error wrapper used by the single-request path. It preserves the underlying cause, keeps stdout empty on failure, and stops fetching pages. Only cli/cmd/api/api.go and its test file change.

The tests add 14 subcases covering real HTTP truncation, bytes returned together with a read error, first/later pages, non-paginated fallback, HTTP error precedence, JSON/text modes, request counts, and immediate exactly-once body closure.

Type of Change

  • 🐛 Bug fix
  • ✨ New feature
  • 💥 Breaking change
  • 📚 Documentation update
  • 🎨 Refactor
  • ⚡ Performance improvement
  • 🧪 Test
  • 🔧 Configuration / Build / CI

Testing

Validated locally on Linux/amd64 with Go 1.26.8, from the independent cli/ module:

  • go test ./cmd/api -run TestAPI_Paginate -count=20
  • go test ./cmd/api -race -count=1
  • go test -count=1 ./...
  • go test ./... -race -count=1
  • go vet ./...
  • go build ./...
  • golangci-lint run --new-from-rev=origin/main ./... (v2.14.0, zero issues)
  • scripts/check-skill-wire-vocab.sh
  • scripts/check-secret-tokens.sh
  • go mod verify, gofmt inspection, and git diff --check

All pass. The identical new tests fail in all 14 subcases on baseline bccb4b151bae403508da77fbb174efc79dc47c1a and pass with the fix. The failures were checked for their specific expected causes, including false success, wrong error classification, and an unwanted third request.

Built-binary checks against a local interrupted HTTP fixture confirm exit 7, network.error, and empty stdout for failed first/later pages in JSON and text modes. Separate agent reviews reran the full CLI checks and tested complete-looking JSON followed by short Content-Length, an unterminated chunked body, and a corrupt gzip checksum. Those transport probes fail on baseline and pass with this fix.

Existing successful merge, server-capped pagination, non-paginated fallback, and non-GET controls remain green. Successful paginated text output has a pre-existing FormatOptions.Emit limitation; this change leaves it unchanged and makes the original read error take precedence when a transfer fails.

Scope: the full default CLI suite includes its acceptance contracts. The root backend, frontend, standalone SDK test suite, live acceptance_e2e suite were not run locally; native macOS/Windows execution was initially left to upstream CI. Fixtures use local HTTP/fake services; no live providers or real credentials were used. No kernel-level network isolation is claimed.

Hosted CI: source-bound validation passed.

Upstream CI subsequently passed on this head: CLI build, race tests and vet on Linux, macOS and Windows, plus CLI golangci-lint.

Checklist

  • Diff whitespace check passes
  • Changed source files are formatted
  • Targeted tests pass
  • Diff-scoped lint passes
  • Full CLI-module checks pass; broader suites are listed above
  • Code reviewed by coding agents, including two independent reviews
  • Added tests covering the change
  • Related documentation updated (not needed for this existing error contract)
  • Breaking changes documented (none)

AI assistance: coding agents prepared the patch, tests, and reviews. No personal human review is claimed.

@dvd233
dvd233 marked this pull request as ready for review October 6, 2026 05:48
Copilot AI balanced review requested due to automatic review settings October 6, 2026 05:48

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@lyingbug
lyingbug merged commit 4ea94af into Tencent:main Oct 7, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants