Skip to content

fix(microservices): handle non-json mqtt response packets gracefully - #17534

Open
tushardev-365 wants to merge 1 commit into
nestjs:masterfrom
tushardev-365:fix/mqtt-client-non-json-response
Open

fix(microservices): handle non-json mqtt response packets gracefully#17534
tushardev-365 wants to merge 1 commit into
nestjs:masterfrom
tushardev-365:fix/mqtt-client-non-json-response

Conversation

@tushardev-365

Copy link
Copy Markdown

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Other... Please describe:

What is the current behavior?

ClientMqtt.createResponseCallback parses every incoming reply with an unguarded JSON.parse:

return async (channel: string, buffer: Buffer) => {
  const packet = JSON.parse(buffer.toString());

A non-JSON payload arriving on a subscribed reply topic (a plain-text reply from a non-Nest service sharing the broker, a misconfigured publisher, a binary payload) makes the parse throw inside the async 'message' listener. That becomes an unhandled promise rejection, which crashes the process under Node's default policy, and the pending request observable never settles.

ClientMqtt is the outlier here. Every sibling transport already guards this exact parse: ClientRedis.createResponseCallback wraps it in try/catch and falls back to the raw content with a debug log, and the MQTT, Redis and RMQ servers all parse incoming messages defensively.

Issue Number: N/A

What is the new behavior?

The parse is guarded the same way ClientRedis does it: on failure, log at debug level and hand the raw content to the deserializer, so custom deserializers keep working with non-JSON formats and an unparseable stray message can no longer crash the client.

Added a spec mirroring the existing createResponseCallback cases: a non-JSON buffer must not throw and must reach the deserializer. Verification runs:

  • with the fix: 36 passed (36)
  • with the current master source and only the new spec added: 1 failed | 35 passed, the failing one being exactly the new case

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

ClientMqtt.createResponseCallback parsed every incoming reply with an
unguarded JSON.parse. A non-json payload on a subscribed reply topic threw
inside the async mqtt message listener, producing an unhandled rejection,
and the pending request observable never settled.

Every other transport already guards this parse: ClientRedis falls back to
the raw content with a debug log, and the mqtt, redis and rmq servers all
parse defensively. This applies the same guard ClientRedis uses, passing
the raw content to the deserializer.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant