Skip to content

fix: honor timeout when a signal is also passed #121

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
sensei-woo wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: honor timeout when a signal is also passed #121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1a33ada
fix(fetch): honor timeout when a signal is also passed
sensei-woo Aug 29, 2026
47e41f6
fix(fetch): release the timeout timer when the request rejects
sensei-woo Aug 29, 2026
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
17 changes: 17 additions & 0 deletions doc/content/docs/timeout-and-retry.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,23 @@ const res = await $fetch("/api/users", {
});
```

The timeout composes with your own `signal` — pass both and whichever aborts
first wins, with the caller's abort reason preserved.

```ts twoslash title="fetch.ts"
import { createFetch } from "@better-fetch/fetch";

const $fetch = createFetch({
baseURL: "http://localhost:3000",
})
const controller = new AbortController();
// ---cut---
const res = await $fetch("/api/users", {
timeout: 10000,
signal: controller.signal,
});
```


## Auto Retry

Expand Down
313 changes: 163 additions & 150 deletions packages/better-fetch/src/fetch.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import type { BetterFetchOption, BetterFetchResponse } from "./types";
import { getURL } from "./url";
import {
detectResponseType,
forwardAbortSignal,
getBody,
getFetch,
getHeaders,
Expand Down Expand Up @@ -39,182 +40,194 @@ export const betterFetch = async <
} = await initializePlugins(url, options);
const fetch = getFetch(opts);
const controller = new AbortController();
const signal = opts.signal ?? controller.signal;
const _url = getURL(__url, opts);
const headers = await getHeaders(opts);
const body = getBody(opts, headers);
const method = getMethod(__url, opts);
// one stable object for the whole request so per-request state keyed on
// its identity survives hooks that return a replacement context
const context = {
...opts,
url: _url,
headers,
body,
method,
signal,
};
/**
* Run all on request hooks
*/
for (const onRequest of hooks.onRequest) {
if (onRequest) {
const res = await onRequest(context);
if (typeof res === "object" && res !== null) {
Object.assign(context, res);
// the request always listens to our own controller so `timeout` stays armed
// even when the caller passes a `signal`; the caller's signal is forwarded
// onto the controller, and whichever aborts first wins.
const releaseSignal = forwardAbortSignal(controller, opts.signal);
const signal = controller.signal;
// cleared in `finally` too: a rejected fetch (caller abort, network error)
// must not leave the timer holding the event loop until the deadline
let clearTimeout = () => {};
try {
const _url = getURL(__url, opts);
const headers = await getHeaders(opts);
const body = getBody(opts, headers);
const method = getMethod(__url, opts);
// one stable object for the whole request so per-request state keyed on
// its identity survives hooks that return a replacement context
const context = {
...opts,
url: _url,
headers,
body,
method,
signal,
};
/**
* Run all on request hooks
*/
for (const onRequest of hooks.onRequest) {
if (onRequest) {
const res = await onRequest(context);
if (typeof res === "object" && res !== null) {
Object.assign(context, res);

@greptile-apps greptile-apps Bot Aug 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Hooks disconnect cancellation controller

When an onRequest hook returns its own signal, Object.assign replaces the internal controller signal before dispatch, but getTimeout still aborts only the internal controller. A hanging fetch then ignores both the configured timeout and the forwarded caller cancellation.

Knowledge Base Used:

Fix in Cursor Fix in Codex Fix in Claude Code

@sensei-woo sensei-woo Aug 29, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

True, but pre-existing: on main the same Object.assign(context, res) already let an onRequest hook replace signal, detaching the timeout the same way. Leaving it out of this PR to keep the diff to the timeout/signal composition; happy to follow up separately if maintainers want hooks prevented from overriding signal.

@greptile-apps greptile-apps Bot Aug 29, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

That’s fair. This behavior predates the PR, and the change does not introduce a new way for hooks to replace context.signal; it only composes the caller signal with the internal controller before hooks run. Since preventing hooks from overriding cancellation is a separate API/behavior decision, I agree it should be handled separately rather than expanding this fix. Resolving this comment.

Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.

}
}
}
}
if (
("pipeTo" in context && typeof context.pipeTo === "function") ||
typeof options?.body?.pipe === "function"
) {
if (!("duplex" in context)) {
context.duplex = "half";
if (
("pipeTo" in context && typeof context.pipeTo === "function") ||
typeof options?.body?.pipe === "function"
) {
if (!("duplex" in context)) {
context.duplex = "half";
}
}
}

const { clearTimeout } = getTimeout(opts, controller);
let response = await fetch(context.url, context);
clearTimeout();
clearTimeout = getTimeout(opts, controller).clearTimeout;
let response = await fetch(context.url, context);
clearTimeout();

const responseContext = {
response,
request: context,
};
const responseContext = {
response,
request: context,
};

for (const onResponse of hooks.onResponse) {
if (onResponse) {
const r = await onResponse({
...responseContext,
response: options?.hookOptions?.cloneResponse
? response.clone()
: response,
});
if (r instanceof Response) {
response = r;
} else if (typeof r === "object" && r !== null) {
response = r.response;
for (const onResponse of hooks.onResponse) {
if (onResponse) {
const r = await onResponse({
...responseContext,
response: options?.hookOptions?.cloneResponse
? response.clone()
: response,
});
if (r instanceof Response) {
response = r;
} else if (typeof r === "object" && r !== null) {
response = r.response;
}
}
}
}

/**
* OK Branch
*/
if (response.ok) {
const hasBody = context.method !== "HEAD";
if (!hasBody) {
/**
* OK Branch
*/
if (response.ok) {
const hasBody = context.method !== "HEAD";
if (!hasBody) {
return {
data: "" as any,
error: null,
} as any;
}
const responseType = detectResponseType(response);
const successContext = {
data: null as any,
response,
request: context,
};
if (responseType === "json" || responseType === "text") {
const text = await response.text();
const parser = context.jsonParser ?? jsonParse;
successContext.data = await parser(text);
} else {
successContext.data = await response[responseType]();
}

/**
* Parse the data if the output schema is defined
*/
if (context?.output) {
if (context.output && !context.disableValidation) {
successContext.data = await parseStandardSchema(
context.output as StandardSchemaV1,
successContext.data,
);
}
}

for (const onSuccess of hooks.onSuccess) {
if (onSuccess) {
await onSuccess({
...successContext,
response: options?.hookOptions?.cloneResponse
? response.clone()
: response,
});
}
}

if (options?.throw) {
return successContext.data;
}

return {
data: "" as any,
data: successContext.data,
error: null,
} as any;
}
const responseType = detectResponseType(response);
const successContext = {
data: null as any,
const parser = options?.jsonParser ?? jsonParse;
const responseText = await response.text();
const isJSONResponse = isJSONParsable(responseText);
const errorObject = isJSONResponse ? await parser(responseText) : null;
/**
* Error Branch
*/
const errorContext = {
response,
responseText,
request: context,
error: {
...errorObject,
status: response.status,
statusText: response.statusText,
},
};
if (responseType === "json" || responseType === "text") {
const text = await response.text();
const parser = context.jsonParser ?? jsonParse;
successContext.data = await parser(text);
} else {
successContext.data = await response[responseType]();
}

/**
* Parse the data if the output schema is defined
*/
if (context?.output) {
if (context.output && !context.disableValidation) {
successContext.data = await parseStandardSchema(
context.output as StandardSchemaV1,
successContext.data,
);
}
}

for (const onSuccess of hooks.onSuccess) {
if (onSuccess) {
await onSuccess({
...successContext,
for (const onError of hooks.onError) {
if (onError) {
await onError({
...errorContext,
response: options?.hookOptions?.cloneResponse
? response.clone()
: response,
});
}
}

if (options?.throw) {
return successContext.data;
}

return {
data: successContext.data,
error: null,
} as any;
}
const parser = options?.jsonParser ?? jsonParse;
const responseText = await response.text();
const isJSONResponse = isJSONParsable(responseText);
const errorObject = isJSONResponse ? await parser(responseText) : null;
/**
* Error Branch
*/
const errorContext = {
response,
responseText,
request: context,
error: {
...errorObject,
status: response.status,
statusText: response.statusText,
},
};
for (const onError of hooks.onError) {
if (onError) {
await onError({
...errorContext,
response: options?.hookOptions?.cloneResponse
? response.clone()
: response,
});
}
}

if (options?.retry) {
const retryStrategy = createRetryStrategy(options.retry);
const _retryAttempt = options.retryAttempt ?? 0;
if (await retryStrategy.shouldAttemptRetry(_retryAttempt, response)) {
for (const onRetry of hooks.onRetry) {
if (onRetry) {
await onRetry(responseContext);
if (options?.retry) {
const retryStrategy = createRetryStrategy(options.retry);
const _retryAttempt = options.retryAttempt ?? 0;
if (await retryStrategy.shouldAttemptRetry(_retryAttempt, response)) {
for (const onRetry of hooks.onRetry) {
if (onRetry) {
await onRetry(responseContext);
}
}
const delay = retryStrategy.getDelay(_retryAttempt);
await new Promise((resolve) => setTimeout(resolve, delay));
return await betterFetch(url, {
...options,
retryAttempt: _retryAttempt + 1,
});
}
const delay = retryStrategy.getDelay(_retryAttempt);
await new Promise((resolve) => setTimeout(resolve, delay));
return await betterFetch(url, {
...options,
retryAttempt: _retryAttempt + 1,
});
}
}

if (options?.throw) {
throw new BetterFetchError(
response.status,
response.statusText,
isJSONResponse ? errorObject : responseText,
);
if (options?.throw) {
throw new BetterFetchError(
response.status,
response.statusText,
isJSONResponse ? errorObject : responseText,
);
}
return {
data: null,
error: {
...errorObject,
status: response.status,
statusText: response.statusText,
},
} as any;
} finally {
clearTimeout();
releaseSignal();
}
return {
data: null,
error: {
...errorObject,
status: response.status,
statusText: response.statusText,
},
} as any;
};
Loading