MQTT in node auto-detect mode should include trying to parse JSON

[dceejay]

Current Behavior

Currently the MQTT in node when in default automatic mode will only return strings or buffers (if payload is not utf8). In order to parse JSON that mode has to be manually selected.

See discussion https://discourse.nodered.org/t/improvement-of-mqtt-in-nodes/55445

Expected Behavior

Proposed behaviour rather than expected - I would propose that the automatic mode should be "fully automatic" and try to parse the incoming payload to the highest possible capability - ie JSON if possible. Then fail back to string - then back to buffer as today. Obviously the existing fixed modes can still be selected manually.

This would be more in line with what I would interpret as automatic - and doing our best to help users by default. We do get a lot of Q&A on the forum where the answer is - "turn on json parsing for your mqtt node input" - and this change would reduce that.

However it is a breaking change in behaviour - but one that I think is worth doing as we move to 3.0. Yes you could add yet another checkbox to the options but I don't feel adding more options is the right answer when the existing behaviour can be improved in line with user expectations.

I'm happy to implement if accepted.

Steps To Reproduce

No response

Example flow

paste your flow here

Environment

• Node-RED version:
• Node.js version:
• npm version:
• Platform/OS:
• Browser:

[knolleary]

Having considered this, I have concerns about making this change in behaviour to the auto mode.

To be clear - I agree that in a perfect world, auto mode would try to JSON parse. The question is whether the impact of making this change can be justified - even with a major version change (ie 3.0).

I would suggest that JSON over MQTT is a very common pattern implemented as an MQTT node wired to a JSON node - both with their default configurations. And that pattern will be broken by this change.

If we were discussing a relatively uncommon edge case, then it would be easier to justify the breakage for the greater good. But we're talking about changing the default behaviour of the MQTT node that will have a large impact in terms of the number of flows that need fixing to continue working.

I think we need to find a way to introduce this behaviour that doesn't break existing flows.

I propose we add auto-detect (JSON, string or buffer) option - and make that the default for newly added nodes. Existing nodes will continue with just the string/buffer auto-detect option.

Would could then also add a deprecation warning to the auto-detect string/buffer option - (in both UI and maybe logged on node deploy) - that encourages users to migrate to the new option. This gives them time to migrate and minimise the impact when we eventually remove the old option in 4.0.

[Steve-Mcl]

@knolleary I think this is a reasonable approach

I will begin work on this tomorrow assuming @dceejay has no objections.

[dceejay]

No objections

[Steve-Mcl]

@dceejay @knolleary Updated PR and added a comment here: https://github.com/node-red/node-red/pull/3530#issuecomment-1105730063

[knolleary]

Shipped in 3.0