feat: convert tests to vitest, address timer/promise leaks

.circleci/config.yml

@@ -2,6 +2,10 @@ version: 2.1
 
 commands:
   test-nodejs:
+    parameters:
+      vitest-version:
+        type: string
+        default: ''
     steps:
       - run:
           name: Versions
@@ -10,6 +14,12 @@ commands:
       - run:
           name: Install dependencies
           command: npm install
+      - when:
+          condition: << parameters.vitest-version >>
+          steps:
+            - run:
+                name: Install vitest << parameters.vitest-version >>
+                command: npm install vitest@<< parameters.vitest-version >>
       - run:
           name: Build
           command: npm run build
@@ -20,32 +30,27 @@ commands:
           name: Test
           command: npm run test
 jobs:
-  node-v8:
+  node-v16:
     docker:
-      - image: node:8
+      - image: node:16
     steps:
-      - test-nodejs
-  node-v10:
-    docker:
-      - image: node:10
-    steps:
-      - test-nodejs
-  node-v12:
+      - test-nodejs:
+          vitest-version: '^0.34'
+  node-v20:
     docker:
-      - image: node:12
+      - image: node:20
     steps:
       - test-nodejs
-  node-v13:
+  node-v24:
     docker:
-      - image: node:13
+      - image: node:24
     steps:
       - test-nodejs
 
 workflows:
   version: 2
   node-multi-build:
     jobs:
-      - node-v8
-      - node-v10
-      - node-v12
-      - node-v13
+      - node-v16
+      - node-v20
+      - node-v24

.github/workflows/main.yml

@@ -4,9 +4,9 @@ name: CI
 on:
   # Triggers the workflow on push or pull request events but only for the master branch
   push:
-    branches: [ master ]
+    branches: [master]
   pull_request:
-    branches: [ master ]
+    branches: [master]
 
   # Allows you to run this workflow manually from the Actions tab
   workflow_dispatch:
@@ -16,7 +16,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        node-version: [8.x, 10.x, 12.x, 14.x]
+        node-version: [16.x, 20.x, 24.x]
     steps:
       # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it
       - uses: actions/checkout@v2
@@ -27,6 +27,9 @@ jobs:
       - name: Build and test
         run: |
           npm install
+          if [[ "${{ matrix.node-version }}" == "16.x" ]]; then
+            npm i vitest@^0.34
+          fi
           npm run build
           npm run lint
           npm run test

.gitignore

@@ -5,3 +5,5 @@ testCoverage
 *.DS_Store
 profiling
 dist
+yarn.lock
+package-lock.json

package.json

@@ -6,8 +6,10 @@
   "types": "dist/tarn.d.ts",
   "license": "MIT",
   "scripts": {
-    "test": "mocha --slow 10 --timeout 5000 --reporter spec tests.js",
-    "test-bail": "mocha --slow 10 --timeout 5000 --reporter spec --bail tests.js",
+    "pretest": "npm run build",
+    "test": "vitest run",
+    "test-bail": "vitest run --bail 1",
+    "prebuild": "rm -rf dist",
     "build": "tsc",
     "clean": "rm -rf dist",
     "prepublishOnly": "tsc",
@@ -48,20 +50,17 @@
     ]
   },
   "devDependencies": {
+    "@types/node": "^20",
     "@typescript-eslint/eslint-plugin": "^2.21.0",
     "@typescript-eslint/parser": "^2.21.0",
     "bluebird": "^3.7.2",
     "eslint": "^6.8.0",
     "eslint-config-prettier": "^6.10.0",
     "eslint-plugin-prettier": "^3.1.2",
-    "expect.js": "^0.3.1",
     "husky": "^1.3.1",
     "lint-staged": "^9.5.0",
-    "mocha": "^7.1.0",
     "prettier": "^1.19.1",
-    "typescript": "3.8.3"
-  },
-  "dependencies": {
-    "@types/node": "^10.17.17"
+    "typescript": "^5.9.3",
+    "vitest": "^4.1.2"
   }
 }

src/PendingOperation.ts

@@ -21,6 +21,9 @@ export class PendingOperation<T> {
         }
       }
 
+      // Settle the deferred to prevent leaking the timeout wrapper's internal handlers.
+      // The wrapper promise is already rejected so this only settles the inner chain.
+      this.deferred.reject(err);
       this.isRejected = true;
       return Promise.reject(err);
     });

src/Pool.ts

@@ -1,6 +1,6 @@
 import { PendingOperation } from './PendingOperation';
 import { Resource } from './Resource';
-import { checkOptionalTime, delay, duration, now, reflect, tryPromise } from './utils';
+import { checkOptionalTime, duration, now, reflect, tryPromise } from './utils';
 import { EventEmitter } from 'events';
 import { clearInterval } from 'timers';
 
@@ -29,7 +29,8 @@ export class Pool<T> {
   protected pendingAcquires: PendingOperation<T>[];
   protected pendingDestroys: PendingOperation<T>[];
   protected pendingValidations: PendingOperation<T>[];
-  protected interval: NodeJS.Timer | null;
+  protected pendingRetryTimers: ReturnType<typeof setTimeout>[];
+  protected interval: ReturnType<typeof setInterval> | null;
   protected destroyed = false;
   protected propagateCreateError: boolean;
   protected idleTimeoutMillis: number;
@@ -154,6 +155,7 @@ export class Pool<T> {
     this.pendingCreates = [];
     this.pendingAcquires = [];
     this.pendingDestroys = [];
+    this.pendingRetryTimers = [];
 
     // When acquire is pending, but also still in validation phase
     this.pendingValidations = [];
@@ -254,6 +256,7 @@ export class Pool<T> {
         numDestroyed < maxDestroy
       ) {
         numDestroyed++;
+        free.settle();
         this._destroy(free.resource);
       } else {
         newFree.push(free);
@@ -275,13 +278,15 @@ export class Pool<T> {
 
     this._stopReaping();
     this.destroyed = true;
+    this.pendingRetryTimers.forEach(timer => clearTimeout(timer));
+    this.pendingRetryTimers = [];
 
     // First wait for all the pending creates get ready.
     return reflect(
       Promise.all(this.pendingCreates.map(create => reflect(create.promise)))
         .then(() => {
           // eslint-disable-next-line
-          return new Promise((resolve, reject) => {
+          return new Promise<void>((resolve, reject) => {
             // poll every 100ms and wait that all validations are ready
             if (this.numPendingValidations() === 0) {
               resolve();
@@ -309,6 +314,8 @@ export class Pool<T> {
           );
         })
         .then(() => {
+          // Settle free resource deferreds to prevent promise leaks.
+          this.free.forEach(free => free.settle());
           // Now we can destroy all the freed resources.
           return Promise.all(this.free.map(free => reflect(this._destroy(free.resource))));
         })
@@ -435,6 +442,7 @@ export class Pool<T> {
               remove(this.used, free);
               // Only destroy the resource if the validation has failed
               if (!validationSuccess) {
+                free.settle();
                 this._destroy(free.resource);
 
                 // Since we destroyed an invalid resource and were not able to fulfill
@@ -501,6 +509,10 @@ export class Pool<T> {
         return null;
       })
       .catch(err => {
+        if (this.destroyed) {
+          return;
+        }
+
         if (this.propagateCreateError && this.pendingAcquires.length !== 0) {
           // If propagateCreateError is true, we don't retry the create
           // but reject the first pending acquire immediately. Intentionally
@@ -517,7 +529,11 @@ export class Pool<T> {
         });
 
         // Not returned on purpose.
-        delay(this.createRetryIntervalMillis).then(() => this._tryAcquireOrCreate());
+        const retryTimer = setTimeout(() => {
+          remove(this.pendingRetryTimers, retryTimer);
+          this._tryAcquireOrCreate();
+        }, this.createRetryIntervalMillis);
+        this.pendingRetryTimers.push(retryTimer);
       });
   }
 
@@ -543,6 +559,7 @@ export class Pool<T> {
       .then(resource => {
         if (pendingCreate.isRejected) {
           this.destroyer(resource);
+          pendingCreate.resolve(resource);
           return null;
         }
 
@@ -556,6 +573,7 @@ export class Pool<T> {
       })
       .catch(err => {
         if (pendingCreate.isRejected) {
+          pendingCreate.reject(err);
           return null;
         }
 
@@ -633,7 +651,10 @@ export class Pool<T> {
       } catch (err) {
         // There's nothing we can do here but log the error. This would otherwise
         // leak out as an unhandled exception.
-        this.log(`Tarn: event handler "${eventName}" threw an exception ${err.stack}`, 'warn');
+        this.log(
+          `Tarn: event handler "${eventName}" threw an exception ${(err as Error).stack}`,
+          'warn'
+        );
       }
     });
   }

src/Resource.ts

@@ -18,4 +18,8 @@ export class Resource<T> {
     this.deferred.resolve(undefined);
     return new Resource(this.resource);
   }
+
+  settle() {
+    this.deferred.resolve(undefined);
+  }
 }