openclaw/clawhub
1
0
Fork 0
mirror of https://github.com/openclaw/clawhub.git synced 2026-07-22 03:35:29 -04:00
Code Issues 183 Packages Projects Releases 1 Wiki Activity

[PR #285] [CLOSED] refactor(cli): centralize HTTP status error mapping #308

New Issue
Closed
opened 2026-02-15 17:16:48 -05:00 by yindo · 0 comments
yindo commented 2026-02-15 17:16:48 -05:00
Owner
Copy Link
Copy Source

📋 Pull Request Information

Original PR: https://github.com/openclaw/clawhub/pull/285
Author: @steipete
Created: 2/13/2026
Status: ❌ Closed

Base: main ← Head: fix/http-timeout-followups

📝 Commits (5)

  • f500c73 fix(cli): throw Error on timeout aborts
  • 8665d44 fix(http): wrap fetch calls in try-finally to prevent timer leaks
  • d4bc86d Merge remote-tracking branch 'origin/main' into fix/cli-timeout
  • 5476e8a fix(cli): unify timeout abort handling
  • f697310 refactor(cli): centralize HTTP status errors and timeout tests

📊 Changes

3 files changed (+107 additions, -55 deletions)

View changed files

📝 e2e/clawdhub.e2e.test.ts (+1 -1)
📝 packages/clawdhub/src/http.test.ts (+73 -1)
📝 packages/clawdhub/src/http.ts (+33 -53)

📄 Description

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

🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.

## 📋 Pull Request Information **Original PR:** https://github.com/openclaw/clawhub/pull/285 **Author:** [@steipete](https://github.com/steipete) **Created:** 2/13/2026 **Status:** ❌ Closed **Base:** `main` ← **Head:** `fix/http-timeout-followups` --- ### 📝 Commits (5) - [`f500c73`](https://github.com/openclaw/clawhub/commit/f500c733729e3a08a791069e86c86a4952e7b698) fix(cli): throw Error on timeout aborts - [`8665d44`](https://github.com/openclaw/clawhub/commit/8665d447386c3d12de817904466729ba37efdf14) fix(http): wrap fetch calls in try-finally to prevent timer leaks - [`d4bc86d`](https://github.com/openclaw/clawhub/commit/d4bc86d5f4668f5a9bbcacd31ecd868decad5079) Merge remote-tracking branch 'origin/main' into fix/cli-timeout - [`5476e8a`](https://github.com/openclaw/clawhub/commit/5476e8a786642ddba2aa41e18986b29cee53c252) fix(cli): unify timeout abort handling - [`f697310`](https://github.com/openclaw/clawhub/commit/f697310f62ffb4d8c8c3ca250a7cc0575f382a9f) refactor(cli): centralize HTTP status errors and timeout tests ### 📊 Changes **3 files changed** (+107 additions, -55 deletions) <details> <summary>View changed files</summary> 📝 `e2e/clawdhub.e2e.test.ts` (+1 -1) 📝 `packages/clawdhub/src/http.test.ts` (+73 -1) 📝 `packages/clawdhub/src/http.ts` (+33 -53) </details> ### 📄 Description ## 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_comment --> <h2>Greptile Overview</h2> <h3>Greptile Summary</h3> 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. <h3>Confidence Score: 4/5</h3> - 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` <sub>Last reviewed commit: f697310</sub> <!-- greptile_other_comments_section --> <!-- /greptile_comment --> --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>
yindo added the pull-request label 2026-02-15 17:16:48 -05:00
yindo closed this issue 2026-02-15 17:16:48 -05:00
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
bug
pull-request
Mirrored from GitHub Pull Request
No Label pull-request
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
yindo
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: openclaw/clawhub#308
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?