Skip to content

fix(eon): preserve the Last Will on graceful shutdown - #66

Merged
dpazj merged 1 commit into
masterfrom
fix/session-termination
Nov 3, 2026
Merged

dpazj merged 1 commit into
masterfrom
fix/session-termination

Conversation

@dpazj

@dpazj dpazj commented Nov 3, 2026

Copy link
Copy Markdown
Owner

Summary

Fixes the Edge Node session termination path so it no longer sends DATA after NDEATH and no longer sends a normal MQTT 5 DISCONNECT when the client can let the broker publish the Last Will.

  • Add Client::disconnect_with_will(). The default returns false, so existing client implementations keep the current behaviour.
  • Implement it for srad-client-rumqtt: it returns true without sending a DISCONNECT packet, so dropping the event loop closes the connection and the broker publishes the will.
  • In NodeHandle::cancel, stop accepting data publishes, publish the NDEATH explicitly, and skip client.disconnect() when the client handled the will path.
  • Add regression tests for the capability and the shutdown behaviour.

Why

edge:SessionTerminationTest failed with:

payloads-ndeath-will-message-publisher-disconnect-mqtt50

because rumqttc always sends NormalDisconnection (0.24, 0.25 and master hardcode it), and srad used a normal disconnect. The test also exposed a race: with the connection kept open briefly during shutdown, the data loop published NDATA after NDEATH, which the TCK monitor reports as an out-of-order sequence number.

Result

edge:SessionTerminationTest   PASS   35 assertions: 25 passed, 10 not executed
edge:SendDataTest             PASS   40 passed, 4 not executed
edge:SendComplexDataTest      PASS   48/48

The MQTT 5 disconnect-reason assertion is now NOT EXECUTED rather than FAIL. cargo fmt --all --check, cargo clippy --all-targets -- -D warnings and cargo test pass.

Trade-offs

  • rumqttc cannot send MQTT 5 Disconnect with Will Message (0x04), so this closes the connection without a DISCONNECT. The assertion stays unexecuted until rumqttc exposes reason codes; there is a TODO in the rumqtt client.
  • The NDEATH is still published explicitly because brokers (including HiveMQ) do not expose server-published wills to extensions, so the TCK and some subscribers would not see a will-only death. The broker will also publish the registered will, so subscribers receive the death certificate twice. This is unavoidable without protocol-level reason-code support and an extension API that surfaces wills.

Publish the NDEATH, stop accepting data publishes, and let clients that
support it disconnect without an MQTT DISCONNECT packet so the broker
publishes the Last Will.

This stops DATA messages arriving after NDEATH (which the TCK monitor
reports as a sequence error) and addresses the MQTT 5 intentional
disconnect requirement as far as rumqttc allows. rumqttc 0.24 cannot set
the 'Disconnect with Will Message' reason code, so the connection is
closed instead of sending a normal DISCONNECT.

- Add Client::disconnect_with_will with a default of false.
- Implement it for the rumqttc client.
- Stop data publishes before publishing the NDEATH in NodeHandle::cancel.
- Add regression tests for the trait and shutdown behaviour.
@dpazj
dpazj force-pushed the fix/session-termination branch from 547b358 to 359d2c3 Compare November 3, 2026 20:21
@dpazj
dpazj merged commit 785ebdb into master Nov 3, 2026
7 checks passed
@dpazj
dpazj deleted the fix/session-termination branch November 3, 2026 20:23
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