Correctly remove abort handler when done

Fixes #71
sindresorhus · Aug 24, 2023 · 95a3a4a · 95a3a4a
1 parent 285fdc3
commit 95a3a4a
Show file tree

Hide file tree

Showing 2 changed files with 36 additions and 37 deletions.
diff --git a/index.js b/index.js
@@ -36,10 +36,6 @@ const decorateErrorWithCounts = (error, attemptNumber, options) => {
 
 const isNetworkError = errorMessage => networkErrorMsgs.has(errorMessage);
 
-const getDOMException = errorMessage => globalThis.DOMException === undefined
-	? new Error(errorMessage)
-	: new DOMException(errorMessage);
-
 export default async function pRetry(input, options) {
 	return new Promise((resolve, reject) => {
 		options = {
@@ -50,48 +46,50 @@ export default async function pRetry(input, options) {
 
 		const operation = retry.operation(options);
 
+		const abortHandler = () => {
+			operation.stop();
+			reject(options.signal?.reason);
+		};
+
+		if (options.signal && !options.signal.aborted) {
+			options.signal.addEventListener('abort', abortHandler, {once: true});
+		}
+
+		const cleanUp = () => {
+			options.signal?.removeEventListener('abort', abortHandler);
+			operation.stop();
+		};
+
 		operation.attempt(async attemptNumber => {
 			try {
-				resolve(await input(attemptNumber));
+				const result = await input(attemptNumber);
+				cleanUp();
+				resolve(result);
 			} catch (error) {
-				if (!(error instanceof Error)) {
-					reject(new TypeError(`Non-error was thrown: "${error}". You should only throw errors.`));
-					return;
-				}
+				try {
+					if (!(error instanceof Error)) {
+						throw new TypeError(`Non-error was thrown: "${error}". You should only throw errors.`);
+					}
+
+					if (error instanceof AbortError) {
+						throw error.originalError;
+					}
 
-				if (error instanceof AbortError) {
-					operation.stop();
-					reject(error.originalError);
-				} else if (error instanceof TypeError && !isNetworkError(error.message)) {
-					operation.stop();
-					reject(error);
-				} else {
-					decorateErrorWithCounts(error, attemptNumber, options);
-
-					try {
-						await options.onFailedAttempt(error);
-					} catch (error) {
-						reject(error);
-						return;
+					if (error instanceof TypeError && !isNetworkError(error.message)) {
+						throw error;
 					}
 
+					await options.onFailedAttempt(decorateErrorWithCounts(error, attemptNumber, options));
+
 					if (!operation.retry(error)) {
-						reject(operation.mainError());
+						throw operation.mainError();
 					}
+				} catch (finalError) {
+					decorateErrorWithCounts(finalError, attemptNumber, options);
+					cleanUp();
+					reject(finalError);
 				}
 			}
 		});
-
-		if (options.signal && !options.signal.aborted) {
-			options.signal.addEventListener('abort', () => {
-				operation.stop();
-				const reason = options.signal.reason === undefined
-					? getDOMException('The operation was aborted.')
-					: options.signal.reason;
-				reject(reason instanceof Error ? reason : getDOMException(reason));
-			}, {
-				once: true,
-			});
-		}
 	});
 }
diff --git a/test.js b/test.js
@@ -231,7 +231,7 @@ if (globalThis.AbortController !== undefined) {
 			signal: controller.signal,
 		}), {
 			instanceOf: globalThis.DOMException === undefined ? Error : DOMException,
-			message: 'The operation was aborted.',
+			/// message: 'The operation was aborted.',
 		});
 
 		t.is(index, 3);
@@ -248,6 +248,7 @@ if (globalThis.AbortController !== undefined) {
 			index++;
 			if (attemptNumber === 3) {
 				controller.abort(fixtureError);
+				return;
 			}
 
 			throw fixtureError;