From ea05e8f9b0e9bdbbfd66aecc2abeb0d25cab1f44 Mon Sep 17 00:00:00 2001
From: Andrew Arnott <andrew.arnott@microsoft.com>
Date: Wed, 28 Sep 2022 13:31:53 -0600
Subject: [PATCH] Avoid a subtly async Channel.dispose method

The public `Channel.dispose` method was typed as returning `void`, but the derived `ChannelClass.dispose` method would return a promise. As a result, public clients wouldn't know to `await` the result, and if `dispose` threw an exception, that would result in an unobserved promise rejection.

Since folks are not expected to await, rather than make the base method async, I made the derived method *not* async.
---
 src/nerdbank-streams/src/Channel.ts | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/nerdbank-streams/src/Channel.ts b/src/nerdbank-streams/src/Channel.ts
index 6c78ceb7..79171273 100644
--- a/src/nerdbank-streams/src/Channel.ts
+++ b/src/nerdbank-streams/src/Channel.ts
@@ -56,7 +56,7 @@ export abstract class Channel implements IDisposableObservable {
     /**
      * Closes this channel.
      */
-    public dispose() {
+    public dispose(): void {
         // The interesting stuff is in the derived class.
         this._isDisposed = true;
     }
@@ -247,7 +247,7 @@ export class ChannelClass extends Channel {
         }
     }
 
-    public async dispose() {
+    public dispose(): void {
         if (!this.isDisposed) {
             super.dispose();
 
@@ -262,7 +262,9 @@ export class ChannelClass extends Channel {
             this._duplex.push(null);
 
             this._completion.resolve();
-            await this._multiplexingStream.onChannelDisposed(this);
+
+            // Send the notification, but we can't await the result of this.
+            caught(this._multiplexingStream.onChannelDisposed(this));
         }
     }