Merge pull request #49 from berniegp/timeout-fix

Clear timeout timer when a request terminates
berniegp · Jan 18, 2025 · 3891b29 · 3891b29
2 parents 2bd64f1 + ae60646
commit 3891b29
Show file tree

Hide file tree

Showing 4 changed files with 109 additions and 20 deletions.
diff --git a/README.md b/README.md
@@ -188,17 +188,18 @@ it('should produce a success response', async () => {
     request.respond(200, responseHeaders, response);
   };
 
+  const savedXMLHttpRequest = globalThis.XMLHttpRequest;
   try {
     // Install in the global context so "new XMLHttpRequest()" creates MockXhr instances
-    global.XMLHttpRequest = MockXhr;
+    globalThis.XMLHttpRequest = MockXhr;
 
     // Do something that send()s an XMLHttpRequest to '/my/url' and returns a Promise
     // that resolves to the parsed JSON response
     const result = await functionToTest();
     assert.equal(result.message, 'Success!');
   } finally {
     // Restore the original XMLHttpRequest
-    delete global.XMLHttpRequest;
+    globalThis.XMLHttpRequest = savedXMLHttpRequest;
   }
 });
 ```

diff --git a/src/MockXhr.ts b/src/MockXhr.ts
@@ -856,6 +856,7 @@ export default class MockXhr
 
   private _terminateFetchController() {
     delete this._currentRequest;
+    this._clearScheduleTimeout();
   }
 
   private _fireProgressEvent(name: TXhrProgressEventNames, transmitted: number, length: number) {
@@ -877,9 +878,7 @@ export default class MockXhr
 
   private _scheduleRequestTimeout() {
     // Cancel any previous timeout task
-    if (this._timeoutTask) {
-      clearTimeout(this._timeoutTask);
-    }
+    this._clearScheduleTimeout();
 
     if (this._timeout > 0) {
       // The timeout delay must be measured relative to the start of fetching
@@ -889,11 +888,18 @@ export default class MockXhr
         if (this._sendFlag) {
           this._currentRequest?.setRequestTimeout();
         }
-        delete this._timeoutTask;
       }, delay);
     }
   }
 
+  private _clearScheduleTimeout() {
+    if (this._timeoutTask) {
+      clearTimeout(this._timeoutTask);
+    }
+
+    delete this._timeoutTask;
+  }
+
   private _getPrototype() {
     return this.constructor as typeof MockXhr;
   }

diff --git a/test/Factories.test.ts b/test/Factories.test.ts
@@ -5,8 +5,6 @@ import MockXhr from '../src/MockXhr.ts';
 import MockXhrRequest from '../src/MockXhrRequest.ts';
 import { newMockXhr, newServer } from '../src/Factories.ts';
 
-interface Globals { XMLHttpRequest?: typeof XMLHttpRequest }
-
 describe('Factories', () => {
   describe('newMockXhr()', () => {
     describe('Isolation', () => {
@@ -148,11 +146,9 @@ describe('Factories', () => {
     });
 
     it('should work with the low-level quick start code', async () => {
-      const global: Globals = {};
       function functionToTest(): Promise<{ message: string }> {
         return new Promise((resolve, reject) => {
-          // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
-          const xhr = new global.XMLHttpRequest!() as MockXhr;
+          const xhr = new globalThis.XMLHttpRequest();
           xhr.open('GET', '/my/url');
           // eslint-disable-next-line @typescript-eslint/no-unsafe-argument
           xhr.onload = () => { resolve(JSON.parse(xhr.response)); };
@@ -172,17 +168,18 @@ describe('Factories', () => {
         request.respond(200, responseHeaders, response);
       };
 
+      const savedXMLHttpRequest = globalThis.XMLHttpRequest;
       try {
         // Install in the global context so "new XMLHttpRequest()" creates MockXhr instances
-        global.XMLHttpRequest = MockXhr;
+        globalThis.XMLHttpRequest = MockXhr;
 
         // Do something that send()s an XMLHttpRequest to '/my/url' and returns a Promise
         // that resolves to the parsed JSON response
         const result = await functionToTest();
         assert.equal(result.message, 'Success!');
       } finally {
         // Restore the original XMLHttpRequest
-        delete global.XMLHttpRequest;
+        globalThis.XMLHttpRequest = savedXMLHttpRequest;
       }
     });
   });

diff --git a/test/MockXhr.test.ts b/test/MockXhr.test.ts
@@ -331,19 +331,15 @@ describe('MockXhr', () => {
         context.mock.timers.enable();
         const xhr = new MockXhr();
 
-        const onSend = new Promise<MockXhrRequest>((resolve) => {
-          xhr.onSend = (request) => {
-            request.respond();
-            resolve(request);
-          };
-        });
+        const onSend = new Promise<MockXhrRequest>((resolve) => { xhr.onSend = resolve; });
         let timedOut = false;
         xhr.addEventListener('timeout', () => { timedOut = true; });
         xhr.open('GET', '/url');
         xhr.send();
         xhr.timeout = 1;
 
-        await onSend;
+        const request = await onSend;
+        request.respond();
 
         // Move past the timeout
         context.mock.timers.tick(20);
@@ -440,6 +436,95 @@ describe('MockXhr', () => {
           MockXhr.timeoutEnabled = true;
         }
       });
+
+      it('does not leak timer handles after response', async () => {
+        let nextTimerHandle = 1;
+        const activeTimerHandles: number[] = [];
+        const savedSetTimeout = globalThis.setTimeout;
+        const savedClearTimeout = globalThis.clearTimeout;
+        try {
+          const setTimeoutMock = Object.assign(() => {
+            activeTimerHandles.push(nextTimerHandle);
+            return nextTimerHandle++;
+          }, setTimeout);
+          globalThis.setTimeout = setTimeoutMock;
+          globalThis.clearTimeout = (handle) => {
+            activeTimerHandles.splice(activeTimerHandles.indexOf(handle as number));
+          };
+
+          const xhr = new MockXhr();
+
+          const onSend = new Promise<MockXhrRequest>((resolve) => { xhr.onSend = resolve; });
+          xhr.open('GET', '/url');
+          xhr.send();
+          xhr.timeout = 1;
+
+          const request = await onSend;
+          request.respond();
+          assert.strictEqual(activeTimerHandles.length, 0, 'all timers should be cleared');
+        } finally {
+          globalThis.setTimeout = savedSetTimeout;
+          globalThis.clearTimeout = savedClearTimeout;
+        }
+      });
+
+      it('does not leak timer handles after re-open', () => {
+        let nextTimerHandle = 1;
+        const activeTimerHandles: number[] = [];
+        const savedSetTimeout = globalThis.setTimeout;
+        const savedClearTimeout = globalThis.clearTimeout;
+        try {
+          const setTimeoutMock = Object.assign(() => {
+            activeTimerHandles.push(nextTimerHandle);
+            return nextTimerHandle++;
+          }, setTimeout);
+          globalThis.setTimeout = setTimeoutMock;
+          globalThis.clearTimeout = (handle) => {
+            activeTimerHandles.splice(activeTimerHandles.indexOf(handle as number));
+          };
+
+          const xhr = new MockXhr();
+
+          xhr.open('GET', '/url');
+          xhr.send();
+          xhr.timeout = 1;
+          xhr.open('GET', '/url');
+
+          assert.strictEqual(activeTimerHandles.length, 0, 'all timers should be cleared');
+        } finally {
+          globalThis.setTimeout = savedSetTimeout;
+          globalThis.clearTimeout = savedClearTimeout;
+        }
+      });
+
+      it('does not leak timer handles after abort', () => {
+        let nextTimerHandle = 1;
+        const activeTimerHandles: number[] = [];
+        const savedSetTimeout = globalThis.setTimeout;
+        const savedClearTimeout = globalThis.clearTimeout;
+        try {
+          const setTimeoutMock = Object.assign(() => {
+            activeTimerHandles.push(nextTimerHandle);
+            return nextTimerHandle++;
+          }, setTimeout);
+          globalThis.setTimeout = setTimeoutMock;
+          globalThis.clearTimeout = (handle) => {
+            activeTimerHandles.splice(activeTimerHandles.indexOf(handle as number));
+          };
+
+          const xhr = new MockXhr();
+
+          xhr.open('GET', '/url');
+          xhr.send();
+          xhr.timeout = 1;
+          xhr.abort();
+
+          assert.strictEqual(activeTimerHandles.length, 0, 'all timers should be cleared');
+        } finally {
+          globalThis.setTimeout = savedSetTimeout;
+          globalThis.clearTimeout = savedClearTimeout;
+        }
+      });
     });
 
     describe('withCredentials attribute', () => {