SyntaxError: Unexpected token u

[skeggse]

I'm not sure what's causing this yet, apart from the obvious that JSON.parse is passed undefined. I'm not trying to do anything particularly fancy, I'll see if I can track it down or reduce it to a workable test-case.

Node v0.10.24 and v0.10.26.

undefined:1
undefined
^
SyntaxError: Unexpected token u
    at Object.parse (native)
    at unpack (/{REDACTED}/node_modules/axon/node_modules/amp-message/index.js:136:32)
    at decode (/{REDACTED}/node_modules/axon/node_modules/amp-message/index.js:83:15)
    at new Message (/{REDACTED}/node_modules/axon/node_modules/amp-message/index.js:35:37)
    at Parser.<anonymous> (/{REDACTED}/node_modules/axon/lib/sockets/sock.js:223:15)
    at Parser.EventEmitter.emit (events.js:95:17)
    at Parser._write (/{REDACTED}/node_modules/axon/node_modules/amp/lib/stream.js:91:16)
    at doWrite (_stream_writable.js:221:10)
    at writeOrBuffer (_stream_writable.js:211:5)
    at Parser.Writable.write (_stream_writable.js:180:11)

[tj]

hmm I don't think we really have any asserts in place (delegating that to the user) so you may want to add a few for data being sent. One thing for debugging is to add if (msg == null) console.trace('wut') to axon in your node_modules so you can see where it was sent

[tj]

re-open if it turns out to be axon but I think there's just some input getting messed

[skeggse]

I had to step away but before I left I think I discovered that the buffer slice method is returning undefined! I really hope that isn't the case, I'll know more in a bit.

[tj]

interesting! that would be weird, let me know if you find anything

[skeggse]

Ha silly human make mistake.

Don't mind me, just...passing undefined to the send method.

[skeggse]

It seems like it would be valuable to catch errors thrown during the deserialization process and enable the user of axon to catch those errors, rather than allowing the errors to halt the process. If I pulled something of that sort together, would you consider a pull request?

[tj]

we should add asserts on input, and assume everything that gets in is valid. Even just assert(msg, 'message required')-ish asserts would be fine

[gjohnson]

+1 to assert. gogogo!

[skeggse]

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.

Anywho, I submitted pull request #125, but it'll need some revision.

[tj]

for amp-message I'd almost rather just convert it to null and send that, since it's a valid value, just not ideal since it usually represents a user error. Maybe we should do that instead, there's nothing really wrong with sending undefined

[skeggse]

Sounds good to me, :+1: for simplicity.

Alternatively, if we don't want to break some weird fringe case that somehow depends on the value being exactly undefined, we could just check if the arguments is equal to the string 'undefined' during the decode process.