nats-io / nats.node

Node.js client for NATS, the cloud native messaging system.
https://nats.io
Apache License 2.0
1.55k stars 163 forks source link

Setting the option json = true changes the method signature for publish #93

Closed Nullmage closed 6 years ago

Nullmage commented 8 years ago

Setting the json option to true makes the msg argument for the publish method non-optional.

const nats = require("nats").connect({ "url": "...", "json": true });

Invoking nats.request("topic", callback) will now cause an error because of the type check of msg https://github.com/nats-io/node-nats/blob/master/lib/nats.js#L990. The msg argument gets defaulted to EMPTY (which is the empty string '').

It seems a bit unintuitive that the signature for the method changes based on an option. I would still like to send empty messages but let the nats library automatically JSON.parse the responses from request invocations.

mkozjak commented 8 years ago

For some reason, the link from above does not work. Here's a working one: https://github.com/nats-io/node-nats/blob/master/lib/nats.js#L990.

Btw, I'm having the same complaint.

aricart commented 8 years ago

@GlurG @mkozjak the request seems reasonable. However there are few subtle considerations. The node API is provides support for:

The main issue is how are the types interpreted, and more importantly what happens when a non-node client receives the message.

By simply looking at JSON http://www.json.org from the javascript side, the following serializations are all valid JSON for purposes of decoding {}, [], "", number, true, false, null. Currently node nats is only allowing object/array types to be encoded as JSON, as any other type is rejected with an exception. [JSON.parse() is actually quite unhappy about some of these values]

Out of the above, it would seem that null is the proper default value if no value is provided to the publish call.

Before we do this, we'll have to see how other languages handle these values, because by specifying a format, we may also cause subtle issues on non-javascript clients that don't adhere correctly to deserialization/serialization (go unmarshaller can for example return nil). It will also create a possible situation for older node clients as some values not possible before would be.

aricart commented 8 years ago

Did a bit more testing with the encoding. JSON.parse() was unhappy with some values that should have worked correctly. It had to do with the the quoting. IE. JSON.parse('""') is happy for node, while JSON.parse("''") is not.

aricart commented 6 years ago

If values are not provided in request/reply or publish - the value is set to null.