tj / axon

message-oriented socket library for node.js heavily inspired by zeromq
MIT License
1.5k stars 155 forks source link

Add typechecking to Socket#pack to avoid parsing undefined #125

Open skeggse opened 10 years ago

skeggse commented 10 years ago

Fixes #124 provided we can make the tests work.

Alright, so I can make the tests fail by adding push.send(undefined); to line 16 of test.arg-types.js, simply because it causes the Unexpected token u SyntaxError.

The message is then encoded as a buffer containing 110000000b6a3a756e646566696e6564, which includes j:undefined. That means that, as confirmed by walking through with Node's debugger, the JSON.stringify line in node-amp-message is being passed undefined, which returns undefined, which is coerced into the string 'undefined'. I'm wondering whether this would be more suited for amp-message rather than axon, given that we're not currently looping through the args before buffering them in Socket#pack.

Also, given that send usually queues messages until the socket is actually ready, the error won't be thrown until the socket is ready which means I can't use assert.throws in the test case.

navaru commented 9 years ago

@skeggse is this still valid? Otherwise can you please close the issue

skeggse commented 9 years ago

Yes, this appears to still be an issue.