Fix MQTT 5 QoS 2 handling of failed PUBREC reason codes

[PandaAB]

Problem and root cause

Fixes #895.

When a broker rejects a QoS 2 publish, MQTT 5 returns the failure reason code in PUBREC, for example 0x87 Not authorized.

Currently, Client._handle_pubrec() unpacks this reason code but does not use it. It unconditionally moves the outgoing message to mqtt_ms_wait_for_pubcomp and sends PUBREL. Mosquitto then responds with a PUBCOMP without a reason code, causing _handle_pubackcomp("PUBCOMP") to complete the message through _do_on_publish() with an implicit Success.

As a result, the application receives on_publish(..., Success) even though the broker rejected and discarded the message.

This was reproduced against Mosquitto 2.0.22 using MQTT 5 and an ACL that only allowed publishing to allowed/#:

• QoS 1: on_publish reports Not authorized (0x87) because the PUBACK reason code reaches the callback.
• QoS 2: the broker sends PUBREC 0x87, Paho sends PUBREL, and on_publish ultimately reports Success (0x00).

The behavior was reproduced in three independent runs on master commit 26cf3e3503bfde014f7c67dd12f4bed23f74a575, using a fresh broker container and client identifier for each run.

MQTT 5 specification behavior

The MQTT 5 specification requires that:

• PUBREL is sent only when the received PUBREC reason code is below 0x80.
• A PUBREC reason code of 0x80 or greater completes the PUBLISH exchange as a failure and the PUBLISH must not be retransmitted.
• The packet identifier becomes available for reuse after receiving either PUBCOMP or a failed PUBREC.
• A PUBREC with Remaining Length 2 has an implicit Success reason code.

These requirements are defined in sections 3.5.2.1, 4.3.3, and 4.4, including requirements MQTT-4.3.3-4 and MQTT-4.4.0-2.

Fix

When _handle_pubrec() receives an MQTT 5 PUBREC with a failure reason code (>= 0x80), it now:

• does not send PUBREL;
• completes the message through the existing _do_on_publish(mid, reason_code, properties) path;
• reports the PUBREC reason code and properties through Callback API v2;
• removes the message from _out_messages;
• releases the inflight slot and packet identifier;
• wakes callers waiting in wait_for_publish().

The existing behavior is preserved for:

• PUBREC without an explicit reason code, which implies Success;
• PUBREC reason codes below 0x80;
• MQTT 3.1.1;
• unknown packet identifiers;
• duplicate PUBREC packets after the message has already completed.

The fix reuses _do_on_publish() rather than duplicating its callback and state-cleanup logic.

MessageInfo.rc retains its existing MQTTErrorCode meaning. The broker-provided failure reason remains available through on_publish, consistent with the existing QoS 1 PUBACK failure path.

Callback API compatibility

Callback API v2 continues to use:

on_publish(client, userdata, mid, reason_code, properties)

For a failed PUBREC, it receives the original PUBREC reason code and properties. When the packet contains no properties, an empty Properties(PacketTypes.PUBREC) object is provided.

Callback API v1 retains its existing three-argument signature:

on_publish(client, userdata, mid)

It cannot expose the reason code, but the callback still runs once and the message no longer remains pending.

Tests

The new tests/test_client.py::TestPubrecError coverage contains 9 test methods and 32 parametrized cases across TCP and Unix sockets. It uses the existing in-process fake broker and requires no external broker.

Coverage includes:

• all PUBREC failure reason codes defined in section 3.5.2.1: 0x80, 0x83, 0x87, 0x90, 0x91, 0x97, and 0x99;
• exact reason-code delivery to on_publish;
• suppression of PUBREL after a failed PUBREC;
• cleanup of _out_messages and _inflight_messages;
• completion of wait_for_publish();
• continued usability of the connection after the failure;
• implicit Success and explicit 0x00 PUBREC behavior;
• propagation of Reason String and User Property values;
• correct PacketTypes.PUBREC property type with both received and synthesized properties;
• release of an inflight slot when max_inflight_messages is 1;
• duplicate failed PUBREC handling;
• unknown and late packet identifiers;
• Callback API v1 compatibility;
• the existing QoS 1 PUBACK failure behavior as a reference case.

The new failure-path tests fail on unmodified master and pass with this change.

Full local test result:

177 passed, 24 skipped, 1 xfailed

The skipped tests depend on paho.mqtt.testing and are unrelated to this change.

Fault-injection checks covering the failure condition, PUBREL suppression, callback result, state cleanup, and PUBREC property type are each detected by the new tests.

Docker A/B verification

The behavior was also verified against Mosquitto 2.0.22 using MQTT 5, username/password authentication, and an ACL allowing writes only to allowed/#.

Three before/after rounds were run with a fresh broker container and client identifier each time.

Before the fix:

broker sends PUBREC rc135
client sends PUBREL
on_publish reports Success

After the fix:

broker sends PUBREC rc135
client sends no PUBREL
on_publish reports 135 Not authorized

An allowed QoS 2 publish continues to complete through PUBREC → PUBREL → PUBCOMP. The other tested scenarios—allowed QoS 0/1/2 and denied QoS 0/1—behave identically before and after the change.

Compatibility

• No public API is changed.
• Callback API v1 retains its existing signature.
• MQTT 3.1.1 and QoS 1 behavior are unchanged.
• MQTT 5 failure reporting remains consistent with the existing QoS 1 PUBACK failure path.

[Copilot]

Copilot review overview

🟡 Changes recommended

Fix the grammatical article errors in the test docstrings at lines 1073 and 1662.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1

Open (1)

• Use “An MQTT” in the docstring · New

What changed in this PR

Fixes MQTT 5 QoS 2 failure reporting when a broker rejects a publish via PUBREC.

Changes:

• Propagates failed PUBREC reason codes to callbacks.
• Suppresses PUBREL and cleans up message state.
• Adds comprehensive regression tests.

File	Summary
tests/​test_client.py	Adds PUBREC failure-path coverage. Critical: change “A MQTT” to “An MQTT” at lines 1073 and 1662 (3 votes).
src/​paho/​mqtt/​client.py	Implements failed PUBREC handling and cleanup.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[JamesParrott]

Please on your fork, go into the settings and click "enable Workflows" so everyone can see the results of the test suite

[PandaAB]

Workflows are now enabled on my fork, and the pre-commit, lint, and Python 3.9–3.14 tox runs have all completed successfully. Thanks!

[pswsm]

Not a big fan of this kind of explanation comments. Code should be self-explanatory (which in this case I think it is).

[PandaAB]

Thanks, agreed. I’ve removed the explanatory comment.

[pswsm]

missing newlines, no linter?

[PandaAB]

Good catch, thank you! I added the missing blank lines between the top-level classes and ran an explicit E302/E303 check over the changed Python files in addition to the project’s existing checks.