Merge pull request #16050 from Godmartinz/inactive-slack-hook

Adds try/catch around notification failing with an inactive webhook for better user experience
snipe · Jan 10, 2025 · 224f04d · 224f04d
2 parents f0bcf78 + 001ccf1
commit 224f04d
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 33 deletions.
diff --git a/app/Listeners/CheckoutableListener.php b/app/Listeners/CheckoutableListener.php
@@ -46,7 +46,7 @@ class CheckoutableListener
      */
     public function onCheckedOut($event)
     {
-        if ($this->shouldNotSendAnyNotifications($event->checkoutable)){
+        if ($this->shouldNotSendAnyNotifications($event->checkoutable)) {
             return;
         }
 
@@ -57,15 +57,15 @@ public function onCheckedOut($event)
         $acceptance = $this->getCheckoutAcceptance($event);
         $adminCcEmailsArray = [];
 
-        if($settings->admin_cc_email !== '') {
+        if ($settings->admin_cc_email !== '') {
             $adminCcEmail = $settings->admin_cc_email;
             $adminCcEmailsArray = array_map('trim', explode(',', $adminCcEmail));
         }
         $ccEmails = array_filter($adminCcEmailsArray);
         $mailable = $this->getCheckoutMailType($event, $acceptance);
         $notifiable = $this->getNotifiables($event);
 
-        if  ($event->checkedOutTo->locale){
+        if ($event->checkedOutTo->locale) {
             $mailable->locale($event->checkedOutTo->locale);
         }
         // Send email notifications
@@ -77,41 +77,50 @@ public function onCheckedOut($event)
              * 3. The item should send an email at check-in/check-out
              */
 
-                if ($event->checkoutable->requireAcceptance() || $event->checkoutable->getEula() ||
-                    $this->checkoutableShouldSendEmail($event)) {
-                    Log::info('Sending checkout email, Locale: ' . ($event->checkedOutTo->locale ?? 'default'));
-                    if (!empty($notifiable)) {
-                        Mail::to($notifiable)->cc($ccEmails)->send($mailable);
-                    } elseif (!empty($ccEmails)) {
-                        Mail::cc($ccEmails)->send($mailable);
-                    }
-                    Log::info('Checkout Mail sent.');
+            if ($event->checkoutable->requireAcceptance() || $event->checkoutable->getEula() ||
+                $this->checkoutableShouldSendEmail($event)) {
+                Log::info('Sending checkout email, Locale: ' . ($event->checkedOutTo->locale ?? 'default'));
+                if (!empty($notifiable)) {
+                    Mail::to($notifiable)->cc($ccEmails)->send($mailable);
+                } elseif (!empty($ccEmails)) {
+                    Mail::cc($ccEmails)->send($mailable);
                 }
+                Log::info('Checkout Mail sent.');
+            }
         } catch (ClientException $e) {
             Log::debug("Exception caught during checkout email: " . $e->getMessage());
         } catch (Exception $e) {
             Log::debug("Exception caught during checkout email: " . $e->getMessage());
         }
 //                 Send Webhook notification
-        try{
-                if ($this->shouldSendWebhookNotification()) {
-                    if ($this->newMicrosoftTeamsWebhookEnabled()) {
-                        $message = $this->getCheckoutNotification($event)->toMicrosoftTeams();
-                        $notification = new TeamsNotification(Setting::getSettings()->webhook_endpoint);
-                        $notification->success()->sendMessage($message[0], $message[1]);  // Send the message to Microsoft Teams
-                    } else {
-                        Notification::route($this->webhookSelected(), Setting::getSettings()->webhook_endpoint)
-                            ->notify($this->getCheckoutNotification($event, $acceptance));
-                    }
+        try {
+            if ($this->shouldSendWebhookNotification()) {
+                if ($this->newMicrosoftTeamsWebhookEnabled()) {
+                    $message = $this->getCheckoutNotification($event)->toMicrosoftTeams();
+                    $notification = new TeamsNotification(Setting::getSettings()->webhook_endpoint);
+                    $notification->success()->sendMessage($message[0], $message[1]);  // Send the message to Microsoft Teams
+                } else {
+
+                    Notification::route($this->webhookSelected(), Setting::getSettings()->webhook_endpoint)
+                        ->notify($this->getCheckoutNotification($event, $acceptance));
                 }
+            }
         } catch (ClientException $e) {
-            Log::debug("Exception caught during checkout notification: " . $e->getMessage());
+            Log::error("ClientException caught during checkin notification: " . $e->getMessage());
+            return redirect()->back()->with('warning', ucfirst(Setting::getSettings()->webhook_selected) .trans('admin/settings/message.webhook.webhook_fail') );
         } catch (Exception $e) {
-            Log::debug("Exception caught during checkout notification: " . $e->getMessage());
+            Log::error(ucfirst(Setting::getSettings()->webhook_selected) . ' webhook notification failed:', [
+                'error' => $e->getMessage(),
+                'webhook_endpoint' => Setting::getSettings()->webhook_endpoint,
+                'event' => $event,
+            ]);
+            return redirect()->back()->with('warning', ucfirst(Setting::getSettings()->webhook_selected) . trans('admin/settings/message.webhook.webhook_fail'));
         }
     }
 
 
+
+
     /**
      * Notify the user and post to webhook about the checked in checkoutable
      */    
@@ -178,18 +187,24 @@ public function onCheckedIn($event)
         try {
             if ($this->shouldSendWebhookNotification()) {
                 if ($this->newMicrosoftTeamsWebhookEnabled()) {
-                        $message = $this->getCheckinNotification($event)->toMicrosoftTeams();
-                        $notification = new TeamsNotification(Setting::getSettings()->webhook_endpoint);
-                        $notification->success()->sendMessage($message[0], $message[1]);  // Send the message to Microsoft Teams
-                    } else {
-                        Notification::route($this->webhookSelected(), Setting::getSettings()->webhook_endpoint)
-                            ->notify($this->getCheckinNotification($event));
-                    }
+                    $message = $this->getCheckinNotification($event)->toMicrosoftTeams();
+                    $notification = new TeamsNotification(Setting::getSettings()->webhook_endpoint);
+                    $notification->success()->sendMessage($message[0], $message[1]); // Send the message to Microsoft Teams
+                } else {
+                    Notification::route($this->webhookSelected(), Setting::getSettings()->webhook_endpoint)
+                        ->notify($this->getCheckinNotification($event));
                 }
+            }
         } catch (ClientException $e) {
-            Log::warning("Exception caught during checkin notification: " . $e->getMessage());
+            Log::error("ClientException caught during checkin notification: " . $e->getMessage());
+            return redirect()->back()->with('warning', ucfirst(Setting::getSettings()->webhook_selected) .trans('admin/settings/message.webhook.webhook_fail'));
         } catch (Exception $e) {
-            Log::warning("Exception caught during checkin notification: " . $e->getMessage());
+            Log::error(ucfirst(Setting::getSettings()->webhook_selected) . ' webhook notification failed:', [
+                'error' => $e->getMessage(),
+                'webhook_endpoint' => Setting::getSettings()->webhook_endpoint,
+                'event' => $event,
+            ]);
+            return redirect()->back()->with('warning', ucfirst(Setting::getSettings()->webhook_selected) .trans('admin/settings/message.webhook.webhook_fail'));
         }
     }      
 

diff --git a/resources/lang/en-US/admin/settings/message.php b/resources/lang/en-US/admin/settings/message.php
@@ -45,5 +45,6 @@
         'error' => 'Something went wrong. :app responded with: :error_message',
         'error_redirect' => 'ERROR: 301/302 :endpoint returns a redirect. For security reasons, we don’t follow redirects. Please use the actual endpoint.',
         'error_misc' => 'Something went wrong. :( ',
+        'webhook_fail' => ' webhook notification failed: Check to make sure the URL is still valid.',
     ]
 ];