From 47a77eabf2d85152f370d2daa15a70b5fc7d57d7 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 10 Oct 2023 15:06:08 -0700 Subject: [PATCH 1/6] Avoid logging error messages for webhook request failures --- app/Listeners/CheckoutableListener.php | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/app/Listeners/CheckoutableListener.php b/app/Listeners/CheckoutableListener.php index d9680f82a14a..13579ce6b66b 100644 --- a/app/Listeners/CheckoutableListener.php +++ b/app/Listeners/CheckoutableListener.php @@ -18,6 +18,7 @@ use App\Notifications\CheckoutAssetNotification; use App\Notifications\CheckoutConsumableNotification; use App\Notifications\CheckoutLicenseSeatNotification; +use GuzzleHttp\Exception\ClientException; use Illuminate\Support\Facades\Notification; use Exception; use Log; @@ -61,6 +62,16 @@ public function onCheckedOut($event) ); } } catch (Exception $e) { + if ($e instanceof ClientException){ + $statusCode = $e->getResponse()->getStatusCode(); + // If status code is in 400 range, we don't want to log it as an error + // @todo: 300 and 500 as well? + if ($statusCode >= 400 && $statusCode < 500) { + Log::debug("Exception caught during checkout notification: ".$e->getMessage()); + return; + } + } + Log::error("Exception caught during checkout notification: ".$e->getMessage()); } } From ab3a3de59b64cd837f6d824f086a5749cdd60bf3 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 10 Oct 2023 15:13:46 -0700 Subject: [PATCH 2/6] Fire webhook notification after sending emails --- app/Listeners/CheckoutableListener.php | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/app/Listeners/CheckoutableListener.php b/app/Listeners/CheckoutableListener.php index 13579ce6b66b..917a0377030b 100644 --- a/app/Listeners/CheckoutableListener.php +++ b/app/Listeners/CheckoutableListener.php @@ -45,11 +45,6 @@ public function onCheckedOut($event) $acceptance = $this->getCheckoutAcceptance($event); try { - if ($this->shouldSendWebhookNotification()) { - Notification::route('slack', Setting::getSettings()->webhook_endpoint) - ->notify($this->getCheckoutNotification($event)); - } - if (! $event->checkedOutTo->locale) { Notification::locale(Setting::getSettings()->locale)->send( $this->getNotifiables($event), @@ -61,6 +56,11 @@ public function onCheckedOut($event) $this->getCheckoutNotification($event, $acceptance) ); } + + if ($this->shouldSendWebhookNotification()) { + Notification::route('slack', Setting::getSettings()->webhook_endpoint) + ->notify($this->getCheckoutNotification($event)); + } } catch (Exception $e) { if ($e instanceof ClientException){ $statusCode = $e->getResponse()->getStatusCode(); From 2a29566458521bf7a56b2a8ab2845268ac4590da Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 10 Oct 2023 15:15:02 -0700 Subject: [PATCH 3/6] Catch all ClientExceptions on check out --- app/Listeners/CheckoutableListener.php | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/app/Listeners/CheckoutableListener.php b/app/Listeners/CheckoutableListener.php index 917a0377030b..01e5041f674f 100644 --- a/app/Listeners/CheckoutableListener.php +++ b/app/Listeners/CheckoutableListener.php @@ -63,13 +63,8 @@ public function onCheckedOut($event) } } catch (Exception $e) { if ($e instanceof ClientException){ - $statusCode = $e->getResponse()->getStatusCode(); - // If status code is in 400 range, we don't want to log it as an error - // @todo: 300 and 500 as well? - if ($statusCode >= 400 && $statusCode < 500) { - Log::debug("Exception caught during checkout notification: ".$e->getMessage()); - return; - } + Log::debug("Exception caught during checkout notification: ".$e->getMessage()); + return; } Log::error("Exception caught during checkout notification: ".$e->getMessage()); From 9ef598d07bdaf6565ec79cf7078697e1634e56be Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 10 Oct 2023 15:16:12 -0700 Subject: [PATCH 4/6] Apply changes to exception handling for check outs to check ins --- app/Listeners/CheckoutableListener.php | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/app/Listeners/CheckoutableListener.php b/app/Listeners/CheckoutableListener.php index 01e5041f674f..34d93550eda6 100644 --- a/app/Listeners/CheckoutableListener.php +++ b/app/Listeners/CheckoutableListener.php @@ -98,11 +98,6 @@ public function onCheckedIn($event) } try { - if ($this->shouldSendWebhookNotification()) { - Notification::route('slack', Setting::getSettings()->webhook_endpoint) - ->notify($this->getCheckinNotification($event)); - } - // Use default locale if (! $event->checkedOutTo->locale) { Notification::locale(Setting::getSettings()->locale)->send( @@ -115,7 +110,17 @@ public function onCheckedIn($event) $this->getCheckinNotification($event) ); } + + if ($this->shouldSendWebhookNotification()) { + Notification::route('slack', Setting::getSettings()->webhook_endpoint) + ->notify($this->getCheckinNotification($event)); + } } catch (Exception $e) { + if ($e instanceof ClientException){ + Log::debug("Exception caught during checkout notification: ".$e->getMessage()); + return; + } + Log::error("Exception caught during checkin notification: ".$e->getMessage()); } } From dae9e6d09659a811c1c1d93548f3b501ff1deaf1 Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 10 Oct 2023 15:18:37 -0700 Subject: [PATCH 5/6] Improve try catch blocks --- app/Listeners/CheckoutableListener.php | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/app/Listeners/CheckoutableListener.php b/app/Listeners/CheckoutableListener.php index 34d93550eda6..ec51130b1a1d 100644 --- a/app/Listeners/CheckoutableListener.php +++ b/app/Listeners/CheckoutableListener.php @@ -42,7 +42,7 @@ public function onCheckedOut($event) /** * Make a checkout acceptance and attach it in the notification */ - $acceptance = $this->getCheckoutAcceptance($event); + $acceptance = $this->getCheckoutAcceptance($event); try { if (! $event->checkedOutTo->locale) { @@ -61,12 +61,11 @@ public function onCheckedOut($event) Notification::route('slack', Setting::getSettings()->webhook_endpoint) ->notify($this->getCheckoutNotification($event)); } - } catch (Exception $e) { - if ($e instanceof ClientException){ - Log::debug("Exception caught during checkout notification: ".$e->getMessage()); - return; - } - + } + catch (ClientException $e){ + Log::debug("Exception caught during checkout notification: ".$e->getMessage()); + } + catch (Exception $e) { Log::error("Exception caught during checkout notification: ".$e->getMessage()); } } @@ -115,12 +114,11 @@ public function onCheckedIn($event) Notification::route('slack', Setting::getSettings()->webhook_endpoint) ->notify($this->getCheckinNotification($event)); } - } catch (Exception $e) { - if ($e instanceof ClientException){ - Log::debug("Exception caught during checkout notification: ".$e->getMessage()); - return; - } - + } + catch (ClientException $e){ + Log::debug("Exception caught during checkout notification: ".$e->getMessage()); + } + catch (Exception $e) { Log::error("Exception caught during checkin notification: ".$e->getMessage()); } } From 43b9e6401c62e8f99d46fe383b2aca1d45cbe08f Mon Sep 17 00:00:00 2001 From: Marcus Moore Date: Tue, 10 Oct 2023 15:18:55 -0700 Subject: [PATCH 6/6] Formatting --- app/Listeners/CheckoutableListener.php | 20 ++++++++------------ 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/app/Listeners/CheckoutableListener.php b/app/Listeners/CheckoutableListener.php index ec51130b1a1d..17a8a6c1bf5e 100644 --- a/app/Listeners/CheckoutableListener.php +++ b/app/Listeners/CheckoutableListener.php @@ -61,12 +61,10 @@ public function onCheckedOut($event) Notification::route('slack', Setting::getSettings()->webhook_endpoint) ->notify($this->getCheckoutNotification($event)); } - } - catch (ClientException $e){ - Log::debug("Exception caught during checkout notification: ".$e->getMessage()); - } - catch (Exception $e) { - Log::error("Exception caught during checkout notification: ".$e->getMessage()); + } catch (ClientException $e) { + Log::debug("Exception caught during checkout notification: " . $e->getMessage()); + } catch (Exception $e) { + Log::error("Exception caught during checkout notification: " . $e->getMessage()); } } @@ -114,12 +112,10 @@ public function onCheckedIn($event) Notification::route('slack', Setting::getSettings()->webhook_endpoint) ->notify($this->getCheckinNotification($event)); } - } - catch (ClientException $e){ - Log::debug("Exception caught during checkout notification: ".$e->getMessage()); - } - catch (Exception $e) { - Log::error("Exception caught during checkin notification: ".$e->getMessage()); + } catch (ClientException $e) { + Log::debug("Exception caught during checkout notification: " . $e->getMessage()); + } catch (Exception $e) { + Log::error("Exception caught during checkin notification: " . $e->getMessage()); } }