fix(api): validate HTTP status and add image download timeout

[chris-anstey-runware]

Summary

• Surface non-2xx HTTP responses as errors in internal/api/client.go, even when the body isn't a parseable APIResponse. API-level error objects still take precedence, so existing typed errors (e.g. ErrUnauthorized) keep flowing.
• Adds a unified download function for use by all inference results.
• Adds sensible timeouts for audio, video and image inference result downloads.
• Add internal/api/client_test.go covering 5xx without errors body, 4xx with errors body, and 401 invalidApiKey.

Test plan

• go test ./...
• make lint
• Manually trigger an HTTP 5xx (e.g. wrong base URL) and confirm error message includes the status code

🤖 Generated with Claude Code

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. To trigger a review, include review in the PR description. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 14b214c4-3a4d-4310-a8fd-91065478288c

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch fix/http-status-and-download-timeout

---

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

[coderabbitai]

🧹 Nitpick comments (1)

internal/api/client_test.go (1)

41-43: ⚡ Quick win

Strengthen the 400 test to prove API-error precedence (not just message presence).

At Line 41, strings.Contains(err.Error(), "bad input") can still pass if behavior regresses to the fallback API returned HTTP 400: ... path, because that string may include "bad input" too.

Suggested assertion tightening

  if !strings.Contains(err.Error(), "bad input") {
  	t.Errorf("expected error to surface API error message, got: %v", err)
  }
+ if strings.Contains(err.Error(), "API returned HTTP 400") {
+ 	t.Fatalf("expected API error precedence over HTTP fallback, got: %v", err)
+ }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/api/client_test.go` around lines 41 - 43, The current test only
checks strings.Contains(err.Error(), "bad input") which could still pass if the
error is the fallback "API returned HTTP 400: ..."; update the assertion on err
to prove the API's error string takes precedence by (1) asserting err.Error()
does not contain the fallback prefix "API returned HTTP 400" and (2) asserting
the error message exactly equals (or at least starts/contains only) the
API-provided message "bad input" instead of relying on a loose contains check;
locate the check using the err variable and the existing strings.Contains call
and replace it with the two stronger assertions.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@internal/api/client_test.go`:
- Around line 41-43: The current test only checks strings.Contains(err.Error(),
"bad input") which could still pass if the error is the fallback "API returned
HTTP 400: ..."; update the assertion on err to prove the API's error string
takes precedence by (1) asserting err.Error() does not contain the fallback
prefix "API returned HTTP 400" and (2) asserting the error message exactly
equals (or at least starts/contains only) the API-provided message "bad input"
instead of relying on a loose contains check; locate the check using the err
variable and the existing strings.Contains call and replace it with the two
stronger assertions.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 245920c3-1427-41ec-b12d-84ffb5bf7153

📥 Commits

Reviewing files that changed from the base of the PR and between 2648925 and dda438a.

📒 Files selected for processing (3)

• cmd/image_inference.go
• internal/api/client.go
• internal/api/client_test.go

[Copilot]

Pull request overview

This PR improves CLI robustness by (a) surfacing HTTP-level failures from the Runware API client and (b) introducing timeout controls for API calls and result downloads, plus adding API client tests.

Changes:

• Update internal/api client error handling to prioritize API errors[] but also surface non-2xx HTTP responses when the body can’t be parsed as an APIResponse.
• Add --request-timeout (persistent) and wire command contexts through API calls; add --download-timeout for image result downloads.
• Add internal/api/client_test.go covering several error-response scenarios.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
internal/api/client.go	Adjust API response parsing / error surfacing; add response-body truncation helper.
internal/api/client_test.go	Add tests for non-2xx responses with/without API error bodies and unauthorized behavior.
cmd/root.go	Add --request-timeout and contextWithTimeout helper used across commands.
cmd/common.go	Define default request/download timeout constants.
cmd/image_inference.go	Use timeout-derived contexts for API calls; add per-image download timeout wrapper.
cmd/video_inference.go	Switch API calls/polling and downloads to use the timeout-derived context.
cmd/audio_inference.go	Switch API calls/polling and downloads to use the timeout-derived context.
cmd/text_inference.go	Use timeout-derived context for API calls.
cmd/ping.go	Use timeout-derived context for API calls.
cmd/model_search.go	Use timeout-derived context for API calls.
cmd/auth.go	Use timeout-derived context for API calls (key validation/status).
cmd/account.go	Use timeout-derived context for API calls (account details).

Comments suppressed due to low confidence (1)

cmd/image_inference.go:363

• This still uses http.DefaultClient for downloads, and the only timeout comes from the request context. The PR description mentions replacing http.DefaultClient with a timeout-bound client; either implement a dedicated http.Client{Timeout: ...} here (and decide whether it’s per-image or global), or update the PR description to match the context-based timeout approach.

	req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
	if err != nil {
		return fmt.Errorf("failed to create request: %w", err)
	}
	resp, err := http.DefaultClient.Do(req)
	if err != nil {

---

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

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 7 comments.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.