From 2dcab24f8ceec4b586fbd9fe998cf070bf0f3b67 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Fri, 23 Nov 2018 16:53:32 +0100 Subject: [PATCH 1/4] handle mail send error gracefully log the error in case a notification mail of a new share couldn't be send to the recipient and finish the share operation successfully Signed-off-by: Bjoern Schiessle --- lib/private/Share20/Manager.php | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 9497b2c263738..48776b5498830 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -714,7 +714,7 @@ public function createShare(\OCP\Share\IShare $share) { * @param string $initiator user ID of share sender * @param string $shareWith email address of share receiver * @param \DateTime|null $expiration - * @throws \Exception If mail couldn't be sent + * @return bool */ protected function sendMailNotification(IL10N $l, $filename, @@ -773,7 +773,18 @@ protected function sendMailNotification(IL10N $l, } $message->useTemplate($emailTemplate); - $this->mailer->send($message); + try { + $failedRecipients = $this->mailer->send($message); + if(!empty($failedRecipients)) { + $this->logger->error('Share notification mail could not be send to: ' . implode(', ', $failedRecipients)); + return false; + } + } catch (\Exception $e) { + $this->logger->error('Share notification mail could not be send: ' . $e->getMessage()); + return false; + } + + return true; } /** From 849ad5a8bb209969f33867a813c4c8e557050519 Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Wed, 28 Nov 2018 19:49:33 +0100 Subject: [PATCH 2/4] log full exception Signed-off-by: Bjoern Schiessle --- lib/private/Share20/Manager.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 48776b5498830..753b83ae2cd34 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -780,7 +780,7 @@ protected function sendMailNotification(IL10N $l, return false; } } catch (\Exception $e) { - $this->logger->error('Share notification mail could not be send: ' . $e->getMessage()); + $this->logger->logException($e, ['message' => 'Share notification mail could not be send']); return false; } From 41de36b5d82333e29a81f7d9b9e00d4e1227015c Mon Sep 17 00:00:00 2001 From: Bjoern Schiessle Date: Tue, 4 Dec 2018 19:33:30 +0100 Subject: [PATCH 3/4] fix typo Signed-off-by: Bjoern Schiessle --- lib/private/Share20/Manager.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 753b83ae2cd34..86865702a7386 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -776,11 +776,11 @@ protected function sendMailNotification(IL10N $l, try { $failedRecipients = $this->mailer->send($message); if(!empty($failedRecipients)) { - $this->logger->error('Share notification mail could not be send to: ' . implode(', ', $failedRecipients)); + $this->logger->error('Share notification mail could not be sent to: ' . implode(', ', $failedRecipients)); return false; } } catch (\Exception $e) { - $this->logger->logException($e, ['message' => 'Share notification mail could not be send']); + $this->logger->logException($e, ['message' => 'Share notification mail could not be sent']); return false; } From a7c897b4458d64cd64a5aded471760119f0dd8c1 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Wed, 30 Jan 2019 15:32:19 +0100 Subject: [PATCH 4/4] Fix typos and unused return values Signed-off-by: Christoph Wurst --- lib/private/Share20/Manager.php | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/lib/private/Share20/Manager.php b/lib/private/Share20/Manager.php index 86865702a7386..57b1db155d98b 100644 --- a/lib/private/Share20/Manager.php +++ b/lib/private/Share20/Manager.php @@ -692,15 +692,15 @@ public function createShare(\OCP\Share\IShare $share) { $emailAddress, $share->getExpirationDate() ); - $this->logger->debug('Send share notification to ' . $emailAddress . ' for share with ID ' . $share->getId(), ['app' => 'share']); + $this->logger->debug('Sent share notification to ' . $emailAddress . ' for share with ID ' . $share->getId(), ['app' => 'share']); } else { - $this->logger->debug('Share notification not send to ' . $share->getSharedWith() . ' because email address is not set.', ['app' => 'share']); + $this->logger->debug('Share notification not sent to ' . $share->getSharedWith() . ' because email address is not set.', ['app' => 'share']); } } else { - $this->logger->debug('Share notification not send to ' . $share->getSharedWith() . ' because user could not be found.', ['app' => 'share']); + $this->logger->debug('Share notification not sent to ' . $share->getSharedWith() . ' because user could not be found.', ['app' => 'share']); } } else { - $this->logger->debug('Share notification not send because mailsend is false.', ['app' => 'share']); + $this->logger->debug('Share notification not sent because mailsend is false.', ['app' => 'share']); } } @@ -708,13 +708,16 @@ public function createShare(\OCP\Share\IShare $share) { } /** + * Send mail notifications + * + * This method will catch and log mail transmission errors + * * @param IL10N $l Language of the recipient * @param string $filename file/folder name * @param string $link link to the file/folder * @param string $initiator user ID of share sender * @param string $shareWith email address of share receiver * @param \DateTime|null $expiration - * @return bool */ protected function sendMailNotification(IL10N $l, $filename, @@ -777,14 +780,11 @@ protected function sendMailNotification(IL10N $l, $failedRecipients = $this->mailer->send($message); if(!empty($failedRecipients)) { $this->logger->error('Share notification mail could not be sent to: ' . implode(', ', $failedRecipients)); - return false; + return; } } catch (\Exception $e) { $this->logger->logException($e, ['message' => 'Share notification mail could not be sent']); - return false; } - - return true; } /**