Azure / Azure/fetch-event-source

`fetchEventSource` doesn't abort when passing in an already aborted signal

Open
#98 0 comments 9 reactions 0 assignees View on GitHub
Dominant language
TypeScript
Stars
2.9k
Forks
196
PR merge metrics
No merged PRs in 30d

Description

Javascript's default `fetch` rejects with an `AbortError` when passing an already aborted signal:

```js
const controller = new AbortController()
controller.abort()
await fetch("https://google.com", { signal: controller.signal })
Uncaught:
DOMException [AbortError]: This operation was aborted
at node:internal/deps/undici/undici:13392:13
at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
at async REPL17:1:33
```

However, when using fetch-event-source it fails to detect that the signal is already aborted:
```js
const controller = new AbortController()
controller.abort()
await fetchEventSource("https://any-website-supporting-sse.com", { signal: controller.signal })
// the call above is never aborted
```
This doesn't abort the fetch.

The reason is that `fetchEventSource` creates a new aborter and subscribes to the original signal's `abort` event to then abort its own controller, but the `abort` event does not trigger retroactively (i.e. if the signal is already aborted). Here's the relevant snippet from the `fetchEventSource` implementation:

```js
let curRequestController: AbortController;

function dispose() {
// ...
curRequestController.abort();
}

inputSignal?.addEventListener('abort', () => {
// this callback is not called if the inputSignal is already aborted!
// hence, curRequestController is not aborted
dispose();
resolve();
});

async function create() {
curRequestController = new AbortController();
try {
const response = await fetch(input, {
...rest,
headers,
signal: curRequestController.signal,
});
// ...
} catch (err) {
if (!curRequestController.signal.aborted) {
// ...
}
}
};
```

I patched it locally as follows:
```js
inputSignal?.addEventListener('abort', () => {
dispose();
// no longer resolving the promise here
// because 1) we should reject instead of resolve when aborted
// and 2) dispose() will abort the curRequestController.signal
// which will cause the ongoing fetch request to throw an AbortError
// which we will catch and then we will reject the promise with the error
});

async function create() {
curRequestController = new AbortController();
// use the input signal if it is already aborted
const sig = inputSignal.aborted ? inputSignal : curRequestController.signal
try {
const response = await fetch(input, {
...rest,
headers,
signal: sig,
});
// ...
} catch (err) {
if (sig.aborted) {
dispose();
reject(err);
} else if (!curRequestController.signal.aborted) {
// ...
}
}
};
```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.