Skip to content

Commit

Permalink
fix: do not catch encoding errors
Browse files Browse the repository at this point in the history
It does not make sense to catch the errors thrown by JSON.stringify()
and convert them to an ERROR packet (which are meant for namespace
authentication errors), it should be caught higher in the stack.

Related: 92c530d
  • Loading branch information
darrachequesne committed Sep 22, 2020
1 parent 567c0ca commit aeae87c
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 21 deletions.
18 changes: 1 addition & 17 deletions lib/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,12 +87,7 @@ export class Encoder {

// json data
if (null != obj.data) {
const payload = tryStringify(obj.data);
if (payload !== false) {
str += payload;
} else {
return ERROR_PACKET;
}
str += JSON.stringify(obj.data);
}

debug("encoded %j as %s", obj, str);
Expand All @@ -117,22 +112,11 @@ export class Encoder {
}
}

const ERROR_PACKET = PacketType.ERROR + '"encode error"';

function tryStringify(str) {
try {
return JSON.stringify(str);
} catch (e) {
return false;
}
}

/**
* A socket.io Decoder instance
*
* @return {Object} decoder
*/
// @ts-ignore
export class Decoder extends Emitter {
private reconstructor: BinaryReconstructor;

Expand Down
6 changes: 2 additions & 4 deletions test/parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ describe('parser', function(){
});
});

it('properly handles circular objects', function() {
it('throws an error when encoding circular objects', function() {
var a = {};
a.b = a;

Expand All @@ -72,9 +72,7 @@ describe('parser', function(){

const encoder = new Encoder();

encoder.encode(data, function(encodedPackets) {
expect(encodedPackets[0]).to.be('4"encode error"');
});
expect(() => encoder.encode(data)).to.throwException();
});

it('decodes a bad binary packet', function(){
Expand Down

0 comments on commit aeae87c

Please sign in to comment.