Skip to content

Commit

Permalink
Revert "src: fix up WebSocketShard errors (discordjs#3722)"
Browse files Browse the repository at this point in the history 
This reverts commit a1638e0.
  • Loading branch information
@samsamson33
samsamson33 committed Feb 27, 2020
1 parent 9fe42f3 commit 0820ea4
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 85 deletions.
8 changes: 5 additions & 3 deletions src/client/websocket/WebSocketManager.js
View file
Expand Up @@ -18,7 +18,7 @@ const BeforeReadyWhitelist = [
WSEvents.GUILD_MEMBER_REMOVE,
];

const UNRECOVERABLE_CLOSE_CODES = Object.keys(WSCodes).slice(1);
const UNRECOVERABLE_CLOSE_CODES = [4004, 4010, 4011];
const UNRESUMABLE_CLOSE_CODES = [1000, 4006, 4007];

/**
Expand Down Expand Up @@ -235,7 +235,7 @@ class WebSocketManager extends EventEmitter {
this.debug(`Session ID is present, attempting an immediate reconnect...`, shard);
this.reconnect(true);
} else {
shard.destroy({ reset: true, emit: false, log: false });
shard.destroy(undefined, true);
this.reconnect();
}
});
Expand All @@ -245,6 +245,8 @@ class WebSocketManager extends EventEmitter {
});

shard.on(ShardEvents.DESTROYED, () => {
shard._cleanupConnection();

this.debug('Shard was destroyed but no WebSocket connection was present! Reconnecting...', shard);

this.client.emit(Events.SHARD_RECONNECTING, shard.id);
Expand Down Expand Up @@ -340,7 +342,7 @@ class WebSocketManager extends EventEmitter {
this.debug(`Manager was destroyed. Called by:\n${new Error('MANAGER_DESTROYED').stack}`);
this.destroyed = true;
this.shardQueue.clear();
for (const shard of this.shards.values()) shard.destroy({ closeCode: 1000, reset: true, emit: false });
for (const shard of this.shards.values()) shard.destroy(1000, true);
}

/**
Expand Down
118 changes: 38 additions & 80 deletions src/client/websocket/WebSocketShard.js
View file
Expand Up @@ -178,8 +178,7 @@ class WebSocketShard extends EventEmitter {
this.off(ShardEvents.CLOSE, onClose);
this.off(ShardEvents.READY, onReady);
this.off(ShardEvents.RESUMED, onResumed);
this.off(ShardEvents.INVALID_SESSION, onInvalidOrDestroyed);
this.off(ShardEvents.DESTROYED, onInvalidOrDestroyed);
this.off(ShardEvents.INVALID_SESSION, onInvalid);
};

const onReady = () => {
Expand All @@ -197,7 +196,7 @@ class WebSocketShard extends EventEmitter {
reject(event);
};

const onInvalidOrDestroyed = () => {
const onInvalid = () => {
cleanup();
// eslint-disable-next-line prefer-promise-reject-errors
reject();
Expand All @@ -206,8 +205,7 @@ class WebSocketShard extends EventEmitter {
this.once(ShardEvents.READY, onReady);
this.once(ShardEvents.RESUMED, onResumed);
this.once(ShardEvents.CLOSE, onClose);
this.once(ShardEvents.INVALID_SESSION, onInvalidOrDestroyed);
this.once(ShardEvents.DESTROYED, onInvalidOrDestroyed);
this.once(ShardEvents.INVALID_SESSION, onInvalid);

if (this.connection && this.connection.readyState === WebSocket.OPEN) {
this.debug('An open connection was found, attempting an immediate identify.');
Expand All @@ -216,9 +214,10 @@ class WebSocketShard extends EventEmitter {
}

if (this.connection) {
this.debug(`A connection object was found. Cleaning up before continuing.
this.debug(`A connection was found. Cleaning up before continuing.
State: ${CONNECTION_STATE[this.connection.readyState]}`);
this.destroy({ emit: false });
this._cleanupConnection();
this.connection.close(1000);
}

const wsQuery = { v: client.options.ws.version };
Expand All @@ -234,9 +233,9 @@ class WebSocketShard extends EventEmitter {

this.debug(
`[CONNECT]
Gateway : ${gateway}
Version : ${client.options.ws.version}
Encoding : ${WebSocket.encoding}
Gateway: ${gateway}
Version: ${client.options.ws.version}
Encoding: ${WebSocket.encoding}
Compression: ${zlib ? 'zlib-stream' : 'none'}`);

this.status = this.status === Status.DISCONNECTED ? Status.RECONNECTING : Status.CONNECTING;
Expand Down Expand Up @@ -339,13 +338,11 @@ class WebSocketShard extends EventEmitter {

this.debug(`[CLOSE]
Event Code: ${event.code}
Clean : ${event.wasClean}
Reason : ${event.reason || 'No reason received'}`);
Clean: ${event.wasClean}
Reason: ${event.reason || 'No reason received'}`);

this.setHeartbeatTimer(-1);
this.setHelloTimeout(-1);
// If we still have a connection object, clean up its listeners
if (this.connection) this._cleanupConnection();

this.status = Status.DISCONNECTED;

Expand All @@ -365,7 +362,7 @@ class WebSocketShard extends EventEmitter {
*/
onPacket(packet) {
if (!packet) {
this.debug(`Received broken packet: '${packet}'.`);
this.debug(`Received broken packet: ${packet}.`);
return;
}

Expand Down Expand Up @@ -409,7 +406,7 @@ class WebSocketShard extends EventEmitter {
this.identify();
break;
case OPCodes.RECONNECT:
this.destroy({ closeCode: 4000 });
this.connection.close(1001);
break;
case OPCodes.INVALID_SESSION:
this.debug(`[INVALID SESSION] Resumable: ${packet.d}.`);
Expand All @@ -421,7 +418,7 @@ class WebSocketShard extends EventEmitter {
// Reset the sequence
this.sequence = -1;
// Reset the session ID as it's invalid
this.sessionID = undefined;
this.sessionID = null;
// Set the status to reconnecting
this.status = Status.RECONNECTING;
// Finally, emit the INVALID_SESSION event
Expand Down Expand Up @@ -498,7 +495,7 @@ class WebSocketShard extends EventEmitter {
this.debug('Setting a HELLO timeout for 20s.');
this.helloTimeout = this.manager.client.setTimeout(() => {
this.debug('Did not receive HELLO in time. Destroying and connecting again.');
this.destroy({ reset: true, closeCode: 4009 });
this.destroy(4009);
}, 20000);
}

Expand Down Expand Up @@ -538,9 +535,9 @@ class WebSocketShard extends EventEmitter {
`[${tag}] Didn't receive a heartbeat ack last time, assuming zombie connection. Destroying and reconnecting.
Status : ${STATUS_KEYS[this.status]}
Sequence : ${this.sequence}
Connection State: ${this.connection ? CONNECTION_STATE[this.connection.readyState] : 'No Connection??'}`);

this.destroy({ closeCode: 4009, reset: true });
Connection State: ${this.connection ? CONNECTION_STATE[this.connection.readyState] : 'No Connection??'}`
);
this.destroy(4009);
return;
}

Expand Down Expand Up @@ -639,8 +636,8 @@ class WebSocketShard extends EventEmitter {
*/
_send(data) {
if (!this.connection || this.connection.readyState !== WebSocket.OPEN) {
this.debug(`Tried to send packet '${JSON.stringify(data)}' but no WebSocket is available!`);
this.destroy({ close: 4000 });
this.debug(`Tried to send packet ${JSON.stringify(data)} but no WebSocket is available! Resetting the shard...`);
this.destroy(4000);
return;
}

Expand Down Expand Up @@ -673,61 +670,35 @@ class WebSocketShard extends EventEmitter {

/**
* Destroys this shard and closes its WebSocket connection.
* @param {Object} [options={ closeCode: 1000, reset: false, emit: true, log: true }] Options for destroying the shard
* @param {number} [closeCode=1000] The close code to use
* @param {boolean} [cleanup=false] If the shard should attempt a reconnect
* @private
*/
destroy({ closeCode = 1000, reset = false, emit = true, log = true } = {}) {
if (log) {
this.debug(`[DESTROY]
Close Code : ${closeCode}
Reset : ${reset}
Emit DESTROYED: ${emit}`);
}
destroy(closeCode = 1000, cleanup = false) {
this.debug(`Destroying with close code ${closeCode}, attempting a reconnect: ${!cleanup}`);

// Step 0: Remove all timers
this.setHeartbeatTimer(-1);
this.setHelloTimeout(-1);

// Step 1: Close the WebSocket connection, if any, otherwise, emit DESTROYED
if (this.connection) {
// If the connection is currently opened, we will (hopefully) receive close
if (this.connection.readyState === WebSocket.OPEN) {
this.connection.close(closeCode);
} else {
// Connection is not OPEN
this.debug(`WS State: ${CONNECTION_STATE[this.connection.readyState]}`);
// Remove listeners from the connection
this._cleanupConnection();
// Attempt to close the connection just in case
try {
this.connection.close(closeCode);
} catch {
// No-op
}
// Emit the destroyed event if needed
if (emit) this._emitDestroyed();
}
} else if (emit) {
// We requested a destroy, but we had no connection. Emit destroyed
this._emitDestroyed();
// Close the WebSocket connection, if any
if (this.connection && this.connection.readyState === WebSocket.OPEN) {
this.connection.close(closeCode);
} else if (!cleanup) {
/**
* Emitted when a shard is destroyed, but no WebSocket connection was present.
* @private
* @event WebSocketShard#destroyed
*/
this.emit(ShardEvents.DESTROYED);
}

// Step 2: Null the connection object
this.connection = null;

// Step 3: Set the shard status to DISCONNECTED
// Set the shard status
this.status = Status.DISCONNECTED;

// Step 4: Cache the old sequence (use to attempt a resume)
if (this.sequence !== -1) this.closeSequence = this.sequence;

// Step 5: Reset the sequence and session ID if requested
if (reset) {
this.sequence = -1;
this.sessionID = undefined;
}

// Step 6: reset the ratelimit data
// Reset the sequence
this.sequence = -1;
// Reset the ratelimit data
this.ratelimit.remaining = this.ratelimit.total;
this.ratelimit.queue.length = 0;
if (this.ratelimit.timer) {
Expand All @@ -746,19 +717,6 @@ class WebSocketShard extends EventEmitter {
this.connection.onerror =
this.connection.onmessage = null;
}

/**
* Emits the DESTROYED event on the shard
* @private
*/
_emitDestroyed() {
/**
* Emitted when a shard is destroyed, but no WebSocket connection was present.
* @private
* @event WebSocketShard#destroyed
*/
this.emit(ShardEvents.DESTROYED);
}
}

module.exports = WebSocketShard;
3 changes: 1 addition & 2 deletions typings/index.d.ts
View file
Expand Up @@ -1727,9 +1727,8 @@ declare module 'discord.js' {
private identifyResume(): void;
private _send(data: object): void;
private processQueue(): void;
private destroy(destroyOptions?: { closeCode?: number; reset?: boolean; emit?: boolean; log?: boolean }): void;
private destroy(closeCode: number): void;
private _cleanupConnection(): void;
private _emitDestroyed(): void;

public send(data: object): void;
public on(event: 'ready', listener: () => void): this;
Expand Down

0 comments on commit 0820ea4

Please sign in to comment.