refactor(cli): centralize HTTP status error mapping

[steipete]

Summary

• centralize HTTP status-to-error mapping in one helper
• reuse safe response text helper across request paths
• add timeout coverage for apiRequest path
• assert clearTimeout calls in timeout regression tests

Testing

• bun run lint
• bun run test packages/clawdhub/src/http.test.ts
• bun run test
• bun run build

Greptile Overview

Greptile Summary

Refactored HTTP request error handling by centralizing status-to-error mapping logic into a single throwHttpStatusError helper function and extracting fetchWithTimeout and readResponseTextSafe helpers to eliminate duplication across apiRequest, apiRequestForm, fetchText, and downloadZip.

Key changes:

• Added fetchWithTimeout helper that properly wraps abort signals and ensures clearTimeout is called in a finally block
• Changed timeout abort reason from string 'Timeout' to Error('Timeout') for consistent error handling
• Added readResponseTextSafe to safely extract response text with fallback
• Centralized HTTP status error logic in throwHttpStatusError (429 and 5xx throw retryable Error, others throw AbortError)
• Updated all curl helper functions to use throwHttpStatusError, except fetchTextViaCurl which still contains duplicated logic
• Added comprehensive timeout tests with clearTimeout assertions for apiRequest and fetchText paths

One function (fetchTextViaCurl at lines 298-303) was not refactored and still contains the old duplicated error handling pattern.

Confidence Score: 4/5

• This PR is safe to merge after fixing the missed refactoring in fetchTextViaCurl
• The refactoring successfully consolidates error handling logic and adds proper timeout coverage. However, one function (fetchTextViaCurl) was not updated to use the new centralized helper, leaving inconsistent error handling that defeats the purpose of the refactor. This is a straightforward fix but important for maintaining code consistency.
• Pay attention to packages/clawdhub/src/http.ts line 298-303 where fetchTextViaCurl needs to use throwHttpStatusError

_{Last reviewed commit: f697310}

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated (UTC)
clawhub	Ready	Preview, Comment	Feb 14, 2026 1:38am

[cursor]

PR Summary

Medium Risk
Touches shared HTTP request and retry behavior, so mistakes could change which errors are retried vs treated as terminal across the CLI. The change is mostly refactoring with added tests, but it affects all non-Bun network paths.

Overview
Refactors packages/clawdhub/src/http.ts to centralize HTTP failure handling via throwHttpStatusError (retryable for 429/5xx, non-retryable via AbortError otherwise) and to reuse readResponseTextSafe when building error messages.

Replaces per-call timeout wiring in apiRequest, apiRequestForm, fetchText, and downloadZip with a shared fetchWithTimeout that aborts with new Error('Timeout') and always clearTimeouts; updates e2e timeout abort reason to match. Adds vitest coverage ensuring timeouts surface as Error('Timeout'), trigger retries (3 attempts), and call clearTimeout for both apiRequest and fetchText.

^{Written by Cursor Bugbot for commit f697310. This will update automatically on new commits. Configure here.}

[steipete]

Superseded by #286 (clean branch, same changes).

[greptile-apps]

_{3 files reviewed, 1 comment}

_{Edit Code Review Agent Settings | Greptile}

[greptile-apps]

Additional Comments (1)

packages/clawdhub/src/http.ts
fetchTextViaCurl still has duplicated error handling logic instead of using the centralized throwHttpStatusError helper

  if (status < 200 || status >= 300) {
    throwHttpStatusError(status, body)
  }

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}

Prompt To Fix With AI

This is a comment left during a code review.
Path: packages/clawdhub/src/http.ts
Line: 298:303

Comment:
`fetchTextViaCurl` still has duplicated error handling logic instead of using the centralized `throwHttpStatusError` helper

```suggestion
  if (status < 200 || status >= 300) {
    throwHttpStatusError(status, body)
  }
```

<sub>Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!</sub>

How can I resolve this? If you propose a fix, please make it concise.

[vercel]

Additional Suggestion:

The fetchTextViaCurl function was missed during refactoring and still uses manual HTTP status error handling instead of the centralized throwHttpStatusError helper.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.}

[cursor]

fetchTextViaCurl not refactored to use centralized helper

Low Severity

fetchTextViaCurl still contains the old inline error-handling pattern (if (status === 429 || status >= 500) … throw new AbortError(…)) instead of calling the new throwHttpStatusError helper. Every other curl function (fetchJsonViaCurl, fetchJsonFormViaCurl, fetchBinaryViaCurl) was updated to use the centralized helper, making this the only remaining duplication — which defeats the stated purpose of the refactor and risks divergent bug fixes in the future.

Additional Locations (1)

• packages/clawdhub/src/http.ts#L167-L174