Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent reusing the socket after receiving a server_disconnect #647

Merged
merged 6 commits into from
Nov 22, 2019
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const socketReferenceBufferTime = 2000;

interface ISocketReference {
socket: SocketIOClient.Socket | undefined;
socketClosing?: boolean;
anthony-murphy marked this conversation as resolved.
Show resolved Hide resolved
references: number;
delayDeleteTimeout?: NodeJS.Timeout;
delayDeleteTimeoutSetTime?: number;
Expand Down Expand Up @@ -144,17 +145,18 @@ export class OdspDocumentDeltaConnection extends DocumentDeltaConnection impleme
timeout: timeoutMs,
anthony-murphy marked this conversation as resolved.
Show resolved Hide resolved
});

socketReference = {
socket,
references: 1,
};

socket.on("server_disconnect", (socketError: IOdspSocketError) => {
// Raise it as disconnect.
// That produces cleaner telemetry (no errors) and keeps protocol simpler (and not driver-specific).
socketReference!.socketClosing = true;
GaryWilber marked this conversation as resolved.
Show resolved Hide resolved
socket.emit("disconnect", errorObjectFromOdspError(socketError));
});

socketReference = {
socket,
references: 1,
};

OdspDocumentDeltaConnection.socketIoSockets.set(key, socketReference);
debug(`Created new socketio reference for ${key}`);
}
Expand All @@ -180,7 +182,7 @@ export class OdspDocumentDeltaConnection extends DocumentDeltaConnection impleme

debug(`Removed socketio reference for ${key}. Remaining references: ${socketReference.references}.`);

if (isFatalError || (socketReference.socket && !socketReference.socket.connected)) {
if (isFatalError || socketReference.socketClosing || (socketReference.socket && !socketReference.socket.connected)) {
// delete the reference if a fatal error occurred or if the socket is not connected
if (socketReference.socket) {
socketReference.socket.disconnect();
Expand Down Expand Up @@ -214,9 +216,9 @@ export class OdspDocumentDeltaConnection extends DocumentDeltaConnection impleme
* @param socketReferenceKey - socket reference key
*/
constructor(
socket: SocketIOClient.Socket,
documentId: string,
private socketReferenceKey: string | undefined) {
socket: SocketIOClient.Socket,
documentId: string,
private socketReferenceKey: string | undefined) {
super(socket, documentId);
}

Expand All @@ -228,7 +230,7 @@ export class OdspDocumentDeltaConnection extends DocumentDeltaConnection impleme
throw new Error("Invalid socket reference key");
}

OdspDocumentDeltaConnection.removeSocketIoReference(this.socketReferenceKey);
OdspDocumentDeltaConnection.removeSocketIoReference(this.socketReferenceKey, socketProtocolError);
this.socketReferenceKey = undefined;

this.emit("disconnect", "client closing connection");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,12 @@ export class DocumentDeltaConnection extends EventEmitter implements IDocumentDe
reject(createErrorObject("connect_timeout", "Socket connection timed out"));
});

// Listen for disconnects
this.addConnectionListener("disconnect", () => {
this.disconnect(true);
reject(createErrorObject("disconnect", "Socket disconnectd"));
});

this.addConnectionListener("connect_document_success", (response: IConnected) => {
this.removeTrackedListeners(true);
resolve(response);
Expand Down