Skip to content

fix: increase upload timeout and wrap non-Error abort rejections #534

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
taylorhou wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: increase upload timeout and wrap non-Error abort rejections #534

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 23 additions & 8 deletions packages/clawdhub/src/http.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import { ApiRoutes, parseArk } from './schema/index.js'

const REQUEST_TIMEOUT_MS = 15_000
const REQUEST_TIMEOUT_SECONDS = Math.ceil(REQUEST_TIMEOUT_MS / 1000)
const UPLOAD_TIMEOUT_MS = 120_000
const UPLOAD_TIMEOUT_SECONDS = Math.ceil(UPLOAD_TIMEOUT_MS / 1000)
const RETRY_COUNT = 2
const RETRY_BACKOFF_BASE_MS = 300
const RETRY_BACKOFF_MAX_MS = 5_000
Expand Down Expand Up @@ -143,11 +145,11 @@ export async function apiRequestForm<T>(

const headers: Record<string, string> = { Accept: 'application/json' }
if (args.token) headers.Authorization = `Bearer ${args.token}`
const response = await fetchWithTimeout(url, {
method: args.method,
headers,
body: args.form,
})
const response = await fetchWithTimeout(
url,
{ method: args.method, headers, body: args.form },
UPLOAD_TIMEOUT_MS,
)
if (!response.ok) {
throwHttpStatusError(response.status, await readResponseTextSafe(response), response.headers)
}
Expand Down Expand Up @@ -205,11 +207,24 @@ export async function downloadZip(
)
}

async function fetchWithTimeout(url: string, init: RequestInit): Promise<Response> {
async function fetchWithTimeout(
url: string,
init: RequestInit,
timeoutMs: number = REQUEST_TIMEOUT_MS,
): Promise<Response> {
const controller = new AbortController()
const timeout = setTimeout(() => controller.abort(new Error('Timeout')), REQUEST_TIMEOUT_MS)
const timeout = setTimeout(() => {
// Ensure we always throw a proper Error instance. Some Node/undici versions
// propagate the abort reason as-is; if it is not an Error, pRetry will
// surface "Non-error was thrown" instead of a meaningful message.
controller.abort(new Error(`Request timed out after ${timeoutMs}ms`))
Copy link
Contributor

@greptile-apps greptile-apps bot Feb 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated timeout error message will break existing tests in http.test.ts (lines 264 and 346) that expect .message to be exactly "Timeout". Update test assertions to match the new format:

Suggested change
controller.abort(new Error(`Request timed out after ${timeoutMs}ms`))
controller.abort(new Error(`Request timed out after ${timeoutMs}ms`))

Then update http.test.ts:

// Line 264
expect((caught as Error).message).toContain('timed out')

// Line 346  
expect((caught as Error).message).toContain('timed out')
Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/clawdhub/src/http.ts
Line: 220

Comment:
Updated timeout error message will break existing tests in `http.test.ts` (lines 264 and 346) that expect `.message` to be exactly `"Timeout"`. Update test assertions to match the new format:

```suggestion
    controller.abort(new Error(`Request timed out after ${timeoutMs}ms`))
```

Then update `http.test.ts`:
```typescript
// Line 264
expect((caught as Error).message).toContain('timed out')

// Line 346  
expect((caught as Error).message).toContain('timed out')
```

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

}, timeoutMs)
try {
return await fetch(url, { ...init, signal: controller.signal })
} catch (err) {
// Re-wrap non-Error rejections so pRetry always receives an Error instance.
if (err instanceof Error) throw err
throw new Error(String(err))
} finally {
clearTimeout(timeout)
}
Expand Down Expand Up @@ -425,7 +440,7 @@ async function fetchJsonFormViaCurl(url: string, args: FormRequestArgs) {
'--show-error',
'--location',
'--max-time',
String(REQUEST_TIMEOUT_SECONDS),
String(UPLOAD_TIMEOUT_SECONDS),
'--write-out',
CURL_WRITE_OUT_FORMAT,
'-X',
Expand Down