From f7052aa998a31d679e67c702311cafb0513518aa Mon Sep 17 00:00:00 2001 From: Steve Bauman Date: Tue, 20 May 2025 17:36:21 -0400 Subject: [PATCH 1/5] Fix infinite loop potential and fire event when attempts exceeded --- src/Commands/HandleMessageReceived.php | 8 +++++++- src/Commands/WatchMailbox.php | 12 ++++++++---- src/Events/MailboxWatchAttemptsExceeded.php | 18 ++++++++++++++++++ 3 files changed, 33 insertions(+), 5 deletions(-) create mode 100644 src/Events/MailboxWatchAttemptsExceeded.php diff --git a/src/Commands/HandleMessageReceived.php b/src/Commands/HandleMessageReceived.php index 8a1c5ac..e7e217e 100644 --- a/src/Commands/HandleMessageReceived.php +++ b/src/Commands/HandleMessageReceived.php @@ -12,7 +12,9 @@ class HandleMessageReceived * Constructor. */ public function __construct( - protected WatchMailbox $command + protected WatchMailbox $command, + protected int &$attempts, + protected ?int &$lastReceivedAt = null, ) {} /** @@ -24,6 +26,10 @@ public function __invoke(MessageInterface $message): void "Message received: [{$message->uid()}]" ); + $this->attempts = 0; + + $this->lastReceivedAt = time(); + Event::dispatch(new MessageReceived($message)); } } diff --git a/src/Commands/WatchMailbox.php b/src/Commands/WatchMailbox.php index 67471d4..8f3edaa 100644 --- a/src/Commands/WatchMailbox.php +++ b/src/Commands/WatchMailbox.php @@ -3,12 +3,14 @@ namespace DirectoryTree\ImapEngine\Laravel\Commands; use DirectoryTree\ImapEngine\FolderInterface; +use DirectoryTree\ImapEngine\Laravel\Events\MailboxWatchAttemptsExceeded; use DirectoryTree\ImapEngine\Laravel\Facades\Imap; use DirectoryTree\ImapEngine\Laravel\Support\LoopInterface; use DirectoryTree\ImapEngine\MailboxInterface; use DirectoryTree\ImapEngine\Message; use Exception; use Illuminate\Console\Command; +use Illuminate\Support\Facades\Event; use Illuminate\Support\Str; class WatchMailbox extends Command @@ -40,14 +42,14 @@ public function handle(LoopInterface $loop): void $attempts = 0; - $loop->run(function () use ($mailbox, $with, &$attempts) { + $lastReceivedAt = null; + + $loop->run(function () use ($mailbox, $name, $with, &$attempts, &$lastReceivedAt) { try { $folder = $this->folder($mailbox); - $attempts = 0; - $folder->idle( - new HandleMessageReceived($this), + new HandleMessageReceived($this, $attempts, $lastReceivedAt), new ConfigureIdleQuery($with), $this->option('timeout') ); @@ -65,6 +67,8 @@ public function handle(LoopInterface $loop): void if ($attempts >= $this->option('attempts')) { $this->info("Exception: {$e->getMessage()}"); + Event::dispatch(new MailboxWatchAttemptsExceeded($name, $e, $lastReceivedAt)); + throw $e; } diff --git a/src/Events/MailboxWatchAttemptsExceeded.php b/src/Events/MailboxWatchAttemptsExceeded.php new file mode 100644 index 0000000..c34595c --- /dev/null +++ b/src/Events/MailboxWatchAttemptsExceeded.php @@ -0,0 +1,18 @@ + Date: Tue, 20 May 2025 17:36:24 -0400 Subject: [PATCH 2/5] Add test --- tests/Commands/WatchMailboxTest.php | 38 +++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/tests/Commands/WatchMailboxTest.php b/tests/Commands/WatchMailboxTest.php index a2e166f..2ccfe37 100644 --- a/tests/Commands/WatchMailboxTest.php +++ b/tests/Commands/WatchMailboxTest.php @@ -3,6 +3,7 @@ namespace DirectoryTree\ImapEngine\Laravel\Tests; use DirectoryTree\ImapEngine\Laravel\Commands\WatchMailbox; +use DirectoryTree\ImapEngine\Laravel\Events\MailboxWatchAttemptsExceeded; use DirectoryTree\ImapEngine\Laravel\Events\MessageReceived; use DirectoryTree\ImapEngine\Laravel\Facades\Imap; use DirectoryTree\ImapEngine\Laravel\Support\LoopFake; @@ -13,6 +14,7 @@ use Illuminate\Support\Facades\Config; use Illuminate\Support\Facades\Event; use InvalidArgumentException; +use RuntimeException; use function Pest\Laravel\artisan; @@ -48,3 +50,39 @@ fn (MessageReceived $event) => $event->message->is($message) ); }); + +it('dispatches event when failure attempts have been reached', function () { + Config::set('imap.mailboxes.test', [ + 'host' => 'localhost', + 'port' => 993, + 'encryption' => 'ssl', + 'username' => '', + 'password' => '', + ]); + + Imap::fake('test', folders: [ + new class('inbox') extends FakeFolder + { + public function idle( + callable $callback, + ?callable $query = null, + int $timeout = 300 + ): void { + throw new RuntimeException('Simulated exception'); + } + }, + ]); + + Event::fake(); + + try { + artisan(WatchMailbox::class, [ + 'mailbox' => 'test', + '--attempts' => 5, + ]); + } catch (RuntimeException) { + // Do nothing. + } + + Event::assertDispatched(MailboxWatchAttemptsExceeded::class); +}); From 349a9e2c1018251dacbf4c979f9fbfa9eab3074b Mon Sep 17 00:00:00 2001 From: Steve Bauman Date: Tue, 20 May 2025 17:39:28 -0400 Subject: [PATCH 3/5] Fix type --- src/Commands/HandleMessageReceived.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/Commands/HandleMessageReceived.php b/src/Commands/HandleMessageReceived.php index e7e217e..6e32286 100644 --- a/src/Commands/HandleMessageReceived.php +++ b/src/Commands/HandleMessageReceived.php @@ -4,6 +4,8 @@ use DirectoryTree\ImapEngine\Laravel\Events\MessageReceived; use DirectoryTree\ImapEngine\MessageInterface; +use Illuminate\Support\Carbon; +use Illuminate\Support\Facades\Date; use Illuminate\Support\Facades\Event; class HandleMessageReceived @@ -14,7 +16,7 @@ class HandleMessageReceived public function __construct( protected WatchMailbox $command, protected int &$attempts, - protected ?int &$lastReceivedAt = null, + protected ?Carbon &$lastReceivedAt = null, ) {} /** @@ -28,7 +30,7 @@ public function __invoke(MessageInterface $message): void $this->attempts = 0; - $this->lastReceivedAt = time(); + $this->lastReceivedAt = Date::now(); Event::dispatch(new MessageReceived($message)); } From 726446c43ec1b67791e4b502228b5f275f0853f2 Mon Sep 17 00:00:00 2001 From: Steve Bauman Date: Tue, 20 May 2025 17:39:34 -0400 Subject: [PATCH 4/5] Add more assertions --- tests/Commands/WatchMailboxTest.php | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/Commands/WatchMailboxTest.php b/tests/Commands/WatchMailboxTest.php index 2ccfe37..48be9a8 100644 --- a/tests/Commands/WatchMailboxTest.php +++ b/tests/Commands/WatchMailboxTest.php @@ -17,6 +17,7 @@ use RuntimeException; use function Pest\Laravel\artisan; +use function Pest\Laravel\freezeTime; it('throws exception when mailbox is not configured', function () { artisan(WatchMailbox::class, ['mailbox' => 'invalid']); @@ -52,6 +53,8 @@ }); it('dispatches event when failure attempts have been reached', function () { + $datetime = freezeTime(); + Config::set('imap.mailboxes.test', [ 'host' => 'localhost', 'port' => 993, @@ -84,5 +87,9 @@ public function idle( // Do nothing. } - Event::assertDispatched(MailboxWatchAttemptsExceeded::class); + Event::assertDispatched(function (MailboxWatchAttemptsExceeded $event) { + return $event->mailbox === 'test' + && is_null($event->lastReceivedAt) + && $event->exception->getMessage() === 'Simulated exception'; + }); }); From 00a1269d27e8487f3600226bfb5428bb58b5b733 Mon Sep 17 00:00:00 2001 From: Steve Bauman Date: Tue, 20 May 2025 17:42:53 -0400 Subject: [PATCH 5/5] Provide number of attempts in event --- src/Commands/HandleMessageReceived.php | 2 +- src/Commands/WatchMailbox.php | 4 +++- src/Events/MailboxWatchAttemptsExceeded.php | 1 + tests/Commands/WatchMailboxTest.php | 6 ++---- 4 files changed, 7 insertions(+), 6 deletions(-) diff --git a/src/Commands/HandleMessageReceived.php b/src/Commands/HandleMessageReceived.php index 6e32286..b4c98c2 100644 --- a/src/Commands/HandleMessageReceived.php +++ b/src/Commands/HandleMessageReceived.php @@ -15,7 +15,7 @@ class HandleMessageReceived */ public function __construct( protected WatchMailbox $command, - protected int &$attempts, + protected int &$attempts = 0, protected ?Carbon &$lastReceivedAt = null, ) {} diff --git a/src/Commands/WatchMailbox.php b/src/Commands/WatchMailbox.php index 8f3edaa..45a1dac 100644 --- a/src/Commands/WatchMailbox.php +++ b/src/Commands/WatchMailbox.php @@ -67,7 +67,9 @@ public function handle(LoopInterface $loop): void if ($attempts >= $this->option('attempts')) { $this->info("Exception: {$e->getMessage()}"); - Event::dispatch(new MailboxWatchAttemptsExceeded($name, $e, $lastReceivedAt)); + Event::dispatch( + new MailboxWatchAttemptsExceeded($name, $attempts, $e, $lastReceivedAt) + ); throw $e; } diff --git a/src/Events/MailboxWatchAttemptsExceeded.php b/src/Events/MailboxWatchAttemptsExceeded.php index c34595c..5c61262 100644 --- a/src/Events/MailboxWatchAttemptsExceeded.php +++ b/src/Events/MailboxWatchAttemptsExceeded.php @@ -12,6 +12,7 @@ class MailboxWatchAttemptsExceeded */ public function __construct( public string $mailbox, + public int $attempts, public Exception $exception, public ?Carbon $lastReceivedAt = null, ) {} diff --git a/tests/Commands/WatchMailboxTest.php b/tests/Commands/WatchMailboxTest.php index 48be9a8..0c336f0 100644 --- a/tests/Commands/WatchMailboxTest.php +++ b/tests/Commands/WatchMailboxTest.php @@ -17,7 +17,6 @@ use RuntimeException; use function Pest\Laravel\artisan; -use function Pest\Laravel\freezeTime; it('throws exception when mailbox is not configured', function () { artisan(WatchMailbox::class, ['mailbox' => 'invalid']); @@ -53,8 +52,6 @@ }); it('dispatches event when failure attempts have been reached', function () { - $datetime = freezeTime(); - Config::set('imap.mailboxes.test', [ 'host' => 'localhost', 'port' => 993, @@ -88,7 +85,8 @@ public function idle( } Event::assertDispatched(function (MailboxWatchAttemptsExceeded $event) { - return $event->mailbox === 'test' + return $event->attempts === 5 + && $event->mailbox === 'test' && is_null($event->lastReceivedAt) && $event->exception->getMessage() === 'Simulated exception'; });