Skip to content

Commit

Permalink
Avoid a subtly async Channel.dispose method
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
AArnott committed Sep 28, 2022
1 parent df19c89 commit ea05e8f
Showing 1 changed file with 5 additions and 3 deletions.
8 changes: 5 additions & 3 deletions src/nerdbank-streams/src/Channel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -247,7 +247,7 @@ export class ChannelClass extends Channel {
}
}

public async dispose() {
public dispose(): void {
if (!this.isDisposed) {
super.dispose();

Expand All @@ -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));
}
}

Expand Down

0 comments on commit ea05e8f

Please sign in to comment.