Skip to content

feat: add Messenger class for multiple adapter failover support#128

Open
deepshekhardas wants to merge 1 commit into
utopia-php:mainfrom
deepshekhardas:fix/115-messenger-failover
Open

feat: add Messenger class for multiple adapter failover support#128
deepshekhardas wants to merge 1 commit into
utopia-php:mainfrom
deepshekhardas:fix/115-messenger-failover

Conversation

@deepshekhardas

Copy link
Copy Markdown

Port of PR #115 by TorstenDittmann.

Adds Messenger class that orchestrates multiple messaging adapters with automatic failover. If one adapter fails, it falls back to the next in the list.

Changes:

  • New Messenger class with failover orchestration
  • Adapter compatibility validation (same type/message type)
  • 14 comprehensive tests
  • Updated README with usage example

Based on PR utopia-php#115 by TorstenDittmann. Adds Messenger class that orchestrates multiple adapters with automatic failover.
@greptile-apps

greptile-apps Bot commented Jun 14, 2026

Copy link
Copy Markdown

Greptile Summary

This PR introduces a Messenger class that wraps multiple Adapter instances and tries them in sequence, falling back to the next adapter whenever one throws an exception. Constructor-time validation ensures all adapters share the same type and message type.

  • Messenger::send() iterates adapters in order, catches exceptions, and re-throws a combined error only when every adapter has failed; silent (non-exception) failure payloads are intentionally passed through without triggering fallback.
  • Compatibility validation (getType / getMessageType agreement) happens at construction time, and getMaxMessagesPerRequest() returns the minimum across all adapters so consumers can pre-validate batch size.
  • 14 PHPUnit tests cover the happy path, multi-hop failover, all-failed exception formatting, type mismatches, and the explicit no-fallback-on-failure-payload behaviour.

Confidence Score: 4/5

Safe to merge; the failover logic is correct for the common exception path, with one minor gap worth addressing before the next iteration.

The send() catch block uses \Exception rather than \Throwable, so any \Error or \TypeError thrown inside an adapter escapes the failover loop entirely. In practice adapters are unlikely to throw \Error, but the gap exists and could surprise users of third-party adapters. Everything else — validation, ordering, minimum-maxMessages aggregation, and test coverage — looks correct.

src/Utopia/Messaging/Messenger.php — specifically the catch clause in send() and the single-adapter error message wording

Important Files Changed

Filename Overview
src/Utopia/Messaging/Messenger.php New Messenger orchestration class — logic is sound; catch block covers only \Exception, so \Error subclasses from adapters would bypass failover
tests/Messaging/MessengerTest.php 14 tests covering happy-path, failover chain, error cases, and type validation — comprehensive coverage with no test logic issues
README.md Adds a "Multiple Adapters (Failover)" section with a clear usage example; no issues

Reviews (1): Last reviewed commit: "feat: add Messenger class for multiple a..." | Re-trigger Greptile

Comment on lines +90 to +95
if (! \is_a($message, $messageType)) {
throw new \Exception(
'Invalid message type. Expected "'
.$messageType
.'", got "'
.\get_class($message)

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Failover skipped on \Error subclasses

The catch block only catches \Exception, so any \TypeError or other \Error thrown by an adapter will propagate immediately without trying the next adapter, defeating the purpose of failover. Changing to \Throwable would cover both \Exception and \Error hierarchies.

Comment on lines +112 to +122
$adapterCount = \count($this->adapters);
$adapterLabel = $adapterCount === 1 ? 'adapter' : 'adapters';

throw new \Exception(
'All '
.$adapterCount
.' '
.$adapterLabel
." failed:\n"
.\implode("\n", $errors)
);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 "All 1 adapter failed" reads awkwardly; use a distinct message for the single-adapter case to make logs clearer.

Suggested change
$adapterCount = \count($this->adapters);
$adapterLabel = $adapterCount === 1 ? 'adapter' : 'adapters';
throw new \Exception(
'All '
.$adapterCount
.' '
.$adapterLabel
." failed:\n"
.\implode("\n", $errors)
);
$adapterCount = \count($this->adapters);
if ($adapterCount === 1) {
throw new \Exception(
'The adapter failed: '
.\implode("\n", $errors)
);
}
throw new \Exception(
'All '
.$adapterCount
." adapters failed:\n"
.\implode("\n", $errors)
);

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

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