Skip to content

Commit

Permalink
Merge pull request #414 from jolicode/notification-errors
Browse files Browse the repository at this point in the history
Better handle notification errors
  • Loading branch information
lyrixx authored Apr 16, 2024
2 parents 95c1eb1 + 8b89223 commit 75d7b15
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* Allow to use imported class in task from remote import
* Do not load task from `vendor` directory
* Add `context()` function in expression language to enable a task
* Better handle notification errors and exceptions
* Deprecate `Castor\GlobalHelper` class. There are no replacements. Use raw
functions instead
* Deprecate `AfterApplicationInitializationEvent` event. Use
Expand Down
24 changes: 22 additions & 2 deletions src/Helper/Notifier.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@

namespace Castor\Helper;

use Joli\JoliNotif\Exception\InvalidNotificationException;
use Joli\JoliNotif\Notification;
use Joli\JoliNotif\Notifier as JoliNotifier;
use Joli\JoliNotif\Notifier\NullNotifier;
use Psr\Log\LoggerInterface;

class Notifier
{
public function __construct(
private JoliNotifier $notifier
private JoliNotifier $notifier,
private LoggerInterface $logger,
) {
}

Expand All @@ -19,6 +23,22 @@ public function send(string $message): void
->setBody($message)
;

$this->notifier->send($notification);
if ($this->notifier instanceof NullNotifier) {
$this->logger->warning('No supported notifier found, notification not sent.');

return;
}

try {
$success = $this->notifier->send($notification);

if (!$success) {
$this->logger->error('Failed to send notification.');
}
} catch (InvalidNotificationException $e) {
throw $e;
} catch (\Throwable $e) {
$this->logger->error('Failed to send notification: ' . $e->getMessage());
}
}
}
3 changes: 3 additions & 0 deletions tests/Helper/OutputCleaner.php
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ public static function cleanOutput(string $string): string
// Clean the warning on tasks when remote imports are disabled
$string = preg_replace('{hh:mm:ss WARNING \[castor\] Could not import "[\w:/\.-]*" in "[\w:/\.-]*" on line \d+. Reason: Remote imports are disabled\.}m', '', $string);

// Fix notification logs
$string = preg_replace('{hh:mm:ss ERROR \[castor\] Failed to send notification\.}m', '', $string);

// Avoid spacing issues
$string = ltrim($string, "\n"); // Trim output start to avoid empty lines (like after removing remote import warnings)
$string = preg_replace('/ +$/m', '', $string); // Remove trailing space
Expand Down

0 comments on commit 75d7b15

Please sign in to comment.