From e9fa8a438ca586436ff3332deb08338cc0e636f6 Mon Sep 17 00:00:00 2001 From: isonmad Date: Wed, 26 Oct 2016 09:49:45 -0400 Subject: [PATCH] fix Client.destroy bugs (#828) * add test for Client.destroy() * propagate errors in ClientManager.destroy If the promise returned by logout() rejects, previously it would be completely uncaught, and just return an eternally pending promise that never resolved. * fix RESTMethods.logout Without a data argument, the POST that superagent sends causes the discord server to reply with a HTTP 400 error: text: '{"Content-Type": "Expected Content-Type to be one of set([\'application/json\'])."}', * fix Client.destroy _timeouts and _intervals were changed to Set objects in commit 6ede7a32fd29bd44e65e344e1f102332d30c2129 a month ago. Changing them to arrays causes failures if you try to reuse the client object again. * always close websocket in ClientManager.destroy Invoking logout does not implicitly cause the server to close the websocket for you, so cleanup everything. Otherwise the websocket being open keeps node alive and hanging mysteriously until the connection hits a timeout. * fix indentation for eslint --- src/client/Client.js | 19 ++++++++----------- src/client/ClientManager.js | 13 +++++-------- src/client/rest/RESTMethods.js | 2 +- test/destroy.js | 12 ++++++++++++ 4 files changed, 26 insertions(+), 20 deletions(-) create mode 100644 test/destroy.js diff --git a/src/client/Client.js b/src/client/Client.js index 58e7f595f..f81c72954 100644 --- a/src/client/Client.js +++ b/src/client/Client.js @@ -234,17 +234,14 @@ class Client extends EventEmitter { * @returns {Promise} */ destroy() { - return new Promise((resolve, reject) => { - this.manager.destroy().then(() => { - for (const t of this._timeouts) clearTimeout(t); - for (const i of this._intervals) clearInterval(i); - this._timeouts = []; - this._intervals = []; - this.token = null; - this.email = null; - this.password = null; - resolve(); - }).catch(reject); + return this.manager.destroy().then(() => { + for (const t of this._timeouts) clearTimeout(t); + for (const i of this._intervals) clearInterval(i); + this._timeouts.clear(); + this._intervals.clear(); + this.token = null; + this.email = null; + this.password = null; }); } diff --git a/src/client/ClientManager.js b/src/client/ClientManager.js index 89acc523a..5e2a42b04 100644 --- a/src/client/ClientManager.js +++ b/src/client/ClientManager.js @@ -58,14 +58,11 @@ class ClientManager { } destroy() { - return new Promise((resolve) => { - if (!this.client.user.bot) { - this.client.rest.methods.logout().then(resolve); - } else { - this.client.ws.destroy(); - resolve(); - } - }); + let p = Promise.resolve(); + if (!this.client.user.bot) { + p = this.client.rest.methods.logout(); + } + return p.then(() => this.client.ws.destroy()); } } diff --git a/src/client/rest/RESTMethods.js b/src/client/rest/RESTMethods.js index 2bded4206..380a7b714 100644 --- a/src/client/rest/RESTMethods.js +++ b/src/client/rest/RESTMethods.js @@ -35,7 +35,7 @@ class RESTMethods { } logout() { - return this.rest.makeRequest('post', Constants.Endpoints.logout, true); + return this.rest.makeRequest('post', Constants.Endpoints.logout, true, {}); } getGateway() { diff --git a/test/destroy.js b/test/destroy.js new file mode 100644 index 000000000..08f934de0 --- /dev/null +++ b/test/destroy.js @@ -0,0 +1,12 @@ +'use strict'; + +const Discord = require('../'); + +const client = new Discord.Client({ fetch_all_members: false, api_request_method: 'sequential' }); + +const { email, password, token } = require('./auth.json'); + +let p = client.login(token); +p = p.then(() => client.destroy()); +p = p.then(() => client.login(token)); +p = p.then(() => client.destroy());