Skip to content

Add publisher confirms support to Producer - #748

Open
amigian74 wants to merge 1 commit into
php-amqplib:masterfrom
amigian74:feature/publisher-confirms
Open

amigian74 wants to merge 1 commit into
php-amqplib:masterfrom
amigian74:feature/publisher-confirms

Conversation

@amigian74

Copy link
Copy Markdown

What

Adds two opt-in producer options, confirm_select and confirm_timeout, which put the producer's channel into AMQP publisher-confirm mode and make publish() wait for the broker's confirmation.

producers:
    upload_picture:
        connection:       default
        exchange_options: {name: 'upload-picture', type: direct}
        confirm_select:   true   # default false
        confirm_timeout:  10.0   # default 10.0, seconds; 0 waits indefinitely
if (!$producer->publish($body)) {
    // the broker sent basic.nack for this message
}

Why

Today publish() cannot distinguish "written to the socket" from "accepted by the broker". A broker that is out of disk, is refusing publishes, or dies mid-publish is indistinguishable from success.

Applications that need an at-least-once guarantee therefore have to reach past the bundle onto AMQPChannel directly — which means re-implementing the ack/nack handler wiring on every reconnect, and losing confirm mode silently whenever the bundle recreates the channel underneath them.

Prior art

This was proposed once before, in #487 (2017), addressing #460 and #74. It attracted repeated interest from users over three and a half years, received no maintainer review, and was auto-closed by the stale bot in April 2021 — no objection to the feature itself was ever raised.

This is a fresh implementation against current master, not a rebase of that branch.

Design notes

Confirm mode is enabled in an overridden getChannel(), keyed on channel object identity.

Confirm mode is a property of the channel, not of the producer, and the bundle replaces the channel in three situations: BaseAmqp::getChannel() recreates it once closed, BaseAmqp::setChannel() swaps it, and BaseAmqp::reconnect() reconnects underneath it. Enabling it once at construction time would silently drop the producer back to fire-and-forget on any of those, while publish() kept reporting success. Comparing the channel object (not the channel id, which is reused after a reconnect) covers all three cases in one branch.

The extension adds setConfirmSelect() / setConfirmationTimeout() only when the feature is switched on, so no existing producer service definition changes shape — the existing getMethodCalls() assertions in OldSoundRabbitMqExtensionTest needed no edits. There is a regression test pinning that.

publish() now returns bool. It previously returned nothing, and it returns true whenever confirms are disabled, so no caller can start seeing a falsy value it did not see before. $acknowledged is reset before every basic_publish(), so one nacked message cannot poison the result of the next publish.

A confirmation timeout propagates as AMQPTimeoutException rather than being folded into return false. A broker that stops confirming is an infrastructure failure, not a per-message rejection, and collapsing the two would hide it behind the same value a legitimate nack produces. confirm_timeout: 0 maps to php-amqplib's "wait indefinitely".

Note that isConfirmSelect() is added to Producer and deliberately not to ProducerInterface — adding a method to a published interface would be a fatal error for third-party implementations. Callers typed against the interface need an instanceof check. Happy to raise that as a separate issue for the next major if you think the interface should carry it.

Backwards compatibility

None affected. The feature is opt-in and defaults to off. With it off, the only observable change is publish() returning true instead of null, and the channel is not touched at all — there is a test asserting confirm_select() and wait_for_pending_acks() are never called in that case.

Tests

Adds Tests/RabbitMq/ProducerTest.php — there was no producer test before. Seven cases:

  • confirms off by default, channel untouched, publish() returns true
  • confirm_timeout defaults to 10.0
  • confirm mode enabled exactly once, awaited on every publish, with the configured timeout
  • publish() returns false on basic.nack
  • a nacked publish does not poison the following publish
  • confirm mode is re-enabled after the channel is replaced
  • a confirmation timeout surfaces as AMQPTimeoutException

Plus two extension tests: one asserting the wiring when the option is set, one asserting that producers without it get no extra method calls.

Green on the full matrix (PHP 8.2/8.3/8.4 × Symfony 6.4/7.4/8.0), php-cs-fixer clean.

Docs

README gains a "Publisher Confirms" subsection under "Producer" and two lines in the main configuration sample; CHANGELOG updated — as per the contribution guidelines.

Adds two opt-in producer options, `confirm_select` and `confirm_timeout`,
which put the producer's channel into AMQP publisher-confirm mode and make
`publish()` wait for the broker's confirmation.

    producers:
        upload_picture:
            connection:       default
            exchange_options: {name: 'upload-picture', type: direct}
            confirm_select:   true
            confirm_timeout:  10.0

Today `publish()` cannot distinguish "written to the socket" from "accepted
by the broker", so applications needing at-least-once semantics have to
reach past the bundle onto AMQPChannel — and then lose confirm mode
silently whenever the bundle recreates the channel.

Implementation notes:

  * Confirm mode is enabled in an overridden getChannel(), keyed on channel
    object identity, so it survives reconnects, channel recreation after a
    close, and explicit setChannel() calls.
  * The extension adds setConfirmSelect()/setConfirmationTimeout() only when
    the feature is switched on, so no existing producer service definition
    changes shape. confirm_timeout is cast to float at the wiring site,
    since Symfony's FloatNode accepts an int without casting it.
  * publish() now returns bool. It previously returned nothing, and returns
    true whenever confirms are disabled, so no caller can start seeing a
    falsy value it did not see before.
  * A confirmation timeout propagates as AMQPTimeoutException rather than
    being folded into `return false`, so a broken broker cannot be mistaken
    for a rejected message.

Adds Tests/RabbitMq/ProducerTest.php (there was no producer test) plus two
extension tests — one asserting the wiring when the option is set, one
asserting that producers without it get no extra method calls.

README and CHANGELOG updated.
@amigian74
amigian74 requested a review from mihaileu as a code owner September 7, 2026 09:36
@amigian74

Copy link
Copy Markdown
Author

The failing Scrutinizer check here is unrelated to this change — its build errors out while installing dependencies, before any analysis runs:

Failed to download sebastian/object-reflector from dist: Could not authenticate against github.com
Source fallback is disabled. Not trying alternative sources.
...
In AuthHelper.php line 152:
  Could not authenticate against github.com

All 83 packages fail the same way, and with source fallback disabled the build exits 100. That looks like the Scrutinizer integration's GitHub credentials having expired rather than anything in the diff.

It does not seem specific to this PR: #747 shows the same Scrutinizer: error, and the last green Scrutinizer run I can find is #745 back in March.

Separately, the GitHub Actions runs are sitting at "action_required" since this is my first contribution here — if someone gets a chance to approve them, the PR would get real CI results. The same commit is green on my fork's CI.

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