Documentation fails to mention required payload type with externally-published messages

[marceliwac]

Prerequisites

Please answer the following questions for yourself before submitting an issue.

• [x] I am running the latest version
• [x] I checked the documentation and found no answer
• [x] I checked to make sure that this issue has not already been filed
• [x] I'm reporting the issue to the correct repository

Current Behavior

Publishing messages from within the moleculer actions/methods works fine. For example:

Publishing messages with objects in payload ✅

await ctx.broker.sendToChannel('mystream.mysubject',  {'abc': 123})

Publishing messages with strings in payload ✅

await ctx.broker.sendToChannel('mystream.mysubject', 'TEST');

This behaviour changes when publishing messages externally (e.g. using NATS CLI, or from a python script connected to the same NATS instance).

Publishing messages with objects in payload ✅

# Python
await jetstream.publish('mystream.mysubject', str.encode('{"abc": 123}'))

Publishing messages with strings in payload 🚫

# Python
await jetstream.publish('mystream.mysubject', str.encode('TEST'))

This produces the following error:

ERROR: Message redelivered too many times (3). Drop message... 1

Needless to say, it took me a while to pinpoint this issue to the payload, rather than some problems with invalid delivery of messages. This plugin seems to be rejecting the messages / nack'ing them without any information that the messages were received in the first place.

Expected Behavior

Publishing string-based messages (or other types for that matter) works fine. Alternatively, there should be some mention in the documentation on the limitations of how this integrates with externally-published messages.

[marceliwac]

Just to add to this... This comes down to the fact that internally-published messages are serialized and de-serialized using the same serializer, which means that in reality, whatever format they serialize it to is also the format they expect to deserialize later on.

For example, providing JSON works fine because the default serializer (JSON) handles this conversion correctly. Similarly, JSON can also serialise and deserialise strings:

JSON.stringify('Hello world')
// '"Hello world"'
JSON.parse(JSON.stringify('Hello world'))
// 'Hello world'

Following that method, supplying the payload of '"Hello world"' externally should also work, which it does.

I think it's still worth adding something to documentation that emphasizes how externally-published messages are deserialised to prevent people ending up in this pitfall. But more importantly, the current error message is very misleading and adding some better visibility into messages which could not be serialised should be the focus here.

I am happy to open a PR if this is a change you would find helpful.

[icebob]

@marceliwac Could you create a minimal reproduction example code in Python so that we can investigate the issue?

[AndreMaz]

Here's the repro example: https://github.com/moleculerjs/moleculer-channels/blob/issue-76/examples/external-message/index.js

@marceliwac did a good job looking for a source of the problem.

Overall, channels cannot parse the payload if the serializer is not the same

[marceliwac]

@marceliwac Could you create a minimal reproduction example code in Python so that we can investigate the issue?

@icebob , I should (hopefully) be able to go one better with the PR above. Understandably, the added tests only look at the logger usage, so their usefulness might be limited once this issue is resolved. Until then, they should hopefully help with debugging / help highlight the issue.

Overall, channels cannot parse the payload if the serializer is not the same

To add to this, it depends on how the serializers handle data. For example, msgPack and JSON are incompatible, but msgPack can deserialize a JSON message (incorrectly, but without throwing an error). See this test and its result below:

Integration tests
  › Test multiple serializers with NATS adapter
    › should not call the handler if the serializers are mismatched

    Expected: {"other": "JSON", "test": 123}, Anything
    Received: 123, ...

[icebob]

I think it's not a common use-case using different serialized payloads for the same topic.

@marceliwac could you tell us more about your use-case? Are you using the same topic with different serialized payloads? Or different topics but with the same Channels middleware?

[marceliwac]

@icebob I was actually using an entirely separate Python program and used moleculer-channels as interoperability layer, which is how I ran into the serialiser problems.

While I don't expect people to be mixing serialisers (or use moleculer-channels in a similar workflow) too frequently, I think that it could be more robust in how it handles different serialisers, or at least provide clearer error messages.

[marceliwac]

@icebob Is there anything that I could do to resolve this issue? Would you like me to create a PR that displays a dedicated error message if the deserialisation fails (rather than letting the transporter deliver it multiple times and throwing the redelivery error message)?