Skip to content

Explain error messages better (be developer friendly) #429

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Explain error messages better (be developer friendly) #429

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
76 changes: 58 additions & 18 deletions lib/WebSocket.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,10 @@ var protocolVersion = 13;

var closeTimeout = 30 * 1000; // Allow 30 seconds to terminate the connection cleanly

// All possible WebSocket states

var readyStates = ["CONNECTING", "OPEN", "CLOSING", "CLOSED"]

/**
* WebSocket implementation
*
Expand Down Expand Up @@ -83,10 +87,33 @@ util.inherits(WebSocket, EventEmitter);
/**
* Ready States
*/
["CONNECTING", "OPEN", "CLOSING", "CLOSED"].forEach(function each(state, index) {
readyStates.forEach(function each(state, index) {
WebSocket.prototype[state] = WebSocket[state] = index;
});

/**
* A helper to generate a common error many public functions might throw
*/
function generateBadStatusError(instance) {
return new Error(
'Bad socket status! Expected state is OPEN but is currently set to ' + instance.getReadyState(true)
);
}

/**
* Returns the read state. Either as an integer or as a string.
*
* @param {boolean} When asString is set to true, then the ready state is returned as a string. Useful for error messages.
* @api public
*/
WebSocket.prototype.getReadyState = function(asString) {
if (asString) {
return readyStates[this.readyState]
} else {
return this.readyState
}
}

/**
* Gracefully closes the connection, after sending a description message to the server
*
Expand Down Expand Up @@ -136,7 +163,8 @@ WebSocket.prototype.close = function close(code, data) {
* @api public
*/
WebSocket.prototype.pause = function pauser() {
if (this.readyState !== WebSocket.OPEN) throw new Error('not opened');
if (this.readyState !== WebSocket.OPEN)
throw generateBadStatusError(this);

return this._socket.pause();
};
Expand All @@ -150,9 +178,10 @@ WebSocket.prototype.pause = function pauser() {
* @api public
*/
WebSocket.prototype.ping = function ping(data, options, dontFailWhenClosed) {
if (this.readyState !== WebSocket.OPEN) {
if (this.getReadyState() !== WebSocket.OPEN) {
if (dontFailWhenClosed === true) return;
throw new Error('not opened');

throw generateBadStatusError(this);
}

options = options || {};
Expand All @@ -171,9 +200,10 @@ WebSocket.prototype.ping = function ping(data, options, dontFailWhenClosed) {
* @api public
*/
WebSocket.prototype.pong = function(data, options, dontFailWhenClosed) {
if (this.readyState !== WebSocket.OPEN) {
if (this.getReadyState() !== WebSocket.OPEN) {
if (dontFailWhenClosed === true) return;
throw new Error('not opened');

throw generateBadStatusError(this);
}

options = options || {};
Expand All @@ -189,7 +219,8 @@ WebSocket.prototype.pong = function(data, options, dontFailWhenClosed) {
* @api public
*/
WebSocket.prototype.resume = function resume() {
if (this.readyState !== WebSocket.OPEN) throw new Error('not opened');
if (this.readyState !== WebSocket.OPEN)
throw generateBadStatusError(this)

return this._socket.resume();
};
Expand All @@ -209,9 +240,12 @@ WebSocket.prototype.send = function send(data, options, cb) {
options = {};
}

if (this.readyState !== WebSocket.OPEN) {
if (typeof cb === 'function') cb(new Error('not opened'));
else throw new Error('not opened');
if (this.getReadyState() !== WebSocket.OPEN) {

var err = generateBadStatusError(this);

if (typeof cb === 'function') cb(err);
else throw err;
return;
}

Expand Down Expand Up @@ -280,9 +314,12 @@ WebSocket.prototype.stream = function stream(options, cb) {

if (typeof cb !== 'function') throw new Error('callback must be provided');

if (this.readyState !== WebSocket.OPEN) {
if (typeof cb === 'function') cb(new Error('not opened'));
else throw new Error('not opened');
if (this.getReadyState() !== WebSocket.OPEN) {

var err = generateBadStatusError(this);

if (typeof cb === 'function') cb(err);
else throw err;
return;
}

Expand All @@ -303,7 +340,10 @@ WebSocket.prototype.stream = function stream(options, cb) {

function send(data, final) {
try {
if (self.readyState !== WebSocket.OPEN) throw new Error('not opened');
if (self.getReadyState() !== WebSocket.OPEN) {
throw generateBadStatusError(self);
}

options.fin = final === true;
self._sender.send(data, options);
if (!final) process.nextTick(cb.bind(null, null, send));
Expand Down Expand Up @@ -863,10 +903,10 @@ function executeQueueSends(instance) {
function sendStream(instance, stream, options, cb) {
stream.on('data', function incoming(data) {
if (instance.readyState !== WebSocket.OPEN) {
if (typeof cb === 'function') cb(new Error('not opened'));
if (typeof cb === 'function') cb(generateBadStatusError(instance));
else {
delete instance._queue;
instance.emit('error', new Error('not opened'));
instance.emit('error', generateBadStatusError(instance));
}
return;
}
Expand All @@ -877,10 +917,10 @@ function sendStream(instance, stream, options, cb) {

stream.on('end', function end() {
if (instance.readyState !== WebSocket.OPEN) {
if (typeof cb === 'function') cb(new Error('not opened'));
if (typeof cb === 'function') cb(generateBadStatusError(instance));
else {
delete instance._queue;
instance.emit('error', new Error('not opened'));
instance.emit('error', generateBadStatusError(instance));
}
return;
}
Expand Down
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@
"ansi": "0.3.x",
"benchmark": "0.3.x",
"expect.js": "0.3.x",
"mocha": "2.0.x",
"should": "4.3.x",
"mocha": "2.1.0",
"should": "4.4.2",
"tinycolor": "0.0.x"
},
"browser": "./lib/browser.js",
Expand Down
35 changes: 30 additions & 5 deletions test/WebSocket.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,10 @@ describe('WebSocket', function() {
ws.ping();
}
catch (e) {
e.should.be.an.Error;
e.toString().should.equal(
'Error: Bad socket status! Expected state is OPEN but is currently set to CONNECTING'
)
srv.close();
ws.terminate();
done();
Expand Down Expand Up @@ -544,6 +548,11 @@ describe('WebSocket', function() {
ws.pong();
}
catch (e) {
e.should.be.an.Error;
e.toString().should.equal(
'Error: Bad socket status! Expected state is OPEN but is currently set to CONNECTING'
)

srv.close();
ws.terminate();
done();
Expand Down Expand Up @@ -721,6 +730,10 @@ describe('WebSocket', function() {
ws.send('hi');
}
catch (e) {
e.should.be.an.Error;
e.toString().should.equal(
'Error: Bad socket status! Expected state is OPEN but is currently set to CONNECTING'
)
ws.terminate();
srv.close();
done();
Expand Down Expand Up @@ -1031,7 +1044,7 @@ describe('WebSocket', function() {
ws.terminate();
done();
});
ws.on('error', function() { /* That's quite alright -- a send was attempted after close */ });
ws.on('error', function(err) { /* That's quite alright -- a send was attempted after close */ });
srv.on('message', function(data, flags) {
assert.ok(!flags.binary);
assert.ok(areArraysEqual(fs.readFileSync('test/fixtures/textfile', 'utf8'), data));
Expand Down Expand Up @@ -1080,6 +1093,11 @@ describe('WebSocket', function() {
ws.on('error', function() {});
ws.stream(function(error) {
assert.ok(error instanceof Error);

error.toString().should.equal(
'Error: Bad socket status! Expected state is OPEN but is currently set to CONNECTING'
)

ws.terminate();
srv.close();
done();
Expand Down Expand Up @@ -1282,7 +1300,10 @@ describe('WebSocket', function() {
send(payload.substr(5, 5), true);
}
else if (i == 3) {
assert.ok(error);
error.should.be.an.Error;
error.toString().should.equal(
'Error: Bad socket status! Expected state is OPEN but is currently set to CLOSING'
)
errorGiven = true;
}
});
Expand Down Expand Up @@ -1315,7 +1336,11 @@ describe('WebSocket', function() {
fileStream.setEncoding('utf8');
fileStream.bufferSize = 100;
ws.send(fileStream, function(error) {
errorGiven = error != null;
error.should.be.an.Error;
error.toString().should.equal(
'Error: Bad socket status! Expected state is OPEN but is currently set to CLOSING'
)
errorGiven = error !== null;
});
ws.close(1000, 'foobar');
});
Expand Down Expand Up @@ -1926,7 +1951,7 @@ describe('WebSocket', function() {
var errorGiven = false;
ws.on('open', function() {
ws.send('hi', function(error) {
errorGiven = error != null;
errorGiven = error !== undefined;
});
ws.close();
});
Expand All @@ -1949,7 +1974,7 @@ describe('WebSocket', function() {
var errorGiven = false;
ws.on('open', function() {
ws.send('hi', function(error) {
errorGiven = error != null;
errorGiven = error !== null;
});
ws.terminate();
});
Expand Down