enhance(disposablestack): improve error handling

ardatan · Aug 21, 2024 · 8ab228c · 8ab228c
1 parent f5f5fe9
commit 8ab228c
Show file tree

Hide file tree

Showing 5 changed files with 87 additions and 17 deletions.
diff --git a/.changeset/stale-bobcats-develop.md b/.changeset/stale-bobcats-develop.md
@@ -0,0 +1,5 @@
+---
+'@whatwg-node/disposablestack': patch
+---
+
+Improve `disposed` flag and cleanup callbacks on AsyncDisposable on disposeAsync call
diff --git a/packages/disposablestack/src/AsyncDisposableStack.ts b/packages/disposablestack/src/AsyncDisposableStack.ts
@@ -4,7 +4,7 @@ import { isAsyncDisposable, isSyncDisposable, MaybePromise } from './utils.js';
 export class PonyfillAsyncDisposableStack implements AsyncDisposableStack {
   private callbacks: (() => MaybePromise<void>)[] = [];
   get disposed(): boolean {
-    return false;
+    return this.callbacks.length === 0;
   }
 
   use<T extends AsyncDisposable | Disposable | null | undefined>(value: T): T {
@@ -17,12 +17,16 @@ export class PonyfillAsyncDisposableStack implements AsyncDisposableStack {
   }
 
   adopt<T>(value: T, onDisposeAsync: (value: T) => MaybePromise<void>): T {
-    this.callbacks.push(() => onDisposeAsync(value));
+    if (onDisposeAsync) {
+      this.callbacks.push(() => onDisposeAsync(value));
+    }
     return value;
   }
 
   defer(onDisposeAsync: () => MaybePromise<void>): void {
-    this.callbacks.push(onDisposeAsync);
+    if (onDisposeAsync) {
+      this.callbacks.push(onDisposeAsync);
+    }
   }
 
   move(): AsyncDisposableStack {
@@ -36,10 +40,42 @@ export class PonyfillAsyncDisposableStack implements AsyncDisposableStack {
     return this[DisposableSymbols.asyncDispose]();
   }
 
-  async [DisposableSymbols.asyncDispose](): Promise<void> {
-    for (const cb of this.callbacks) {
-      await cb();
+  private _error?: Error;
+
+  private _iterateCallbacks(): MaybePromise<void> {
+    const cb = this.callbacks.pop();
+    if (cb) {
+      try {
+        const res$ = cb();
+        if (res$?.then) {
+          return res$.then(
+            () => this._iterateCallbacks(),
+            error => {
+              this._error = this._error ? new SuppressedError(error, this._error) : error;
+              return this._iterateCallbacks();
+            },
+          );
+        }
+      } catch (error: any) {
+        this._error = this._error ? new SuppressedError(error, this._error) : error;
+      }
+      return this._iterateCallbacks();
+    }
+  }
+
+  [DisposableSymbols.asyncDispose](): Promise<void> {
+    const res$ = this._iterateCallbacks();
+    if (res$?.then) {
+      return res$.then(() => {
+        if (this._error) {
+          throw this._error;
+        }
+      }) as Promise<void>;
+    }
+    if (this._error) {
+      throw this._error;
     }
+    return undefined as any as Promise<void>;
   }
 
   readonly [Symbol.toStringTag]: string = 'AsyncDisposableStack';

diff --git a/packages/disposablestack/src/DisposableStack.ts b/packages/disposablestack/src/DisposableStack.ts
@@ -4,11 +4,7 @@ import { isSyncDisposable } from './utils.js';
 export class PonyfillDisposableStack implements DisposableStack {
   private callbacks: (() => void)[] = [];
   get disposed(): boolean {
-    return false;
-  }
-
-  dispose(): void {
-    return this[DisposableSymbols.dispose]();
+    return this.callbacks.length === 0;
   }
 
   use<T extends Disposable | null | undefined>(value: T): T {
@@ -19,12 +15,16 @@ export class PonyfillDisposableStack implements DisposableStack {
   }
 
   adopt<T>(value: T, onDispose: (value: T) => void): T {
-    this.callbacks.push(() => onDispose(value));
+    if (onDispose) {
+      this.callbacks.push(() => onDispose(value));
+    }
     return value;
   }
 
   defer(onDispose: () => void): void {
-    this.callbacks.push(onDispose);
+    if (onDispose) {
+      this.callbacks.push(onDispose);
+    }
   }
 
   move(): DisposableStack {
@@ -34,11 +34,26 @@ export class PonyfillDisposableStack implements DisposableStack {
     return stack;
   }
 
-  [DisposableSymbols.dispose](): void {
-    for (const cb of this.callbacks) {
-      cb();
+  dispose(): void {
+    return this[DisposableSymbols.dispose]();
+  }
+
+  private _error?: Error;
+
+  private _iterateCallbacks(): void {
+    const cb = this.callbacks.pop();
+    if (cb) {
+      try {
+        cb();
+      } catch (error: any) {
+        this._error = this._error ? new SuppressedError(error, this._error) : error;
+      }
+      return this._iterateCallbacks();
     }
-    this.callbacks = [];
+  }
+
+  [DisposableSymbols.dispose](): void {
+    return this._iterateCallbacks();
   }
 
   readonly [Symbol.toStringTag]: string = 'DisposableStack';

diff --git a/packages/disposablestack/src/SupressedError.ts b/packages/disposablestack/src/SupressedError.ts
@@ -0,0 +1,12 @@
+export class PonyfillSuppressedError extends Error implements SuppressedError {
+  // eslint-disable-next-line n/handle-callback-err
+  constructor(
+    public error: any,
+    public suppressed: any,
+    message?: string,
+  ) {
+    super(message);
+    this.name = 'SuppressedError';
+    Error.captureStackTrace(this, SuppressedError);
+  }
+}
diff --git a/packages/disposablestack/src/index.ts b/packages/disposablestack/src/index.ts
@@ -1,6 +1,8 @@
 import { PonyfillAsyncDisposableStack } from './AsyncDisposableStack.js';
 import { PonyfillDisposableStack } from './DisposableStack.js';
+import { PonyfillSuppressedError } from './SupressedError.js';
 
 export const DisposableStack = globalThis.DisposableStack || PonyfillDisposableStack;
 export const AsyncDisposableStack = globalThis.AsyncDisposableStack || PonyfillAsyncDisposableStack;
+export const SuppressedError = globalThis.SuppressedError || PonyfillSuppressedError;
 export * from './symbols.js';