Repository: wheresrhys/fetch-mock (js)

Issue:
AbortSignal should abort response body
According to [fetch http-network-fetch](https://fetch.spec.whatwg.org/#http-network-fetch), Step 7, reception of the response body should be aborted if the AbortSignal provided to the original call to `fetch` fires. In [node-fetch](https://github.com/node-fetch/node-fetch), this is implemented [here](https://github.com/node-fetch/node-fetch/blob/master/src/index.js#L59), by destroying the stream. Notably, this happens outside the `Response` object.

Unless I missed it, `fetch-mock` does not do this and the AbortSignal only aborts the initial step of resolving the promise returned by `fetch` with the `Response` object.

I'm happy to provide a PR to resolve this, if the issue is valid. 


Pull request:
fix: abort request and response bodies
Fixes #599 

Needs tests

Patch:
diff --git a/package.json b/package.json
index 73a964b..fc45003 100644
--- a/package.json
+++ b/package.json
@@ -21,7 +21,8 @@
     "docs"
   ],
   "scripts": {
-    "lint": "eslint --cache --fix --ext .js,.cjs",
+    "lint:staged": "eslint --cache --fix --ext .js,.cjs",
+    "lint": "eslint --cache --fix --ext .js,.cjs .",
     "lint:ci": "eslint --ext .js,.cjs .",
     "prettier": "prettier --cache --write *.md \"./**/*.md\"",
     "prettier:ci": "prettier *.md \"./**/*.md\"",
@@ -73,7 +74,7 @@
   },
   "lint-staged": {
     "**/*.js": [
-      "npm run lint"
+      "npm run lint:staged"
     ],
     "packages/**/*.js": [
       "npm run types:check"
diff --git a/packages/core/src/Matchers.js b/packages/core/src/Matchers.js
index b4bff45..665a7bd 100644
--- a/packages/core/src/Matchers.js
+++ b/packages/core/src/Matchers.js
@@ -177,16 +177,10 @@ const getExpressParamsMatcher = ({ params: expectedParams, url }) => {
 const getBodyMatcher = (route) => {
 	const { body: expectedBody } = route;
 
-	if (!expectedBody) {
-		return;
-	}
-
 	return ({ options: { body, method = 'get' } }) => {
-		if (['get', 'head', 'delete'].includes(method.toLowerCase())) {
-			// GET requests don’t send a body so even if it exists in the options
-			// we treat as no body because it would never actually make it to the server
-			// in the application code
-			return false;
+		if (method.toLowerCase() === 'get') {
+			// GET requests don’t send a body so the body matcher should be ignored for them
+			return true;
 		}
 
 		let sentBody;
diff --git a/packages/core/src/Router.js b/packages/core/src/Router.js
index 52b1065..ab22083 100644
--- a/packages/core/src/Router.js
+++ b/packages/core/src/Router.js
@@ -157,7 +157,20 @@ export default class Router {
 					// TODO may need to bring that flushy thing back.
 					// Add a test to combvine flush with abort
 					// done();
-					reject(new DOMException('The operation was aborted.', 'AbortError'));
+					const error = new DOMException(
+						'The operation was aborted.',
+						'AbortError',
+					);
+
+					const requestBody = request?.body || options?.body;
+					if (requestBody instanceof ReadableStream) {
+						requestBody.cancel(error);
+					}
+
+					if (callLog?.response?.body) {
+						callLog.response.body.cancel(error);
+					}
+					reject(error);
 				};
 				if (callLog.signal.aborted) {
 					abort();


Test patch:
diff --git a/packages/core/src/__tests__/FetchMock/response-negotiation.test.js b/packages/core/src/__tests__/FetchMock/response-negotiation.test.js
index a930a3e..0fe7605 100644
--- a/packages/core/src/__tests__/FetchMock/response-negotiation.test.js
+++ b/packages/core/src/__tests__/FetchMock/response-negotiation.test.js
@@ -1,4 +1,4 @@
-import { beforeEach, describe, expect, it } from 'vitest';
+import { beforeEach, describe, expect, it, vi } from 'vitest';
 import fetchMock from '../../FetchMock';
 
 describe('response negotiation', () => {
@@ -161,63 +161,115 @@ describe('response negotiation', () => {
 	});
 
 	describe('abortable fetch', () => {
-		const RESPONSE_DELAY = 50;
-		const ABORT_DELAY = 10;
-
-		const getDelayedOk = () =>
-			new Promise((res) => setTimeout(() => res(200), RESPONSE_DELAY));
-
-		const getDelayedAbortController = () => {
+		const getDelayedAbortController = (delay) => {
 			const controller = new AbortController();
-			setTimeout(() => controller.abort(), ABORT_DELAY);
+			setTimeout(() => controller.abort(), delay);
 			return controller;
 		};
-		const expectAbortError = async (...fetchArgs) => {
-			const result = fm.fetchHandler(...fetchArgs);
-			await expect(result).rejects.toThrowError(
-				new DOMException('The operation was aborted.', 'ABortError'),
-			);
-		};
 
-		it('error on signal abort', () => {
-			fm.route('*', getDelayedOk());
-			return expectAbortError('http://a.com', {
-				signal: getDelayedAbortController().signal,
-			});
+		it('error on signal abort', async () => {
+			fm.route('*', 200, { delay: 50 });
+			await expect(
+				fm.fetchHandler('http://a.com', {
+					signal: getDelayedAbortController(10).signal,
+				}),
+			).rejects.toThrowError(
+				new DOMException('The operation was aborted.', 'AbortError'),
+			);
 		});
 
-		it('error on signal abort for request object', () => {
-			fm.route('*', getDelayedOk());
-			return expectAbortError(
-				new fm.config.Request('http://a.com', {
-					signal: getDelayedAbortController().signal,
-				}),
+		it('error on signal abort for request object', async () => {
+			fm.route('*', 200, { delay: 50 });
+			await expect(
+				fm.fetchHandler(
+					new fm.config.Request('http://a.com', {
+						signal: getDelayedAbortController(10).signal,
+					}),
+				),
+			).rejects.toThrowError(
+				new DOMException('The operation was aborted.', 'AbortError'),
 			);
 		});
 
-		it('error when signal already aborted', () => {
+		it('error when signal already aborted', async () => {
 			fm.route('*', 200);
 			const controller = new AbortController();
 			controller.abort();
-			return expectAbortError('http://a.com', {
-				signal: controller.signal,
+			await expect(
+				fm.fetchHandler('http://a.com', {
+					signal: controller.signal,
+				}),
+			).rejects.toThrowError(
+				new DOMException('The operation was aborted.', 'AbortError'),
+			);
+		});
+
+		it('aborts sending request options body stream', async () => {
+			fm.route('*', 200, { delay: 50 });
+			const body = new ReadableStream();
+			vi.spyOn(body, 'cancel');
+			await expect(
+				fm.fetchHandler('http://a.com', {
+					method: 'post',
+					body,
+					signal: getDelayedAbortController(10).signal,
+				}),
+			).rejects.toThrowError(
+				new DOMException('The operation was aborted.', 'AbortError'),
+			);
+			expect(body.cancel).toHaveBeenCalledWith(
+				new DOMException('The operation was aborted.', 'AbortError'),
+			);
+		});
+
+		// this doesn't work as the callLog creatde from the request awaits the body
+		it.skip('aborts sending request body stream', async () => {
+			fm.route('*', 200, { delay: 50 });
+			const body = new ReadableStream();
+			vi.spyOn(body, 'cancel');
+			const request = new Request('http://a.com', {
+				method: 'post',
+				body,
+				duplex: 'half',
+				signal: getDelayedAbortController(10).signal,
 			});
+			await expect(fm.fetchHandler(request)).rejects.toThrowError(
+				new DOMException('The operation was aborted.', 'AbortError'),
+			);
+			expect(body.cancel).toHaveBeenCalledWith(
+				new DOMException('The operation was aborted.', 'AbortError'),
+			);
+		});
+
+		it.skip('aborts receiving response body stream', async () => {
+			// so fiddly to implement a test for this. Uses the same mechanism as cancelling request body though
+			// so I trust that if one works the other does
 		});
 
 		it('go into `done` state even when aborted', async () => {
-			fm.once('http://a.com', getDelayedOk());
-			await expectAbortError('http://a.com', {
-				signal: getDelayedAbortController().signal,
-			});
+			fm.once('http://a.com', 200, { delay: 50 });
+
+			await expect(
+				fm.fetchHandler('http://a.com', {
+					signal: getDelayedAbortController(10).signal,
+				}),
+			).rejects.toThrowError(
+				new DOMException('The operation was aborted.', 'AbortError'),
+			);
+
 			expect(fm.callHistory.done()).toBe(true);
 		});
 
 		it('will flush even when aborted', async () => {
-			fm.route('http://a.com', getDelayedOk());
+			fm.route('http://a.com', 200, { delay: 50 });
 
-			await expectAbortError('http://a.com', {
-				signal: getDelayedAbortController().signal,
-			});
+			await expect(
+				fm.fetchHandler('http://a.com', {
+					signal: getDelayedAbortController(10).signal,
+				}),
+			).rejects.toThrowError(
+				new DOMException('The operation was aborted.', 'AbortError'),
+			);
 			await fm.callHistory.flush();
 			expect(fm.callHistory.done()).toBe(true);
 		});