From 4bfd4534bf71eea61915e031a3f25cff8ee4a34c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Thomas=20M=C3=BCller?=
 <1005065+DeepDiver1975@users.noreply.github.com>
Date: Fri, 6 Oct 2023 09:59:52 +0200
Subject: [PATCH] fix: Mailer::send will always thrown an exception in case of
 errors during delivery

---
 apps/dav/lib/CalDAV/Schedule/IMipPlugin.php   |  6 +-----
 .../unit/CalDAV/Schedule/IMipPluginTest.php   | 10 ----------
 lib/private/Mail/Mailer.php                   |  7 ++++---
 .../Controller/MailSettingsController.php     | 19 +++++--------------
 4 files changed, 10 insertions(+), 32 deletions(-)

diff --git a/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php b/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php
index f444696f9f3b..4fcbc95e535e 100644
--- a/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php
+++ b/apps/dav/lib/CalDAV/Schedule/IMipPlugin.php
@@ -116,12 +116,8 @@ public function schedule(ITip\Message $iTipMessage) {
 			->setSubject($subject)
 			->attach($iTipMessage->message->serialize(), "event.ics", $contentType);
 		try {
-			$failed = $this->mailer->send($message);
+			$this->mailer->send($message);
 			$iTipMessage->scheduleStatus = '1.1; Scheduling message is sent via iMip';
-			if ($failed) {
-				$this->logger->error('Unable to deliver message to {failed}', ['app' => 'dav', 'failed' =>  \implode(', ', $failed)]);
-				$iTipMessage->scheduleStatus = '5.0; EMail delivery failed';
-			}
 		} catch (\Exception $ex) {
 			$this->logger->logException($ex, ['app' => 'dav']);
 			$iTipMessage->scheduleStatus = '5.0; EMail delivery failed';
diff --git a/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php b/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php
index 85540fe29656..534cd97253e0 100644
--- a/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php
+++ b/apps/dav/tests/unit/CalDAV/Schedule/IMipPluginTest.php
@@ -82,16 +82,6 @@ public function testFailedDeliveryWithException(): void {
 		$this->assertIMipState($message, '5.0', 'REQUEST', 'Fellowship meeting');
 	}
 
-	public function testFailedDelivery(): void {
-		$this->mailer->method('send')->willReturn(['foo@example.net']);
-		$this->logger->expects(self::once())->method('error')->with('Unable to deliver message to {failed}', ['app' => 'dav', 'failed' => 'foo@example.net']);
-
-		$message = $this->buildIMIPMessage('REQUEST');
-
-		$this->plugin->schedule($message);
-		$this->assertIMipState($message, '5.0', 'REQUEST', 'Fellowship meeting');
-	}
-
 	public function testDeliveryOfCancel(): void {
 		$this->mailer->expects($this->once())->method('send');
 
diff --git a/lib/private/Mail/Mailer.php b/lib/private/Mail/Mailer.php
index ef95359977d7..8a5f3eed175c 100644
--- a/lib/private/Mail/Mailer.php
+++ b/lib/private/Mail/Mailer.php
@@ -97,8 +97,6 @@ public function send(Message $message): array {
 		try {
 			$this->getInstance($logger ?? null)->send($message->getMessage());
 		} catch (TransportExceptionInterface $e) {
-			$this->logger->logException($e);
-
 			# in case of exception it is expected that none of the mails has been sent
 			$failedRecipients = [];
 
@@ -111,7 +109,10 @@ public function send(Message $message): array {
 				}
 			});
 
-			return $failedRecipients;
+			$this->logger->logException($e, ['failed-recipients' => $recipients]);
+
+			# list of failed recipients is not added by intention to not accidentally disclose private data
+			throw new \RuntimeException("Failed to deliver email", 0, $e);
 		}
 
 		$allRecipients = [];
diff --git a/settings/Controller/MailSettingsController.php b/settings/Controller/MailSettingsController.php
index 2d1743fa8a7d..9f89b3e311d2 100644
--- a/settings/Controller/MailSettingsController.php
+++ b/settings/Controller/MailSettingsController.php
@@ -175,21 +175,12 @@ public function sendTestMail() {
 			$message->setFrom([$this->defaultMailAddress]);
 			$message->setSubject($this->l10n->t('test email settings'));
 			$message->setPlainBody('If you received this email, the settings seem to be correct.');
-			$failed = $this->mailer->send($message);
-			if (empty($failed)) {
-				return ['data' =>
-					['message' =>
-						(string) $this->l10n->t('Email sent')
-					],
-					'status' => 'success'
-				];
-			}
-
-			return [
-				'data' => [
-					'message' => (string) $this->l10n->t('A problem occurred while sending the email. Please revise your settings. (Error: %s)', ['not delivered']),
+			$this->mailer->send($message);
+			return ['data' =>
+				['message' =>
+					(string) $this->l10n->t('Email sent')
 				],
-				'status' => 'error',
+				'status' => 'success'
 			];
 		} catch (\Exception $e) {
 			return [