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

[PR #283] [MERGED] fix(cli): throw Error for all timeout aborts #310

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/283
Author: @steipete
Created: 2/13/2026
Status: ✅ Merged
Merged: 2/13/2026
Merged by: @steipete

Base: main ← Head: fix/cli-timeout-codex

📝 Commits (4)

  • 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

📊 Changes

3 files changed (+60 additions, -20 deletions)

View changed files

📝 e2e/clawdhub.e2e.test.ts (+1 -1)
📝 packages/clawdhub/src/http.test.ts (+45 -1)
📝 packages/clawdhub/src/http.ts (+14 -18)

📄 Description

Summary

  • switch all Node fetch timeout aborts to Error reasons (including fetchText)
  • refactor timeout setup/cleanup into shared fetchWithTimeout helper
  • add regression test to assert timeout failures are real Error instances and still retry

Context

  • follow-up to #37 where one request path (fetchText) still used string abort reasons after merge

Testing

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

Greptile Overview

Greptile Summary

Unified timeout abort handling across all fetch operations to consistently use Error instances instead of strings. Extracted duplicated timeout setup/cleanup logic into a shared fetchWithTimeout helper function that applies to apiRequest, apiRequestForm, fetchText, and downloadZip. Added regression test confirming timeout failures throw proper Error instances and trigger retries via p-retry.

  • Consolidated four duplicate timeout patterns into single fetchWithTimeout helper (packages/clawdhub/src/http.ts:172-180)
  • Changed abort reason from string 'Timeout' to new Error('Timeout') ensuring consistent error handling
  • Added test coverage for fetchText timeout behavior with retry validation (packages/clawdhub/src/http.test.ts:158-200)
  • Updated e2e test helper to match new Error-based abort pattern (e2e/clawdhub.e2e.test.ts:63)

Confidence Score: 5/5

  • This PR is safe to merge with no identified risks
  • The refactoring is well-tested with comprehensive regression coverage, follows DRY principles by eliminating code duplication, and properly handles timeout cleanup in a finally block. The change from string to Error abort reasons is a minor fix that improves consistency and ensures proper retry behavior with p-retry
  • No files require special attention

Last reviewed commit: 5476e8a

🔄 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/283 **Author:** [@steipete](https://github.com/steipete) **Created:** 2/13/2026 **Status:** ✅ Merged **Merged:** 2/13/2026 **Merged by:** [@steipete](https://github.com/steipete) **Base:** `main` ← **Head:** `fix/cli-timeout-codex` --- ### 📝 Commits (4) - [`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 ### 📊 Changes **3 files changed** (+60 additions, -20 deletions) <details> <summary>View changed files</summary> 📝 `e2e/clawdhub.e2e.test.ts` (+1 -1) 📝 `packages/clawdhub/src/http.test.ts` (+45 -1) 📝 `packages/clawdhub/src/http.ts` (+14 -18) </details> ### 📄 Description ## Summary - switch all Node fetch timeout aborts to `Error` reasons (including `fetchText`) - refactor timeout setup/cleanup into shared `fetchWithTimeout` helper - add regression test to assert timeout failures are real `Error` instances and still retry ## Context - follow-up to #37 where one request path (`fetchText`) still used string abort reasons after merge ## 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> Unified timeout abort handling across all fetch operations to consistently use `Error` instances instead of strings. Extracted duplicated timeout setup/cleanup logic into a shared `fetchWithTimeout` helper function that applies to `apiRequest`, `apiRequestForm`, `fetchText`, and `downloadZip`. Added regression test confirming timeout failures throw proper `Error` instances and trigger retries via p-retry. - Consolidated four duplicate timeout patterns into single `fetchWithTimeout` helper (packages/clawdhub/src/http.ts:172-180) - Changed abort reason from string `'Timeout'` to `new Error('Timeout')` ensuring consistent error handling - Added test coverage for `fetchText` timeout behavior with retry validation (packages/clawdhub/src/http.test.ts:158-200) - Updated e2e test helper to match new Error-based abort pattern (e2e/clawdhub.e2e.test.ts:63) <h3>Confidence Score: 5/5</h3> - This PR is safe to merge with no identified risks - The refactoring is well-tested with comprehensive regression coverage, follows DRY principles by eliminating code duplication, and properly handles timeout cleanup in a finally block. The change from string to Error abort reasons is a minor fix that improves consistency and ensures proper retry behavior with p-retry - No files require special attention <sub>Last reviewed commit: 5476e8a</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#310
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?