fix: Mailer::send will always thrown an exception in case of errors d…

…uring delivery
owncloud · Nov 13, 2023 · ad2bb6d · ad2bb6d
1 parent 0634d9f
commit ad2bb6d
Show file tree

Hide file tree

Showing 4 changed files with 10 additions and 32 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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 [