From c801dcff2b12a9061e0c4b1a14374d89463cc913 Mon Sep 17 00:00:00 2001 From: Simon Schick Date: Tue, 24 Nov 2015 02:48:05 +0100 Subject: [PATCH 1/5] fixed rejecting promise invoking callbacks reduces unneeded promise wrap fixed cases where the first argument is callback was set despite success added missing returns in awaitResponse made defaults arrow functions, and commented out their defaults to prevent unsused vars added missing returns for status functions --- src/Client/Client.js | 950 +++++++++++++------------------------------ 1 file changed, 275 insertions(+), 675 deletions(-) diff --git a/src/Client/Client.js b/src/Client/Client.js index 7a7d82399..b4143d723 100644 --- a/src/Client/Client.js +++ b/src/Client/Client.js @@ -47,834 +47,434 @@ class Client extends EventEmitter { } // def login - login(email, password, cb = function (err, token) { }) { - var self = this; - return new Promise((resolve, reject) => { - - self.internal.login(email, password) - .then((token) => { - cb(null, token); - resolve(token); - }) - .catch((e) => { - cb(e); - reject(e); - }); - - }); + login(email, password, callback = (/*err, token*/) => {}) { + return this.internal.login(email, password) + .then((token) => { + callback(null, token); + return token; + }, callback); } // def logout - logout(cb = function (err) { }) { - var self = this; - return new Promise((resolve, reject) => { - - self.internal.logout() - .then(() => { - cb(); - resolve(); - }) - .catch((e) => { - cb(e); - reject(e); - }) - - }) + logout(callback = (/*err*/) => {}) { + return this.internal.logout() + .then(callback, callback); } // def sendMessage - sendMessage(where, content, options = {}, callback = function (e, m) { }) { - var self = this; - return new Promise((resolve, reject) => { + sendMessage(where, content, options = {}, callback = (/*e, m*/) => {}) { + if (typeof options === "function") { + // options is the callback + callback = options; + } - if (typeof options === "function") { - // options is the callback - callback = options; - } - - self.internal.sendMessage(where, content, options) - .then(m => { - callback(null, m); - resolve(m); - }).catch(e => { - callback(e); - reject(e); - }); - - }); + return this.internal.sendMessage(where, content, options) + .then(m => { + callback(null, m); + return m; + }, callback); } // def sendTTSMessage - sendTTSMessage(where, content, callback = function (e, m) { }) { - var self = this; - return new Promise((resolve, reject) => { - self.sendMessage(where, content, { tts: true }) - .then(m => { - callback(null, m); - resolve(m); - }).catch(e => { - callback(e); - reject(e); - }); - - }); + sendTTSMessage(where, content, callback = (/*e, m*/) => {}) { + return this.sendMessage(where, content, { tts: true }) + .then(m => { + callback(null, m); + return m; + }, callback); } // def reply - reply(where, content, options = {}, callback = function (e, m) { }) { - var self = this; - return new Promise((resolve, reject) => { + reply(where, content, options = {}, callback = (/*e, m*/) => {}) { - if (typeof options === "function") { - // options is the callback - callback = options; - } + if (typeof options === "function") { + // options is the callback + callback = options; + } - var msg = self.internal.resolver.resolveMessage(where); - if (msg) { - content = msg.author + ", " + content; - self.internal.sendMessage(msg, content, options) - .then(m => { - callback(null, m); - resolve(m); - }).catch(e => { - callback(e); - reject(e); - }); - } else { - var err = new Error("Destination not resolvable to a message!"); - callback(err); - reject(err); - } - - }); + var msg = this.internal.resolver.resolveMessage(where); + if (msg) { + content = msg.author + ", " + content; + return this.internal.sendMessage(msg, content, options) + .then(m => { + callback(null, m); + return m; + }, callback); + } + var err = new Error("Destination not resolvable to a message!"); + callback(err); + return Promise.reject(err); } // def replyTTS - replyTTS(where, content, callback = function () { }) { - return new Promise((resolve, reject) => { - self.reply(where, content, { tts: true }) - .then(m => { - callback(null, m); - resolve(m); - }).catch(e => { - callback(e); - reject(e); - }); - }); + replyTTS(where, content, callback = (/**/) => {}) { + return this.reply(where, content, { tts: true }) + .then(m => { + callback(null, m); + return m; + }, callback); } // def deleteMessage - deleteMessage(msg, options = {}, callback = function (e) { }) { - var self = this; - return new Promise((resolve, reject) => { - if (typeof options === "function") { - // options is the callback - callback = options; - } + deleteMessage(msg, options = {}, callback = (/*e*/) => {}) { + if (typeof options === "function") { + // options is the callback + callback = options; + } - self.internal.deleteMessage(msg, options) - .then(() => { - callback(); - resolve(); - }) - .catch(e => { - callback(e); - reject(e); - }); - - }); + return this.internal.deleteMessage(msg, options) + .then(() => { + callback(null); + }, callback); } //def updateMessage - updateMessage(msg, content, options = {}, callback = function (err, msg) { }) { - var self = this; - return new Promise((resolve, reject) => { - if (typeof options === "function") { - // options is the callback - callback = options; - } + updateMessage(msg, content, options = {}, callback = (/*err, msg*/) => {}) { + if (typeof options === "function") { + // options is the callback + callback = options; + } - self.internal.updateMessage(msg, content, options) - .then(msg => { - callback(null, msg); - resolve(msg); - }) - .catch(e => { - callback(e); - reject(e); - }); - - }); + return this.internal.updateMessage(msg, content, options) + .then(msg => { + callback(null, msg); + return msg; + }, callback); } // def getChannelLogs - getChannelLogs(where, limit = 500, options = {}, callback = function (err, logs) { }) { - - var self = this; - return new Promise((resolve, reject) => { - if (typeof options === "function") { - // options is the callback - callback = options; - } - self.internal.getChannelLogs(where, limit, options) - .then(logs => { - callback(null, logs); - resolve(logs); - }) - .catch(e => { - callback(e); - reject(e); - }); - - }); + getChannelLogs(where, limit = 500, options = {}, callback = (/*err, logs*/) => {}) { + if (typeof options === "function") { + // options is the callback + callback = options; + } + return this.internal.getChannelLogs(where, limit, options) + .then(logs => { + callback(null, logs); + return logs; + }, callback); } // def getBans - getBans(where, callback = function (err, bans) { }) { - - var self = this; - return new Promise((resolve, reject) => { - self.internal.getBans(where) - .then(bans => { - callback(null, bans); - resolve(bans); - }) - .catch(e => { - callback(e); - reject(e); - }); - - }); - + getBans(where, callback = (/*err, bans*/) => {}) { + return this.internal.getBans(where) + .then(bans => { + callback(null, bans); + return bans; + }, callback); } // def sendFile - sendFile(where, attachment, name = "image.png", callback = function (err, m) { }) { - var self = this; - return new Promise((resolve, reject) => { - self.internal.sendFile(where, attachment, name) - .then(m => { - callback(null, m); - resolve(m); - }) - .catch(e => { - callback(e); - reject(e); - }); - - }); + sendFile(where, attachment, name = "image.png", callback = (/*err, m*/) => {}) { + this.internal.sendFile(where, attachment, name) + .then(m => { + callback(null, m); + return m; + }, callback); } // def joinServer - joinServer(invite, callback = function (err, srv) { }) { - var self = this; - return new Promise((resolve, reject) => { - self.internal.joinServer(invite) - .then(srv => { - callback(null, srv); - resolve(srv); - }) - .catch(e => { - callback(e); - reject(e); - }); - }); + joinServer(invite, callback = (/*err, srv*/) => {}) { + this.internal.joinServer(invite) + .then(srv => { + callback(null, srv); + return srv; + }, callback); } // def createServer - createServer(name, region = "london", callback = function (err, srv) { }) { - var self = this; - return new Promise((resolve, reject) => { - self.internal.createServer(name, region) - .then(srv => { - callback(null, srv); - resolve(srv); - }) - .catch(e => { - callback(e); - reject(e); - }) - }); + createServer(name, region = "london", callback = (/*err, srv*/) => {}) { + this.internal.createServer(name, region) + .then(srv => { + callback(null, srv); + return srv; + }, callback); } // def leaveServer - leaveServer(server, callback = function (err) { }) { - var self = this; - return new Promise((resolve, reject) => { - - self.internal.leaveServer(server) - .then(() => { - callback(); resolve(); - }) - .catch(e => { - callback(e); reject(e); - }) - - }); + leaveServer(server, callback = (/*err*/) => {}) { + return this.internal.leaveServer(server) + .then(callback, callback); } // def createChannel - createChannel(server, name, type = "text", callback = function (err, channel) { }) { - var self = this; - return new Promise((resolve, reject) => { - if (typeof type === "function") { - // options is the callback - callback = type; - } - self.internal.createChannel(server, name, type) - .then((channel) => { - callback(channel); resolve(channel); - }) - .catch(e => { - callback(e); reject(e); - }) - - }); + createChannel(server, name, type = "text", callback = (/*err, channel*/) => {}) { + if (typeof type === "function") { + // options is the callback + callback = type; + } + return this.internal.createChannel(server, name, type) + .then((channel) => { + callback(channel); + return channel; + }, callback); } // def deleteChannel - deleteChannel(channel, callback = function (err) { }) { - var self = this; - return new Promise((resolve, reject) => { - - self.internal.deleteChannel(channel) - .then(() => { - callback(); - resolve(); - }) - .catch(e => { - callback(e); reject(e); - }) - - }); + deleteChannel(channel, callback = (/*err*/) => {}) { + return this.internal.deleteChannel(channel) + .then(callback, callback); } //def banMember - banMember(user, server, length = 1, callback = function (err) { }) { - var self = this; - return new Promise((resolve, reject) => { - if (typeof length === "function") { - // length is the callback - callback = length; - } - self.internal.banMember(user, server, length) - .then(() => { - callback(); - resolve(); - }) - .catch(e => { - callback(e); reject(e); - }) + banMember(user, server, length = 1, callback = (/*err*/) => {}) { - }); + if (typeof length === "function") { + // length is the callback + callback = length; + } + return this.internal.banMember(user, server, length) + .then(callback, callback); } //def unbanMember - unbanMember(user, server, callback = function (err) { }) { - var self = this; - return new Promise((resolve, reject) => { - self.internal.unbanMember(user, server) - .then(() => { - callback(); - resolve(); - }) - .catch(e => { - callback(e); reject(e); - }) - - }); + unbanMember(user, server, callback = (/*err*/) => {}) { + return this.internal.unbanMember(user, server) + .then(callback, callback); } //def kickMember - kickMember(user, server, callback = function (err) { }) { - var self = this; - return new Promise((resolve, reject) => { - self.internal.kickMember(user, server) - .then(() => { - callback(); - resolve(); - }) - .catch(e => { - callback(e); reject(e); - }) - - }); + kickMember(user, server, callback = (/*err*/) => {}) { + return this.internal.kickMember(user, server) + .then(callback, callback); } //def createRole - createRole(server, data = null, callback = function (err, res) { }) { - var self = this; - return new Promise((resolve, reject) => { - if (typeof data === "function") { - // data is the callback - callback = data; - } - self.internal.createRole(server, data) - .then((role) => { - callback(null, role); - resolve(role); - }) - .catch(e => { - callback(e); - reject(e); - }); - - }); + createRole(server, data = null, callback = (/*err, res*/) => {}) { + if (typeof data === "function") { + // data is the callback + callback = data; + } + return this.internal.createRole(server, data) + .then((role) => { + callback(null, role); + return role; + }, callback); } //def updateRole - updateRole(role, data = null, callback = function (err, res) { }) { - var self = this; - return new Promise((resolve, reject) => { - if (typeof data === "function") { - // data is the callback - callback = data; - } - self.internal.updateRole(role, data) - .then((role) => { - callback(null, role); - resolve(role); - }) - .catch(e => { - callback(e); - reject(e); - }); - - }); + updateRole(role, data = null, callback = (/*err, res*/) => {}) { + if (typeof data === "function") { + // data is the callback + callback = data; + } + return this.internal.updateRole(role, data) + .then((role) => { + callback(null, role); + return role; + }, callback); } //def deleteRole - deleteRole(role, callback = function (err) { }) { - - var self = this; - return new Promise((resolve, reject) => { - - self.internal.deleteRole(role) - .then(() => { - callback(); - resolve(); - }) - .catch(e => { - callback(e); - reject(e); - }); - - }); - + deleteRole(role, callback = (/*err*/) => {}) { + return this.internal.deleteRole(role) + .then(callback, callback); } //def addMemberToRole - addMemberToRole(member, role, callback = function (err) { }) { - var self = this; - return new Promise((resolve, reject) => { - - self.internal.addMemberToRole(member, role) - .then(() => { - callback(); - resolve(); - }) - .catch(e => { - callback(e); - reject(e); - }); - - }); + addMemberToRole(member, role, callback = (/*err*/) => {}) { + return this.internal.addMemberToRole(member, role) + .then(callback, callback); } // def addUserToRole - addUserToRole(member, role, callback = function (err) { }) { + addUserToRole(member, role, callback = (/*err*/) => {}) { return this.addMemberToRole(member, role, callback); } // def removeMemberFromRole - removeMemberFromRole(member, role, callback = function (err) { }) { - var self = this; - return new Promise((resolve, reject) => { - - self.internal.removeMemberFromRole(member, role) - .then(() => { - callback(); - resolve(); - }) - .catch(e => { - callback(e); - reject(e); - }); - - }); + removeMemberFromRole(member, role, callback = (/*err*/) => {}) { + return this.internal.removeMemberFromRole(member, role) + .then(callback, callback); } // def removeUserFromRole - removeUserFromRole(member, role, callback = function (err) { }) { + removeUserFromRole(member, role, callback = (/*err*/) => {}) { return this.removeUserFromRole(member, role, callback); } // def createInvite - createInvite(chanServ, options, callback = function (err, invite) { }) { - var self = this; - return new Promise((resolve, reject) => { - if (typeof options === "function") { - // length is the callback - callback = options; - } + createInvite(chanServ, options, callback = (/*err, invite*/) => {}) { - self.internal.createInvite(chanServ, options) - .then(invite => { - callback(null, invite); - resolve(invite); - }) - .catch(e => { - callback(e); - reject(e); - }); + if (typeof options === "function") { + // length is the callback + callback = options; + } - }); + return this.internal.createInvite(chanServ, options) + .then(invite => { + callback(null, invite); + }, callback); } // def deleteInvite - deleteInvite(invite, callback = function (err) { }) { - var self = this; - return new Promise((resolve, reject) => { - self.internal.deleteInvite(invite) - .then(() => { - callback(); - resolve(); - }) - .catch(e => { - callback(e); - reject(e); - }); - }); + deleteInvite(invite, callback = (/*err*/) => {}) { + this.internal.deleteInvite(invite) + .then(callback, callback); } // def overwritePermissions - overwritePermissions(channel, role, options = {}, callback = function (err) { }) { - var self = this; - return new Promise((resolve, reject) => { - - self.internal.overwritePermissions(channel, role, options) - .then(() => { - callback(); - resolve(); - }) - .catch(e => { - callback(e); - reject(e); - }) - - }); + overwritePermissions(channel, role, options = {}, callback = (/*err*/) => {}) { + return this.internal.overwritePermissions(channel, role, options) + .then(callback, callback); } //def setStatus - setStatus(idleStatus, gameID, callback = function (err) { }) { - var self = this; - return new Promise((resolve, reject) => { - if (typeof gameID === "function") { - // gameID is the callback - callback = gameID; - } - else if (typeof idleStatus === "function") { - // idleStatus is the callback - callback = idleStatus; - } + setStatus(idleStatus, gameID, callback = (/*err*/) => {}) { - self.internal.setStatus(idleStatus, gameID) - .then(() => { - callback(); - resolve(); - }) - .catch(e => { - callback(e); - reject(e); - }); + if (typeof gameID === "function") { + // gameID is the callback + callback = gameID; + } else if (typeof idleStatus === "function") { + // idleStatus is the callback + callback = idleStatus; + } - }) + return this.internal.setStatus(idleStatus, gameID) + .then(callback, callback); } //def sendTyping - sendTyping(channel, callback = function (err) { }) { - var self = this; - return new Promise((resolve, reject) => { - - self.internal.sendTyping(channel) - .then(() => { - callback(); - resolve(); - }) - .catch(e => { - callback(e); - reject(e); - }); - - }) + sendTyping(channel, callback = (/*err*/) => {}) { + return this.internal.sendTyping(channel) + .then(callback, callback); } // def setTopic - setTopic(channel, topic, callback = function (err) { }) { - var self = this; - return new Promise((resolve, reject) => { - - self.internal.setTopic(channel, topic) - .then(() => { - callback(); - resolve(); - }) - .catch(e => { - callback(e); - reject(e); - }); - - }) + setTopic(channel, topic, callback = (/*err*/) => {}) { + return this.internal.setTopic(channel, topic) + .then(callback, callback); } //def setChannelName - setChannelName(channel, name, callback = function (err) { }) { - var self = this; - return new Promise((resolve, reject) => { - - self.internal.setChannelName(channel, name) - .then(() => { - callback(); - resolve(); - }) - .catch(e => { - callback(e); - reject(e); - }); - - }) + setChannelName(channel, name, callback = (/*err*/) => {}) { + return this.internal.setChannelName(channel, name) + .then(callback, callback); } //def setChannelNameAndTopic - setChannelNameAndTopic(channel, name, topic, callback = function (err) { }) { - var self = this; - return new Promise((resolve, reject) => { - - self.internal.setChannelNameAndTopic(channel, name, topic) - .then(() => { - callback(); - resolve(); - }) - .catch(e => { - callback(e); - reject(e); - }); - - }) + setChannelNameAndTopic(channel, name, topic, callback = (/*err*/) => {}) { + return this.internal.setChannelNameAndTopic(channel, name, topic) + .then(callback, callback); } //def updateChannel - updateChannel(channel, data, callback = function (err) { }) { - var self = this; - return new Promise((resolve, reject) => { - - self.internal.updateChannel(channel, data) - .then(() => { - callback(); - resolve(); - }) - .catch(e => { - callback(e); - reject(e); - }); - - }) + updateChannel(channel, data, callback = (/*err*/) => {}) { + return this.internal.updateChannel(channel, data) + .then(callback, callback); } //def startTyping - startTyping(channel, callback = function (err) { }) { - var self = this; - return new Promise((resolve, reject) => { - - self.internal.startTyping(channel) - .then(() => { - callback(null); - resolve(); - }) - .catch(e => { - callback(e); - reject(e); - }); - - }); + startTyping(channel, callback = (/*err*/) => {}) { + return this.internal.startTyping(channel) + .then(callback, callback); } //def stopTyping - stopTyping(channel, callback = function (err) { }) { - var self = this; - return new Promise((resolve, reject) => { - - self.internal.stopTyping(channel) - .then(() => { - callback(null); - resolve(); - }) - .catch(e => { - callback(e); - reject(e); - }); - - }); + stopTyping(channel, callback = (/*err*/) => {}) { + return this.internal.stopTyping(channel) + .then(callback, callback); } //def updateDetails - updateDetails(details, callback = function (err) { }) { - var self = this; - return new Promise((resolve, reject) => { - self.internal.updateDetails(details) - .then(() => { - callback(); - resolve(); - }) - .catch(err => { - callback(err); - reject(err); - }); - }); + updateDetails(details, callback = (/*err*/) => {}) { + return this.internal.updateDetails(details) + .then(callback, callback); } //def setUsername - setUsername(name, callback = function (err) { }) { - var self = this; - return new Promise((resolve, reject) => { - self.internal.setUsername(name) - .then(() => { - callback(); - resolve(); - }) - .catch(err => { - callback(err); - reject(err); - }); - }); + setUsername(name, callback = (/*err*/) => {}) { + return this.internal.setUsername(name) + .then(callback, callback); } //def setAvatar - setAvatar(avatar, callback = function (err) { }) { - var self = this; - return new Promise((resolve, reject) => { - self.internal.setAvatar(avatar) - .then(() => { - callback(); - resolve(); - }) - .catch(err => { - callback(err); - reject(err); - }); - }); + setAvatar(avatar, callback = (/*err*/) => {}) { + return this.internal.setAvatar(avatar) + .then(callback, callback); } //def joinVoiceChannel - joinVoiceChannel(channel, callback = function (err) { }) { - var self = this; - return new Promise((resolve, reject) => { - - self.internal.joinVoiceChannel(channel) - .then(chan => { - callback(null, chan); - resolve(chan); - }) - .catch(err => { - callback(err); - reject(err); - }); - - }); + joinVoiceChannel(channel, callback = (/*err*/) => {}) { + return this.internal.joinVoiceChannel(channel) + .then(chan => { + callback(null, chan); + return chan; + }, callback); } - + // def leaveVoiceChannel - leaveVoiceChannel(callback = function (err) { }) { - var self = this; - return new Promise((resolve, reject) => { - self.internal.leaveVoiceChannel() - .then(() => { - callback(); - resolve(); - }) - .catch(err => { - callback(err); - reject(err); - }); - }); + leaveVoiceChannel(callback = (/*err*/) => {}) { + return this.internal.leaveVoiceChannel() + .then(callback, callback); } - + // def awaitResponse - awaitResponse(msg, toSend = null, options = null, callback = function (e, newMsg) { }) { - - var self = this; - - return new Promise((resolve, reject) => { + awaitResponse(msg, toSend = null, options = null, callback = (/*e, newMsg*/) => {}) { - function error(e) { - callback(e); - reject(e); - } + const final = () => + this.internal.awaitResponse(msg) + .then(newMsg => { + callback(null, newMsg); + }); - if (toSend) { - if (typeof toSend === "function") { - // (msg, callback) - callback = toSend; - final(); - } else { - // (msg, toSend, ...) - if (options) { - if (typeof options === "function") { - //(msg, toSend, callback) - callback = options; - this.sendMessage(msg, toSend).then(final).catch(error); - } else { - //(msg, toSend, options, callback) - this.sendMessage(msg, toSend, options).then(final).catch(error); - } - } else { - // (msg, toSend) promise - this.sendMessage(msg, toSend).then(final).catch(error); - } - } + var ret; + + if (toSend) { + if (typeof toSend === "function") { + // (msg, callback) + callback = toSend; } else { - // (msg) promise - final(); + // (msg, toSend, ...) + if (options) { + if (typeof options === "function") { + //(msg, toSend, callback) + callback = options; + ret = this.sendMessage(msg, toSend); + } else { + //(msg, toSend, options, callback) + ret = this.sendMessage(msg, toSend, options); + } + } else { + // (msg, toSend) promise + ret = this.sendMessage(msg, toSend); + } } + } - function final() { - self.internal.awaitResponse(msg) - .then(newMsg => { - resolve(newMsg); - callback(null, newMsg); - }) - .catch(error); - } - - }); - + if (!ret) { + ret = Promise.resolve(); + } + // (msg) promise + return ret.then(final).catch(callback); } - setStatusIdle() { - this.setStatus("idle"); + setStatusIdle(callback = (/*err*/) => {}) { + return this.internal.setStatus("idle") + .then(callback, callback); } - setStatusOnline(cb = function (err) { }) { - this.setStatus("online"); + setStatusOnline(callback = (/*err*/) => {}) { + return this.internal.setStatus("online") + .then(callback, callback); } - setStatusActive() { - this.setStatusOnline(); + setStatusActive(callback) { + return this.setStatusOnline(callback); } - setStatusHere() { - this.setStatusOnline(); + setStatusHere(callback) { + return this.setStatusOnline(callback); } - setStatusAvailable() { - this.setStatusOnline(); + setStatusAvailable(callback) { + return this.setStatusOnline(callback); } - setStatusAway() { - this.setStatusIdle(); + setStatusAway(callback) { + return this.setStatusIdle(callback); } setPlayingGame(game) { - this.setStatus(null, game); + return this.setStatus(null, game); } } -module.exports = Client; \ No newline at end of file +module.exports = Client; From bcbcc1e95d3dfdd22f9724adc5ac624c084b20d6 Mon Sep 17 00:00:00 2001 From: Simon Schick Date: Tue, 24 Nov 2015 02:59:26 +0100 Subject: [PATCH 2/5] Slightly reduced promise bloat --- src/Client/InternalClient.js | 89 ++++++++++++++++-------------------- 1 file changed, 39 insertions(+), 50 deletions(-) diff --git a/src/Client/InternalClient.js b/src/Client/InternalClient.js index d923bd496..673729f4e 100644 --- a/src/Client/InternalClient.js +++ b/src/Client/InternalClient.js @@ -54,16 +54,11 @@ class InternalClient { //def leaveVoiceChannel leaveVoiceChannel() { - var self = this; - return new Promise((resolve, reject) => { - if (self.voiceConnection) { - self.voiceConnection.destroy(); - self.voiceConnection = null; - resolve(); - } else { - resolve(); - } - }); + if (self.voiceConnection) { + self.voiceConnection.destroy(); + self.voiceConnection = null; + } + return Promise.resolve(); } //def awaitResponse @@ -91,51 +86,45 @@ class InternalClient { //def joinVoiceChannel joinVoiceChannel(chann) { - var self = this; - return new Promise((resolve, reject) => { + var channel = self.resolver.resolveVoiceChannel(chann); - var channel = self.resolver.resolveVoiceChannel(chann); + if (!channel) { + return Promise.reject(new Error("voice channel does not exist")); + } + return self.leaveVoiceChannel() + .then(() => { + return new Promise((resolve, reject) => { + var session, token, server = channel.server, endpoint; - if (channel) { + var check = (m) => { + var data = JSON.parse(m); + if (data.t === "VOICE_STATE_UPDATE") { + session = data.d.session_id; + } else if (data.t === "VOICE_SERVER_UPDATE") { + token = data.d.token; + endpoint = data.d.endpoint; + var chan = self.voiceConnection = new VoiceConnection(channel, self.client, session, token, server, endpoint); - self.leaveVoiceChannel().then(next); + chan.on("ready", () => resolve(chan)); + chan.on("error", reject); - function next() { - var session, token, server = channel.server, endpoint; + self.client.emit("debug", "removed temporary voice websocket listeners"); + self.websocket.removeListener("message", check); - var check = (m) => { - var data = JSON.parse(m); - if (data.t === "VOICE_STATE_UPDATE") { - session = data.d.session_id; - } else if (data.t === "VOICE_SERVER_UPDATE") { - token = data.d.token; - endpoint = data.d.endpoint; - var chan = self.voiceConnection = new VoiceConnection(channel, self.client, session, token, server, endpoint); - - chan.on("ready", () => resolve(chan)); - chan.on("error", reject); - - self.client.emit("debug", "removed temporary voice websocket listeners"); - self.websocket.removeListener("message", check); - - } - }; - - self.websocket.on("message", check); - self.sendWS({ - op: 4, - d: { - "guild_id": server.id, - "channel_id": channel.id, - "self_mute": false, - "self_deaf": false - } - }); - } - } else { - reject(new Error("voice channel does not exist")); - } + } + }; + self.websocket.on("message", check); + self.sendWS({ + op: 4, + d: { + "guild_id": server.id, + "channel_id": channel.id, + "self_mute": false, + "self_deaf": false + } + }); + }); }); } @@ -1697,4 +1686,4 @@ class InternalClient { } } -module.exports = InternalClient; \ No newline at end of file +module.exports = InternalClient; From db5b333085c3e1eacd11d8a77327c1cd6c00269d Mon Sep 17 00:00:00 2001 From: Simon Schick Date: Tue, 24 Nov 2015 03:10:06 +0100 Subject: [PATCH 3/5] Forgot some return statements --- src/Constants.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Constants.js b/src/Constants.js index 694365b39..88d052009 100644 --- a/src/Constants.js +++ b/src/Constants.js @@ -87,4 +87,4 @@ var PacketType = { exports.API_ENDPOINT = API; exports.Endpoints = Endpoints; exports.PacketType = PacketType; -exports.Permissions = Permissions; \ No newline at end of file +exports.Permissions = Permissions; From 0b1a1651393e8a6837b441ef45d0284425108559 Mon Sep 17 00:00:00 2001 From: Simon Schick Date: Tue, 24 Nov 2015 03:21:56 +0100 Subject: [PATCH 4/5] Prevent error swallowing --- src/Client/Client.js | 117 ++++++++++++++++++++++--------------------- 1 file changed, 60 insertions(+), 57 deletions(-) diff --git a/src/Client/Client.js b/src/Client/Client.js index b4143d723..4b06ca395 100644 --- a/src/Client/Client.js +++ b/src/Client/Client.js @@ -3,6 +3,13 @@ var InternalClient = require("./InternalClient.js"); var EventEmitter = require("events"); +function errCB(callback) { + return error => { + callback(error); + throw error; + }; +} + class Client extends EventEmitter { /* this class is an interface for the internal @@ -52,13 +59,13 @@ class Client extends EventEmitter { .then((token) => { callback(null, token); return token; - }, callback); + }, errCB(callback)); } // def logout logout(callback = (/*err*/) => {}) { return this.internal.logout() - .then(callback, callback); + .then(callback, errCB(callback)); } // def sendMessage sendMessage(where, content, options = {}, callback = (/*e, m*/) => {}) { @@ -71,7 +78,7 @@ class Client extends EventEmitter { .then(m => { callback(null, m); return m; - }, callback); + }, errCB(callback)); } // def sendTTSMessage @@ -80,7 +87,7 @@ class Client extends EventEmitter { .then(m => { callback(null, m); return m; - }, callback); + }, errCB(callback)); } // def reply reply(where, content, options = {}, callback = (/*e, m*/) => {}) { @@ -97,7 +104,7 @@ class Client extends EventEmitter { .then(m => { callback(null, m); return m; - }, callback); + }, errCB(callback)); } var err = new Error("Destination not resolvable to a message!"); callback(err); @@ -110,7 +117,7 @@ class Client extends EventEmitter { .then(m => { callback(null, m); return m; - }, callback); + }, errCB(callback)); } // def deleteMessage deleteMessage(msg, options = {}, callback = (/*e*/) => {}) { @@ -120,9 +127,7 @@ class Client extends EventEmitter { } return this.internal.deleteMessage(msg, options) - .then(() => { - callback(null); - }, callback); + .then(callback, errCB(callback)); } //def updateMessage updateMessage(msg, content, options = {}, callback = (/*err, msg*/) => {}) { @@ -135,7 +140,7 @@ class Client extends EventEmitter { .then(msg => { callback(null, msg); return msg; - }, callback); + }, errCB(callback)); } // def getChannelLogs @@ -149,7 +154,7 @@ class Client extends EventEmitter { .then(logs => { callback(null, logs); return logs; - }, callback); + }, errCB(callback)); } // def getBans @@ -158,40 +163,40 @@ class Client extends EventEmitter { .then(bans => { callback(null, bans); return bans; - }, callback); + }, errCB(callback)); } // def sendFile sendFile(where, attachment, name = "image.png", callback = (/*err, m*/) => {}) { - this.internal.sendFile(where, attachment, name) + return this.internal.sendFile(where, attachment, name) .then(m => { callback(null, m); return m; - }, callback); + }, errCB(callback)); } // def joinServer joinServer(invite, callback = (/*err, srv*/) => {}) { - this.internal.joinServer(invite) + return this.internal.joinServer(invite) .then(srv => { callback(null, srv); return srv; - }, callback); + }, errCB(callback)); } // def createServer createServer(name, region = "london", callback = (/*err, srv*/) => {}) { - this.internal.createServer(name, region) + return this.internal.createServer(name, region) .then(srv => { callback(null, srv); return srv; - }, callback); + }, errCB(callback)); } // def leaveServer leaveServer(server, callback = (/*err*/) => {}) { return this.internal.leaveServer(server) - .then(callback, callback); + .then(callback, errCB(callback)); } // def createChannel @@ -204,13 +209,13 @@ class Client extends EventEmitter { .then((channel) => { callback(channel); return channel; - }, callback); + }, errCB(callback)); } // def deleteChannel deleteChannel(channel, callback = (/*err*/) => {}) { return this.internal.deleteChannel(channel) - .then(callback, callback); + .then(callback, errCB(callback)); } //def banMember @@ -221,19 +226,19 @@ class Client extends EventEmitter { callback = length; } return this.internal.banMember(user, server, length) - .then(callback, callback); + .then(callback, errCB(callback)); } //def unbanMember unbanMember(user, server, callback = (/*err*/) => {}) { return this.internal.unbanMember(user, server) - .then(callback, callback); + .then(callback, errCB(callback)); } //def kickMember kickMember(user, server, callback = (/*err*/) => {}) { return this.internal.kickMember(user, server) - .then(callback, callback); + .then(callback, errCB(callback)); } //def createRole @@ -246,7 +251,7 @@ class Client extends EventEmitter { .then((role) => { callback(null, role); return role; - }, callback); + }, errCB(callback)); } //def updateRole @@ -259,35 +264,35 @@ class Client extends EventEmitter { .then((role) => { callback(null, role); return role; - }, callback); + }, errCB(callback)); } //def deleteRole deleteRole(role, callback = (/*err*/) => {}) { return this.internal.deleteRole(role) - .then(callback, callback); + .then(callback, errCB(callback)); } //def addMemberToRole addMemberToRole(member, role, callback = (/*err*/) => {}) { return this.internal.addMemberToRole(member, role) - .then(callback, callback); + .then(callback, errCB(callback)); } // def addUserToRole addUserToRole(member, role, callback = (/*err*/) => {}) { - return this.addMemberToRole(member, role, callback); + return this.addMemberToRole(member, role, errCB(callback)); } // def removeMemberFromRole removeMemberFromRole(member, role, callback = (/*err*/) => {}) { return this.internal.removeMemberFromRole(member, role) - .then(callback, callback); + .then(callback, errCB(callback)); } // def removeUserFromRole removeUserFromRole(member, role, callback = (/*err*/) => {}) { - return this.removeUserFromRole(member, role, callback); + return this.removeUserFromRole(member, role, errCB(callback)); } // def createInvite @@ -301,19 +306,19 @@ class Client extends EventEmitter { return this.internal.createInvite(chanServ, options) .then(invite => { callback(null, invite); - }, callback); + }, errCB(callback)); } // def deleteInvite deleteInvite(invite, callback = (/*err*/) => {}) { - this.internal.deleteInvite(invite) - .then(callback, callback); + return this.internal.deleteInvite(invite) + .then(callback, errCB(callback)); } // def overwritePermissions overwritePermissions(channel, role, options = {}, callback = (/*err*/) => {}) { return this.internal.overwritePermissions(channel, role, options) - .then(callback, callback); + .then(callback, errCB(callback)); } //def setStatus @@ -328,67 +333,67 @@ class Client extends EventEmitter { } return this.internal.setStatus(idleStatus, gameID) - .then(callback, callback); + .then(callback, errCB(callback)); } //def sendTyping sendTyping(channel, callback = (/*err*/) => {}) { return this.internal.sendTyping(channel) - .then(callback, callback); + .then(callback, errCB(callback)); } // def setTopic setTopic(channel, topic, callback = (/*err*/) => {}) { return this.internal.setTopic(channel, topic) - .then(callback, callback); + .then(callback, errCB(callback)); } //def setChannelName setChannelName(channel, name, callback = (/*err*/) => {}) { return this.internal.setChannelName(channel, name) - .then(callback, callback); + .then(callback, errCB(callback)); } //def setChannelNameAndTopic setChannelNameAndTopic(channel, name, topic, callback = (/*err*/) => {}) { return this.internal.setChannelNameAndTopic(channel, name, topic) - .then(callback, callback); + .then(callback, errCB(callback)); } //def updateChannel updateChannel(channel, data, callback = (/*err*/) => {}) { return this.internal.updateChannel(channel, data) - .then(callback, callback); + .then(callback, errCB(callback)); } //def startTyping startTyping(channel, callback = (/*err*/) => {}) { return this.internal.startTyping(channel) - .then(callback, callback); + .then(callback, errCB(callback)); } //def stopTyping stopTyping(channel, callback = (/*err*/) => {}) { return this.internal.stopTyping(channel) - .then(callback, callback); + .then(callback, errCB(callback)); } //def updateDetails updateDetails(details, callback = (/*err*/) => {}) { return this.internal.updateDetails(details) - .then(callback, callback); + .then(callback, errCB(callback)); } //def setUsername setUsername(name, callback = (/*err*/) => {}) { return this.internal.setUsername(name) - .then(callback, callback); + .then(callback, errCB(callback)); } //def setAvatar setAvatar(avatar, callback = (/*err*/) => {}) { return this.internal.setAvatar(avatar) - .then(callback, callback); + .then(callback, errCB(callback)); } //def joinVoiceChannel @@ -397,24 +402,18 @@ class Client extends EventEmitter { .then(chan => { callback(null, chan); return chan; - }, callback); + }, errCB(callback)); } // def leaveVoiceChannel leaveVoiceChannel(callback = (/*err*/) => {}) { return this.internal.leaveVoiceChannel() - .then(callback, callback); + .then(callback, errCB(callback)); } // def awaitResponse awaitResponse(msg, toSend = null, options = null, callback = (/*e, newMsg*/) => {}) { - const final = () => - this.internal.awaitResponse(msg) - .then(newMsg => { - callback(null, newMsg); - }); - var ret; if (toSend) { @@ -443,17 +442,21 @@ class Client extends EventEmitter { ret = Promise.resolve(); } // (msg) promise - return ret.then(final).catch(callback); + return ret.then(() => this.internal.awaitResponse(msg)) + .then((newMsg) => { + callback(null, newMsg); + return newMsg; + }, errCB(callback)); } setStatusIdle(callback = (/*err*/) => {}) { return this.internal.setStatus("idle") - .then(callback, callback); + .then(callback, errCB(callback)); } setStatusOnline(callback = (/*err*/) => {}) { return this.internal.setStatus("online") - .then(callback, callback); + .then(callback, errCB(callback)); } setStatusActive(callback) { From fd54dc4f1f887db902cfff5210a52f3a4fd16832 Mon Sep 17 00:00:00 2001 From: Simon Schick Date: Tue, 24 Nov 2015 03:27:56 +0100 Subject: [PATCH 5/5] Fixed stack overflow in removeUserFromRole, removed redudant argument grouping, removed redudant error callback wrap --- src/Client/Client.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Client/Client.js b/src/Client/Client.js index 4b06ca395..f49453585 100644 --- a/src/Client/Client.js +++ b/src/Client/Client.js @@ -56,7 +56,7 @@ class Client extends EventEmitter { // def login login(email, password, callback = (/*err, token*/) => {}) { return this.internal.login(email, password) - .then((token) => { + .then(token => { callback(null, token); return token; }, errCB(callback)); @@ -206,7 +206,7 @@ class Client extends EventEmitter { callback = type; } return this.internal.createChannel(server, name, type) - .then((channel) => { + .then(channel => { callback(channel); return channel; }, errCB(callback)); @@ -248,7 +248,7 @@ class Client extends EventEmitter { callback = data; } return this.internal.createRole(server, data) - .then((role) => { + .then(role => { callback(null, role); return role; }, errCB(callback)); @@ -261,7 +261,7 @@ class Client extends EventEmitter { callback = data; } return this.internal.updateRole(role, data) - .then((role) => { + .then(role => { callback(null, role); return role; }, errCB(callback)); @@ -281,7 +281,7 @@ class Client extends EventEmitter { // def addUserToRole addUserToRole(member, role, callback = (/*err*/) => {}) { - return this.addMemberToRole(member, role, errCB(callback)); + return this.addMemberToRole(member, role, callback); } // def removeMemberFromRole @@ -292,7 +292,7 @@ class Client extends EventEmitter { // def removeUserFromRole removeUserFromRole(member, role, callback = (/*err*/) => {}) { - return this.removeUserFromRole(member, role, errCB(callback)); + return this.removeMemberFromRole(member, role, callback); } // def createInvite @@ -443,7 +443,7 @@ class Client extends EventEmitter { } // (msg) promise return ret.then(() => this.internal.awaitResponse(msg)) - .then((newMsg) => { + .then(newMsg => { callback(null, newMsg); return newMsg; }, errCB(callback));