fix(api): handle body read error and cap response size in FormatHTTPError

[canyugs]

Description

Follow-up to #250. The review-feedback commit addressing CodeRabbit's io.ReadAll finding was accidentally pushed to a branch on zeabur/cli instead of the fork branch backing the PR, so #250 merged without it. This PR lands that commit on main.

Change

In util.FormatHTTPError:

• Stop discarding the io.ReadAll error — I/O failures during body read now surface as failed to read response body (status: %d): <err> instead of silently degrading to the bare status-code message.
• Wrap the body reader with io.LimitReader(resp.Body, 1<<20) to cap error-body buffering at 1 MiB, guarding against oversized error pages from proxies.

Addresses the review threads on #250 from CodeRabbit ("Handle the error from io.ReadAll", Major) and Copilot (unbounded read), plus the zeabur-review-agent LimitReader suggestion.

Verification

• go build ./... clean
• go test ./pkg/util/... pass (existing TestFormatHTTPError matrix still green)
• Manual edge-case sweep: structured JSON w/ and w/o code, HTML proxy error, S3 XML error, empty body, unicode message, 2 MB body truncated at 1 MiB

Related issues & labels

• Refs SEI-717, follow-up to fix(api): surface backend error body in upload failures #250

🤖 Generated with Claude Code

---

^{Need help on this PR? Tag /codesmith with what you need. Autofix is disabled.}

Summary by CodeRabbit

• Bug Fixes
  • Enhanced HTTP error handling with safety limits on response body reads and improved error reporting for read failures.

[linear-code]

SEI-717

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: c8afc707-1943-49fd-9ed2-6a4cf365c7ea

📥 Commits

Reviewing files that changed from the base of the PR and between 19b605f and 3147636.

📒 Files selected for processing (1)

• pkg/util/http.go

---

📝 Walkthrough

Walkthrough

FormatHTTPError in pkg/util/http.go is updated to safely read HTTP response bodies. The function now limits body reads to 1 MiB using io.LimitReader and explicitly handles read errors instead of silently ignoring them. When reading fails, it returns a formatted error message containing the action, HTTP status code, and the underlying read error.

Possibly related PRs

• zeabur/cli#250: Prior update to the same FormatHTTPError utility function, introducing the mechanism that this PR enhances with bounded reads and explicit error handling.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically describes the two main changes: handling body read errors and capping response size in FormatHTTPError, which directly match the primary modifications in the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[Copilot]

Pull request overview

This PR updates util.FormatHTTPError to be safer and more informative when formatting errors from non-2xx HTTP responses, addressing prior review feedback from #250 around unbounded reads and discarded read errors.

Changes:

• Cap buffered error-body reads to 1 MiB via io.LimitReader to avoid unbounded memory usage.
• Surface io.ReadAll failures as a wrapped error instead of silently falling back to a status-only message.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[zeabur-review-agent]

PR #252 — Synthesis (Gandalf, Aggregator)

Overall Verdict: APPROVE (4 approve, 1 request-changes — consensus is approve)

Reviewers: Legolas (request changes), Gimli (approve), Sam (approve), Merry (approve), Frodo (approve)

---

Summary of Changes

This PR hardens FormatHTTPError in pkg/util/http.go by:

1. Wrapping resp.Body with io.LimitReader(resp.Body, 1<<20) to cap reads at 1 MiB — preventing memory exhaustion from oversized proxy/WAF error bodies.
2. Returning a wrapped error (with %w) when the body read fails, instead of silently discarding the failure.

All 4 production call sites in upload paths benefit without per-caller changes.

---

Findings (Deduplicated)

🔴 CRITICAL

None.

🟠 MAJOR

• F1 (Legolas): upload.go:161-165 and service.go:457-461 — Reassignment of resp after a deferred resp.Body.Close() violates Defer Simplicity. Use distinct variable names (e.g., prepResp) for subsequent HTTP responses to avoid subtle resource-leak bugs on refactoring.

🟡 NIT

• N1 (Legolas, Gimli, Sam, Merry, Frodo — unanimous): pkg/util/http_test.go lacks test cases for the two new code paths: (a) a reader that returns an error, and (b) a body exceeding 1 MiB. Adding these locks down the new contract as regression tests.

🟢 INFO

• I1 (Legolas, Gimli, Frodo): Praise for io.LimitReader — excellent memory-safety hardening for CLI stability.
• I2 (Frodo): Fix is minimal, matches the scope requested in fix(api): surface backend error body in upload failures #250 review threads, no drift or scope expansion. Hygiene is excellent.
• I3 (Frodo): All call sites confirmed safe — none assumed the old silent-discard behavior.

---

Recommended Labels

• approve
• size/S

---

Synthesized by Gandalf from 5/7 reviews.