Skip to content

Commit

Permalink
Don't proceed to close socket if connection is already disposed in od…
Browse files Browse the repository at this point in the history
…sp (#14021)

## Description

Don't proceed to close socket if connection is already disposed in odsp.
This is rare conditions only due to complexity around socket
events/tasks scheduling.
  • Loading branch information
jatgarg authored Feb 6, 2023
1 parent 0555ccb commit 184913b
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 3 deletions.
3 changes: 2 additions & 1 deletion api-report/driver-base.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,8 @@ export class DocumentDeltaConnection extends EventEmitterWithErrorHandling<IDocu
checkpointSequenceNumber: number | undefined;
get claims(): ITokenClaims;
get clientId(): string;
protected closeSocket(error: IAnyDriverError): void;
// (undocumented)
protected closeSocketCore(error: IAnyDriverError): void;
protected createErrorObject(handler: string, error?: any, canRetry?: boolean): IAnyDriverError;
// (undocumented)
get details(): IConnected;
Expand Down
22 changes: 21 additions & 1 deletion packages/drivers/driver-base/src/documentDeltaConnection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -318,7 +318,27 @@ export class DocumentDeltaConnection
/**
* Disconnect from the websocket and close the websocket too.
*/
protected closeSocket(error: IAnyDriverError) {
private closeSocket(error: IAnyDriverError) {
if (this._disposed) {
// This would be rare situation due to complexity around socket emitting events.
this.logger.sendTelemetryEvent(
{
eventName: "SocketCloseOnDisposedConnection",
driverVersion,
details: JSON.stringify({
disposed: this._disposed,
socketConnected: this.socket?.connected,
trackedListenerCount: this.trackedListeners.size,
}),
},
error,
);
return;
}
this.closeSocketCore(error);
}

protected closeSocketCore(error: IAnyDriverError) {
this.disconnect(error);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ export class OdspDocumentDeltaConnection extends DocumentDeltaConnection {
* Critical path where we need to also close the socket for an error.
* @param error - Error causing the socket to close.
*/
protected closeSocket(error: IAnyDriverError) {
protected closeSocketCore(error: IAnyDriverError) {
const socket = this.socketReference;
assert(socket !== undefined, 0x416 /* reentrancy not supported in close socket */);
socket.closeSocket(error);
Expand Down

0 comments on commit 184913b

Please sign in to comment.