🐛 Fix AbortAddon ignoring user-provided signals

[js62789]

Fixes a bug where the AbortAddon would ignore user-provided abort signals when users initialize Wretch with a signal or use the .signal() helper method.

Problem

When users provided their own AbortController signal (either via .options({ signal }) or .signal(controller)), the AbortAddon would replace it with its own signal instead of respecting both. This meant that:

1. User-provided signals couldn't abort requests when using the addon
2. The addon's timeout and controller features wouldn't work alongside custom abort logic

Solution

Modified the beforeRequest hook in the AbortAddon to use AbortSignal.any() to merge the addon's controller signal with any existing user-provided signal. Now both signals can independently abort the request.

Changes:

• Updated src/addons/abort.ts to detect existing signals and merge them using AbortSignal.any()
• Added comprehensive test coverage in test/shared/wretch.spec.ts to verify:
  • User signal via .signal() method can abort requests
  • Addon controller can abort when user signal is present
  • User signal via .options({ signal }) can abort requests

[elbywan]

Hey @js62789, thanks for the PR.

When users provided their own AbortController signal (either via .options({ signal }) or .signal(controller)), the AbortAddon would replace it with its own signal instead of respecting both.

This seems wrong? The code appear to do the exact opposite.

• https://github.com/elbywan/wretch/blob/master/src/addons/abort.ts#L122

if (!options["signal"]) { 
  options["signal"] = fetchController.signal
}

Updated src/addons/abort.ts to detect existing signals and merge them using AbortSignal.any()

AbortSignal.any() is still a new thing that is not supported by older browsers (baseline 2024). I don't want to break compatibility.

[js62789]

You’re right, I misspoke earlier. What I meant was something more along the lines of:

When users provide their own AbortController signal (either via .options({ signal }) or .signal(controller)), the AbortAddon leaves that signal untouched and instead aborts an isolated AbortController instance that has no impact on the request.

I still believe this is a significant bug because it prevents callers from reliably cancelling requests using their own signal.

I understand that AbortSignal.any may not yet be widely supported enough for you to want to depend on it directly. Would you be open to an approach that checks for support first, e.g.:

if (existingSignal && "any" in AbortSignal) {
  // use AbortSignal.any(existingSignal, addonSignal)
}

If this direction sounds acceptable, I’m happy to update the PR (or open a new one) with tests and any adjustments you’d like to see. Our team does need to address this behavior, and I’d much rather contribute an upstream fix that works for other users than maintain a fork.

[elbywan]

I still believe this is a significant bug because it prevents callers from reliably cancelling requests using their own signal.

I'll need you to provide a code example that demonstrate this claim, because I don't think that this is true.

If you are referring to setTimeout you need to add pass the custom controller using the { controller } option.

See the docs:

// If you use a custom AbortController associated with the request, pass it in the options object.
wretch("...").addon(AbortAddon()).get().setTimeout(1000, { controller }).json()

[js62789]

You’re right that { controller } works when the caller has an AbortController. The case I’m calling out is when the caller only has an AbortSignal from a framework and no controller to pass.

For example, in Remix v2 loaders you get a Request with a signal, but not the underlying controller:

export async function loader({ request }: LoaderFunctionArgs) {
  return wretch("https://example.com/api")
    .addon(AbortAddon())
    // must use the framework-provided signal so navigation aborts work
    .options({ signal: request.signal })
    .get()
    .setTimeout(1000) // <- this timeout never aborts the request
    .json();
}

In this scenario:

• request.signal does abort the fetch when Remix cancels the request.
• But AbortAddon’s internal fetchController is never wired into options.signal (because it’s already set), so .setTimeout() only aborts that internal controller, not the actual request.

Because Remix (and similar environments) don’t expose an AbortController, there’s no value I can pass into { controller } to make .setTimeout() and request.signal work together. That’s the gap my change is trying to close, with backwards compatibility handled as mentioned earlier.

[elbywan]

Thanks for the example, it's clearer now. 🙇

Since you are working on an environment where it is fine to rely on "newer" APIs like AbortSignal.any, what prevents you from using AbortSignal.timeout(time) in that case?

If this direction sounds acceptable, I’m happy to update the PR (or open a new one) with tests and any adjustments you’d like to see.

// Sounds good to me 👍 
if (existingSignal && typeof AbortSignal !== "undefined && "any" in AbortSignal) {
  // use AbortSignal.any(existingSignal, addonSignal)
}

But in any case, yes adding a guard sounds like a reasonable approach! I just reopened the PR.

[js62789]

Good question — I see where you’re coming from.

AbortSignal.timeout() gives me another AbortSignal, but it doesn’t remove the core problem this PR is trying to solve: wretch only accepts a single options.signal, while we have multiple sources of cancellation:

• the framework’s request.signal (Remix),
• the AbortAddon’s own controller (for .setTimeout() / .controller()),
• and potentially a timeout signal from AbortSignal.timeout().

Even if I used:

const timeoutSignal = AbortSignal.timeout(1000);

I’d still need to merge:

• request.signal,
• the AbortAddon’s controller signal,
• and optionally timeoutSignal,

into the one signal passed to wretch. That merging is exactly what AbortSignal.any(...) (or an equivalent event-bridge implementation) is doing in this PR.

So AbortSignal.timeout() is great for creating a timeout signal, but it doesn’t remove the need for the “fan-in”/merge logic that this change is adding — we still have to support multiple signals, and then present them to wretch as a single combined signal.

[js62789]

It's worth mentioning that we could solve this for ourselves by creating our own AbortAddon variant internally, but that means other users in similar “signal-only” environments wouldn’t benefit, and our fork would likely drift from upstream over time. That’s why I’m investing in getting this behavior right in the addon itself instead of maintaining a separate copy.

[elbywan]

Thanks for the changes, I'll try to review as soon as I have some free time this week.

Good question — I see where you’re coming from.

AbortSignal.timeout() gives me another AbortSignal, but it doesn’t remove the core problem this PR is trying to solve: wretch only accepts a single options.signal, while we have multiple sources of cancellation:
…
(nonsense, you wouldn't be using the abort plugin at all since it only provides setTimeout to abort - and onAbort would work)
…
I’d still need to merge:
…
(more nonsense, same reason as above)
…

In addition, I'm sorry to have to write that but please stop feeding everything I write into your AI agent and copy pasting the answer. Its replies are clearly biased and miss up the point half of the time. It is frustrating.