Improve account setup error reporting

Co-authored-by: julia.kirschenheuter <julia.kirschenheuter@nextcloud.com>
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>

8 files changed, 322 insertions, 152 deletions

diff --git a/lib/Controller/AccountsController.php b/lib/Controller/AccountsController.php
index e86c24f74..eb5c11d19 100644
--- a/lib/Controller/AccountsController.php
+++ b/lib/Controller/AccountsController.php
@@ -35,6 +35,7 @@ use OCA\Mail\Contracts\IMailManager;
 use OCA\Mail\Contracts\IMailTransmission;
 use OCA\Mail\Db\Mailbox;
 use OCA\Mail\Exception\ClientException;
+use OCA\Mail\Exception\CouldNotConnectException;
 use OCA\Mail\Exception\ManyRecipientsException;
 use OCA\Mail\Exception\ServiceException;
 use OCA\Mail\Http\JsonResponse as MailJsonResponse;
@@ -339,28 +340,37 @@ class AccountsController extends Controller {
 	 * @throws ClientException
 	 */
 	public function create(string $accountName, string $emailAddress, string $password = null, string $imapHost = null, int $imapPort = null, string $imapSslMode = null, string $imapUser = null, string $imapPassword = null, string $smtpHost = null, int $smtpPort = null, string $smtpSslMode = null, string $smtpUser = null, string $smtpPassword = null, bool $autoDetect = true): JSONResponse {
-		$account = null;
-		$errorMessage = null;
 		try {
 			if ($autoDetect) {
 				$account = $this->setup->createNewAutoConfiguredAccount($accountName, $emailAddress, $password);
 			} else {
 				$account = $this->setup->createNewAccount($accountName, $emailAddress, $imapHost, $imapPort, $imapSslMode, $imapUser, $imapPassword, $smtpHost, $smtpPort, $smtpSslMode, $smtpUser, $smtpPassword, $this->currentUserId);
 			}
-		} catch (Exception $ex) {
-			$errorMessage = $ex->getMessage();
+		} catch (CouldNotConnectException $e) {
+			$this->logger->info('Creating account failed: ' . $e->getMessage(), [
+				'exception' => $e,
+			]);
+			return \OCA\Mail\Http\JsonResponse::fail([
+				'error' => $e->getReason(),
+				'service' => $e->getService(),
+				'host' => $e->getHost(),
+				'port' => $e->getPort(),
+			]);
+		} catch (ServiceException $e) {
+			$this->logger->error('Creating account failed: ' . $e->getMessage(), [
+				'exception' => $e,
+			]);
+			return \OCA\Mail\Http\JsonResponse::error('Could not create account');
 		}
 
 		if (is_null($account)) {
-			if ($autoDetect) {
-				throw new ClientException($this->l10n->t('Auto detect failed. Please use manual mode.'));
-			} else {
-				$this->logger->error('Creating account failed: ' . $errorMessage);
-				throw new ClientException($this->l10n->t('Creating account failed: ') . $errorMessage);
-			}
+			return \OCA\Mail\Http\JsonResponse::fail([
+				'error' => 'AUTOCONFIG_FAILED',
+				'message' => $this->l10n->t('Auto detect failed. Please use manual mode.'),
+			]);
 		}
 
-		return new JSONResponse($account, Http::STATUS_CREATED);
+		return \OCA\Mail\Http\JsonResponse::success($account, Http::STATUS_CREATED);
 	}
 
 	/**
diff --git a/lib/Controller/SieveController.php b/lib/Controller/SieveController.php
index 11fc5b186..ca1bfd4ce 100644
--- a/lib/Controller/SieveController.php
+++ b/lib/Controller/SieveController.php
@@ -179,7 +179,7 @@ class SieveController extends Controller {
 		try {
 			$this->sieveClientFactory->createClient($sieveHost, $sievePort, $sieveUser, $sievePassword, $sieveSslMode);
 		} catch (ManagesieveException $e) {
-			throw CouldNotConnectException::create($e, 'ManageSieve', $sieveHost, $sievePort);
+			throw new CouldNotConnectException($e, 'ManageSieve', $sieveHost, $sievePort);
 		}
 
 		$mailAccount->setSieveEnabled(true);
@@ -205,13 +205,13 @@ class SieveController extends Controller {
 		$account = $this->accountService->find($this->currentUserId, $id);
 
 		if (!$account->getMailAccount()->isSieveEnabled()) {
-			throw new CouldNotConnectException('ManageSieve is disabled.');
+			throw new ClientException('ManageSieve is disabled.');
 		}
 
 		try {
 			$sieve = $this->sieveClientFactory->getClient($account);
 		} catch (ManagesieveException $e) {
-			throw CouldNotConnectException::create($e, 'ManageSieve', $account->getMailAccount()->getSieveHost(), $account->getMailAccount()->getSievePort());
+			throw new CouldNotConnectException($e, 'ManageSieve', $account->getMailAccount()->getSieveHost(), $account->getMailAccount()->getSievePort());
 		}
 
 		return $sieve;
diff --git a/lib/Exception/CouldNotConnectException.php b/lib/Exception/CouldNotConnectException.php
index c996095ee..e6759e94e 100644
--- a/lib/Exception/CouldNotConnectException.php
+++ b/lib/Exception/CouldNotConnectException.php
@@ -23,14 +23,69 @@ declare(strict_types=1);
 
 namespace OCA\Mail\Exception;
 
+use Horde_Imap_Client_Exception;
 use Throwable;
 
 class CouldNotConnectException extends ServiceException {
-	public static function create(Throwable $exception, string $service, string $host, int $port): self {
-		return new self(
-			"Connection to {$service} at {$host}:{$port} failed. {$exception->getMessage()}",
-			(int)$exception->getCode(),
-			$exception
+
+	/** @var string */
+	private $service;
+
+	/** @var string */
+	private $host;
+
+	/** @var int */
+	private $port;
+
+	/** @var Throwable */
+	private $previous;
+
+	public function __construct(Throwable $previous, string $service, string $host, int $port) {
+		parent::__construct(
+			"Connection to {$service} at {$host}:{$port} failed. {$previous->getMessage()}",
+			(int)$previous->getCode(),
+			$previous
 		);
+		$this->service = $service;
+		$this->host = $host;
+		$this->port = $port;
+		$this->previous = $previous;
+	}
+
+	/**
+	 * @return string
+	 */
+	public function getService(): string {
+		return $this->service;
+	}
+
+	/**
+	 * @return string
+	 */
+	public function getHost(): string {
+		return $this->host;
+	}
+
+	/**
+	 * @return int
+	 */
+	public function getPort(): int {
+		return $this->port;
+	}
+
+	public function getReason(): string {
+		if (!($this->previous instanceof Horde_Imap_Client_Exception)) {
+			return 'OTHER';
+		}
+
+		switch ($this->previous->getCode()) {
+			case Horde_Imap_Client_Exception::LOGIN_AUTHENTICATIONFAILED:
+				return 'AUTHENTICATION';
+			case Horde_Imap_Client_Exception::SERVER_CONNECT:
+			case Horde_Imap_Client_Exception::SERVER_READERROR:
+				return 'CONNECTION_ERROR';
+			default:
+				return 'OTHER';
+		}
 	}
 }
diff --git a/lib/Service/SetupService.php b/lib/Service/SetupService.php
index 78b423425..89152eb96 100644
--- a/lib/Service/SetupService.php
+++ b/lib/Service/SetupService.php
@@ -117,11 +117,12 @@ class SetupService {
 	 * @param string $uid
 	 * @param int|null $accountId
 	 *
+	 * @throws CouldNotConnectException
 	 * @throws ServiceException
 	 *
-	 * @return Account|null
+	 * @return Account
 	 */
-	public function createNewAccount($accountName, $emailAddress, $imapHost, $imapPort, $imapSslMode, $imapUser, $imapPassword, $smtpHost, $smtpPort, $smtpSslMode, $smtpUser, $smtpPassword, $uid, ?int $accountId = null): ?Account {
+	public function createNewAccount($accountName, $emailAddress, $imapHost, $imapPort, $imapSslMode, $imapUser, $imapPassword, $smtpHost, $smtpPort, $smtpSslMode, $smtpUser, $smtpPassword, $uid, ?int $accountId = null): Account {
 		$this->logger->info('Setting up manually configured account');
 		$newAccount = new MailAccount([
 			'accountId' => $accountId,
@@ -165,7 +166,7 @@ class SetupService {
 		try {
 			$imapClient->login();
 		} catch (Horde_Imap_Client_Exception $e) {
-			throw CouldNotConnectException::create($e, 'IMAP', $mailAccount->getInboundHost(), $mailAccount->getInboundPort());
+			throw new CouldNotConnectException($e, 'IMAP', $mailAccount->getInboundHost(), $mailAccount->getInboundPort());
 		}
 
 		$transport = $this->smtpClientFactory->create($account);
@@ -173,7 +174,7 @@ class SetupService {
 			try {
 				$transport->getSMTPObject();
 			} catch (Horde_Mail_Exception $e) {
-				throw CouldNotConnectException::create($e, 'SMTP', $mailAccount->getOutboundHost(), $mailAccount->getOutboundPort());
+				throw new CouldNotConnectException($e, 'SMTP', $mailAccount->getOutboundHost(), $mailAccount->getOutboundPort());
 			}
 		}
 	}
diff --git a/src/components/AccountForm.vue b/src/components/AccountForm.vue
index 2fa0fc265..5e231dc55 100644
--- a/src/components/AccountForm.vue
+++ b/src/components/AccountForm.vue
@@ -13,15 +13,19 @@
 					:placeholder="t('mail', 'Name')"
 					:disabled="loading"
 					autofocus>
-				<label for="auto-address">{{ t('mail', 'Mail Address') }}</label>
+				<label for="auto-address" class="account-form__label--required">{{ t('mail', 'Mail address') }}</label>
 				<input
 					id="auto-address"
-					v-model="autoConfig.emailAddress"
+					v-model.lazy="autoConfig.emailAddress"
 					type="email"
-					:placeholder="t('mail', 'Mail Address')"
+					:placeholder="t('mail', 'name@example.org')"
 					:disabled="loading"
-					required>
-				<label for="auto-password">{{ t('mail', 'Password') }}</label>
+					required
+					@blur="isValidEmail(autoConfig.emailAddress)">
+				<p v-if="!isValidEmail(autoConfig.emailAddress)" class="account-form--error">
+					{{ t('mail', 'Please enter an email of the format name@example.com') }}
+				</p>
+				<label for="auto-password" class="account-form__label--required">{{ t('mail', 'Password') }}</label>
 				<input
 					id="auto-password"
 					v-model="autoConfig.password"
@@ -38,17 +42,21 @@
 					type="text"
 					:placeholder="t('mail', 'Name')"
 					:disabled="loading">
-				<label for="man-address">{{ t('mail', 'Mail Address') }}</label>
+				<label for="man-address" class="account-form__label--required">{{ t('mail', 'Mail address') }}</label>
 				<input
 					id="man-address"
-					v-model="manualConfig.emailAddress"
+					v-model.lazy="manualConfig.emailAddress"
 					type="email"
-					:placeholder="t('mail', 'Mail Address')"
+					:placeholder="t('mail', 'name@example.org')"
 					:disabled="loading"
-					required>
+					required
+					@blur="isValidEmail(manualConfig.emailAddress)">
+				<p v-if="!isValidEmail(manualConfig.emailAddress)" class="account-form--error">
+					{{ t('mail', 'Please enter an email of the format name@example.com') }}
+				</p>
 
 				<h3>{{ t('mail', 'IMAP Settings') }}</h3>
-				<label for="man-imap-host">{{ t('mail', 'IMAP Host') }}</label>
+				<label for="man-imap-host" class="account-form__label--required">{{ t('mail', 'IMAP Host') }}</label>
 				<input
 					id="man-imap-host"
 					v-model="manualConfig.imapHost"
@@ -56,7 +64,9 @@
 					:placeholder="t('mail', 'IMAP Host')"
 					:disabled="loading"
 					required>
-				<h4>{{ t('mail', 'IMAP Security') }}</h4>
+				<h4 class="account-form__heading--required">
+					{{ t('mail', 'IMAP Security') }}
+				</h4>
 				<div class="flex-row">
 					<input
 						id="man-imap-sec-none"
@@ -95,7 +105,7 @@
 						for="man-imap-sec-tls"
 						:class="{primary: manualConfig.imapSslMode === 'tls'}">{{ t('mail', 'STARTTLS') }}</label>
 				</div>
-				<label for="man-imap-port">{{ t('mail', 'IMAP Port') }}</label>
+				<label for="man-imap-port" class="account-form__label--required">{{ t('mail', 'IMAP Port') }}</label>
 				<input
 					id="man-imap-port"
 					v-model="manualConfig.imapPort"
@@ -103,7 +113,7 @@
 					:placeholder="t('mail', 'IMAP Port')"
 					:disabled="loading"
 					required>
-				<label for="man-imap-user">{{ t('mail', 'IMAP User') }}</label>
+				<label for="man-imap-user" class="account-form__label--required">{{ t('mail', 'IMAP User') }}</label>
 				<input
 					id="man-imap-user"
 					v-model="manualConfig.imapUser"
@@ -111,7 +121,7 @@
 					:placeholder="t('mail', 'IMAP User')"
 					:disabled="loading"
 					required>
-				<label for="man-imap-password">{{ t('mail', 'IMAP Password') }}</label>
+				<label for="man-imap-password" class="account-form__label--required">{{ t('mail', 'IMAP Password') }}</label>
 				<input
 					id="man-imap-password"
 					v-model="manualConfig.imapPassword"
@@ -121,7 +131,7 @@
 					required>
 
 				<h3>{{ t('mail', 'SMTP Settings') }}</h3>
-				<label for="man-smtp-host">{{ t('mail', 'SMTP Host') }}</label>
+				<label for="man-smtp-host" class="account-form__label--required">{{ t('mail', 'SMTP Host') }}</label>
 				<input
 					id="man-smtp-host"
 					ref="smtpHost"
@@ -131,7 +141,9 @@
 					:placeholder="t('mail', 'SMTP Host')"
 					:disabled="loading"
 					required>
-				<h4>{{ t('mail', 'SMTP Security') }}</h4>
+				<h4 class="account-form__heading--required">
+					{{ t('mail', 'SMTP Security') }}
+				</h4>
 				<div class="flex-row">
 					<input
 						id="man-smtp-sec-none"
@@ -170,7 +182,7 @@
 						for="man-smtp-sec-tls"
 						:class="{primary: manualConfig.smtpSslMode === 'tls'}">{{ t('mail', 'STARTTLS') }}</label>
 				</div>
-				<label for="man-smtp-port">{{ t('mail', 'SMTP Port') }}</label>
+				<label for="man-smtp-port" class="account-form__label--required">{{ t('mail', 'SMTP Port') }}</label>
 				<input
 					id="man-smtp-port"
 					v-model="manualConfig.smtpPort"
@@ -178,7 +190,7 @@
 					:placeholder="t('mail', 'SMTP Port')"
 					:disabled="loading"
 					required>
-				<label for="man-smtp-user">{{ t('mail', 'SMTP User') }}</label>
+				<label for="man-smtp-user" class="account-form__label--required">{{ t('mail', 'SMTP User') }}</label>
 				<input
 					id="man-smtp-user"
 					v-model="manualConfig.smtpUser"
@@ -186,7 +198,7 @@
 					:placeholder="t('mail', 'SMTP User')"
 					:disabled="loading"
 					required>
-				<label for="man-smtp-password">{{ t('mail', 'SMTP Password') }}</label>
+				<label for="man-smtp-password" class="account-form__label--required">{{ t('mail', 'SMTP Password') }}</label>
 				<input
 					id="man-smtp-password"
 					v-model="manualConfig.smtpPassword"
@@ -196,12 +208,28 @@
 					required>
 			</Tab>
 		</Tabs>
-		<slot name="feedback" />
-		<input type="submit"
-			class="primary"
-			:disabled="loading"
-			:value="submitButtonText"
-			@click.prevent="onSubmit">
+		<div class="account-form__submit-buttons">
+			<button v-if="mode === 'auto'"
+				class="primary account-form__submit-button"
+				type="submit"
+				:disabled="isDisabledAuto"
+				@click.prevent="onSubmit">
+				<span v-if="loading" class="icon-loading-small account-form__submit-button__spinner" />
+				{{ submitButtonText }}
+			</button>
+
+			<button v-else-if="mode === 'manual'"
+				class="primary account-form__submit-button"
+				type="submit"
+				:disabled="isDisabledManual"
+				@click.prevent="onSubmit">
+				<span v-if="loading" class="icon-loading-small account-form__submit-button__spinner" />
+				{{ submitButtonText }}
+			</button>
+		</div>
+		<div class="account-form--feedback">
+			<slot name="feedback" />
+		</div>
 	</form>
 </template>
 
@@ -266,13 +294,39 @@ export default {
 				smtpUser: fromAccountOr('smtpUser', ''),
 				smtpPassword: '',
 			},
-			submitButtonText: this.account ? t('mail', 'Save') : t('mail', 'Connect'),
 		}
 	},
 	computed: {
 		settingsPage() {
 			return this.account !== undefined
 		},
+
+		isDisabledAuto() {
+			switch (this.mode === 'auto') {
+			case !this.autoConfig.emailAddress || !this.isValidEmail(this.autoConfig.emailAddress) || !this.autoConfig.password :
+				return true
+			}
+			return this.loading
+		},
+
+		isDisabledManual() {
+			switch (this.mode === 'manual') {
+			case !this.manualConfig.emailAddress || !this.isValidEmail(this.manualConfig.emailAddress)
+			|| !this.manualConfig.imapHost || !this.manualConfig.imapPort
+			|| !this.manualConfig.imapUser || !this.manualConfig.imapPassword
+			|| !this.manualConfig.smtpHost || !this.manualConfig.smtpPort
+			|| !this.manualConfig.smtpUser || !this.manualConfig.smtpPassword:
+				return true
+			}
+			return this.loading
+		},
+
+		submitButtonText() {
+			if (this.loading) {
+				return t('mail', 'Connecting')
+			}
+			return this.account ? t('mail', 'Save') : t('mail', 'Connect')
+		},
 	},
 	methods: {
 		onModeChanged(e) {
@@ -339,13 +393,17 @@ export default {
 			}
 		},
 		onSubmit(event) {
-			console.debug('account form submitted', { event })
-
-			this.loading = true
-
-			this.saveChanges()
-				.catch((error) => logger.error('could not save account details', { error }))
-				.then(() => (this.loading = false))
+			if (!this.isDisabledManual || !this.isDisabledAuto) {
+				console.debug('account form submitted', { event })
+				this.loading = true
+				this.saveChanges()
+					.catch((error) => logger.error('could not save account details', { error }))
+					.then(() => (this.loading = false))
+			}
+		},
+		isValidEmail(email) {
+			const regExpEmail = /^(([^<>()[\]\\.,;:\s@"]+(\.[^<>()[\]\\.,;:\s@"]+)*)|(".+"))@((\[[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\.[0-9]{1,3}\])|(([a-zA-Z\-0-9]+\.)+[a-zA-Z]{2,}))$/
+			return regExpEmail.test(email)
 		},
 	},
 }
@@ -413,4 +471,33 @@ input[type='radio'][disabled] + label {
 	cursor: default;
 	opacity: 0.5;
 }
+.account-form__label--required:after {
+	content:" *";
+}
+.account-form__heading--required:after {
+	content:" *";
+}
+.account-form__submit-buttons {
+	display: flex;
+	justify-content: center;
+	margin-top: 5px;
+}
+.account-form__submit-button {
+	display: flex;
+	align-items: center;
+}
+.account-form__submit-button__spinner {
+	margin: 0 10px 0 0;
+	height: auto;
+	width: auto;
+}
+.account-form--feedback {
+	color: var(--color-text-maxcontrast);
+	margin-top: 5px;
+	text-align: center;
+}
+.account-form--error {
+	text-align: left;
+	font-size: 14px;
+}
 </style>
diff --git a/src/views/Setup.vue b/src/views/Setup.vue
index 647a50e5e..31285a9d7 100644
--- a/src/views/Setup.vue
+++ b/src/views/Setup.vue
@@ -1,14 +1,17 @@
 <template>
 	<Content app-name="mail">
 		<Navigation v-if="hasAccounts" />
-		<div id="emptycontent">
-			<div class="icon-mail" />
-			<h2>{{ t('mail', 'Connect your mail account') }}</h2>
-			<AccountForm :display-name="displayName" :email="email" :save="onSave">
-				<template v-if="error" #feedback class="warning">
-					{{ error }}
+		<div class="mail-empty-content">
+			<EmptyContent icon="icon-mail">
+				<h2>{{ t('mail', 'Connect your mail account') }}</h2>
+				<template #desc>
+					<AccountForm :display-name="displayName" :email="email" :save="onSave">
+						<template v-if="error" #feedback class="warning">
+							{{ error }}
+						</template>
+					</AccountForm>
 				</template>
-			</AccountForm>
+			</EmptyContent>
 		</div>
 	</Content>
 </template>
@@ -19,6 +22,7 @@ import { loadState } from '@nextcloud/initial-state'
 import { translate as t } from '@nextcloud/l10n'
 
 import AccountForm from '../components/AccountForm'
+import EmptyContent from '@nextcloud/vue/dist/Components/EmptyContent'
 import Navigation from '../components/Navigation'
 import logger from '../logger'
 
@@ -27,6 +31,7 @@ export default {
 	components: {
 		AccountForm,
 		Content,
+		EmptyContent,
 		Navigation,
 	},
 	data() {
@@ -58,10 +63,22 @@ export default {
 				.catch((error) => {
 					logger.error('Could not create account', { error })
 
-					if (error?.data?.message) {
-						this.error = error.data.message
+					if (error.data?.error === 'AUTOCONFIG_FAILED') {
+						this.error = t('mail', 'Auto detect failed. Please try manual mode.')
+					} else if (error.data?.error === 'CONNECTION_ERROR') {
+						if (error.data.service === 'IMAP') {
+							this.error = t('mail', 'Manual config failed. IMAP server is not reachable.')
+						} else if (error.data.service === 'SMTP') {
+							this.error = t('mail', 'Manual config failed. SMTP server is not reachable.')
+						}
+					} else if (error.data?.error === 'AUTHENTICATION') {
+						if (error.data.service === 'IMAP') {
+							this.error = t('mail', 'Manual config failed. IMAP username or password is wrong.')
+						} else if (error.data.service === 'SMTP') {
+							this.error = t('mail', 'Manual config failed. SMTP username or password is wrong.')
+						}
 					} else {
-						this.error = t('mail', 'Unexpected error during account creation')
+						this.error = t('mail', 'There was an error while setting up your account. Please try again.')
 					}
 				})
 		},
@@ -70,7 +87,7 @@ export default {
 </script>
 
 <style>
-#emptycontent {
-	margin-top: 10vh;
+.mail-empty-content {
+	margin: 0 auto;
 }
 </style>
diff --git a/tests/Unit/Controller/AccountsControllerTest.php b/tests/Unit/Controller/AccountsControllerTest.php
index 5f8e8fe36..18e2a4f2b 100644
--- a/tests/Unit/Controller/AccountsControllerTest.php
+++ b/tests/Unit/Controller/AccountsControllerTest.php
@@ -25,7 +25,6 @@ declare(strict_types=1);
 namespace OCA\Mail\Tests\Unit\Controller;
 
 use ChristophWurst\Nextcloud\Testing\TestCase;
-use Exception;
 use Horde_Exception;
 use OCA\Mail\Account;
 use OCA\Mail\Contracts\IMailManager;
@@ -112,7 +111,7 @@ class AccountsControllerTest extends TestCase {
 		$this->request = $this->createMock(IRequest::class);
 		$this->accountService = $this->createMock(AccountService::class);
 		$this->groupsIntegration = $this->createMock(GroupsIntegration::class);
-		$this->groupsIntegration->expects($this->any())
+		$this->groupsIntegration->expects(self::any())
 			->method('expand')
 			->will($this->returnArgument(0));
 		$this->userId = 'manfred';
@@ -144,20 +143,20 @@ class AccountsControllerTest extends TestCase {
 		$this->accountId = 123;
 	}
 
-	public function testIndex() {
-		$this->account->expects($this->once())
+	public function testIndex(): void {
+		$this->account->expects(self::once())
 			->method('jsonSerialize')
-			->will($this->returnValue([
+			->will(self::returnValue([
 				'accountId' => 123,
 			]));
-		$this->accountService->expects($this->once())
+		$this->accountService->expects(self::once())
 			->method('findByUserId')
-			->with($this->equalTo($this->userId))
-			->will($this->returnValue([$this->account]));
-		$this->aliasesService->expects($this->any())
+			->with(self::equalTo($this->userId))
+			->will(self::returnValue([$this->account]));
+		$this->aliasesService->expects(self::any())
 			->method('findAll')
-			->with($this->equalTo($this->accountId), $this->equalTo($this->userId))
-			->will($this->returnValue(['a1', 'a2']));
+			->with(self::equalTo($this->accountId), self::equalTo($this->userId))
+			->will(self::returnValue(['a1', 'a2']));
 
 		$response = $this->controller->index();
 
@@ -170,80 +169,80 @@ class AccountsControllerTest extends TestCase {
 				],
 			]
 		]);
-		$this->assertEquals($expectedResponse, $response);
+		self::assertEquals($expectedResponse, $response);
 	}
 
-	public function testShow() {
-		$this->accountService->expects($this->once())
+	public function testShow(): void {
+		$this->accountService->expects(self::once())
 			->method('find')
-			->with($this->equalTo($this->userId), $this->equalTo($this->accountId))
-			->will($this->returnValue($this->account));
+			->with(self::equalTo($this->userId), self::equalTo($this->accountId))
+			->will(self::returnValue($this->account));
 
 		$response = $this->controller->show($this->accountId);
 
 		$expectedResponse = new JSONResponse($this->account);
-		$this->assertEquals($expectedResponse, $response);
+		self::assertEquals($expectedResponse, $response);
 	}
 
-	public function testShowDoesNotExist() {
-		$this->accountService->expects($this->once())
+	public function testShowDoesNotExist(): void {
+		$this->accountService->expects(self::once())
 			->method('find')
-			->with($this->equalTo($this->userId), $this->equalTo($this->accountId))
-			->will($this->throwException(new DoesNotExistException('test123')));
+			->with(self::equalTo($this->userId), self::equalTo($this->accountId))
+			->will(self::throwException(new DoesNotExistException('test123')));
 		$this->expectException(DoesNotExistException::class);
 
 		$this->controller->show($this->accountId);
 	}
 
-	public function testUpdateSignature() {
-		$this->accountService->expects($this->once())
+	public function testUpdateSignature(): void {
+		$this->accountService->expects(self::once())
 			->method('updateSignature')
-			->with($this->equalTo($this->accountId), $this->equalTo($this->userId), 'sig');
+			->with(self::equalTo($this->accountId), self::equalTo($this->userId), 'sig');
 
 		$response = $this->controller->updateSignature($this->accountId, 'sig');
 
 		$expectedResponse = new JSONResponse();
-		$this->assertEquals($expectedResponse, $response);
+		self::assertEquals($expectedResponse, $response);
 	}
 
-	public function testDeleteSignature() {
-		$this->accountService->expects($this->once())
+	public function testDeleteSignature(): void {
+		$this->accountService->expects(self::once())
 			->method('updateSignature')
-			->with($this->equalTo($this->accountId), $this->equalTo($this->userId), null);
+			->with(self::equalTo($this->accountId), self::equalTo($this->userId), null);
 
 		$response = $this->controller->updateSignature($this->accountId, null);
 
 		$expectedResponse = new JSONResponse();
-		$this->assertEquals($expectedResponse, $response);
+		self::assertEquals($expectedResponse, $response);
 	}
 
-	public function testDestroy() {
-		$this->accountService->expects($this->once())
+	public function testDestroy(): void {
+		$this->accountService->expects(self::once())
 			->method('delete')
-			->with($this->equalTo($this->userId), $this->equalTo($this->accountId));
+			->with(self::equalTo($this->userId), self::equalTo($this->accountId));
 
 		$response = $this->controller->destroy($this->accountId);
 
 		$expectedResponse = new JSONResponse();
-		$this->assertEquals($expectedResponse, $response);
+		self::assertEquals($expectedResponse, $response);
 	}
 
-	public function testDestroyDoesNotExist() {
-		$this->accountService->expects($this->once())
+	public function testDestroyDoesNotExist(): void {
+		$this->accountService->expects(self::once())
 			->method('delete')
-			->with($this->equalTo($this->userId), $this->equalTo($this->accountId))
-			->will($this->throwException(new DoesNotExistException('test')));
+			->with(self::equalTo($this->userId), self::equalTo($this->accountId))
+			->will(self::throwException(new DoesNotExistException('test')));
 		$this->expectException(DoesNotExistException::class);
 
 		$this->controller->destroy($this->accountId);
 	}
 
-	public function testCreateAutoDetectSuccess() {
+	public function testCreateAutoDetectSuccess(): void {
 		$email = 'john@example.com';
 		$password = '123456';
 		$accountName = 'John Doe';
 		$account = $this->createMock(Account::class);
-		$this->setupService->expects($this->once())
+		$this->setupService->expects(self::once())
 			->method('createNewAutoconfiguredAccount')
 			->with($accountName, $email, $password)
 			->willReturn($account);
@@ -251,31 +250,31 @@ class AccountsControllerTest extends TestCase {
 		$response = $this->controller->create($accountName, $email, $password, null, null, null, null, null, null, null, null,
 			null, null, true);
 
-		$expectedResponse = new JSONResponse($account, Http::STATUS_CREATED);
+		$expectedResponse = \OCA\Mail\Http\JsonResponse::success($account, Http::STATUS_CREATED);
 
-		$this->assertEquals($expectedResponse, $response);
+		self::assertEquals($expectedResponse, $response);
 	}
 
-	public function testCreateAutoDetectFailure() {
+	public function testCreateAutoDetectFailure(): void {
 		$email = 'john@example.com';
 		$password = '123456';
 		$accountName = 'John Doe';
-		$this->setupService->expects($this->once())
+		$this->setupService->expects(self::once())
 			->method('createNewAutoconfiguredAccount')
 			->with($accountName, $email, $password)
-			->willThrowException(new \Exception());
+			->willThrowException(new ClientException());
 		$this->expectException(ClientException::class);
 
 		$this->controller->create($accountName, $email, $password, null, null, null, null, null, null, null, null,
 			null, null, true);
 	}
 
-	public function testUpdateAutoDetectSuccess() {
+	public function testUpdateAutoDetectSuccess(): void {
 		$email = 'john@example.com';
 		$password = '123456';
 		$accountName = 'John Doe';
 		$account = $this->createMock(Account::class);
-		$this->setupService->expects($this->once())
+		$this->setupService->expects(self::once())
 			->method('createNewAutoconfiguredAccount')
 			->with($accountName, $email, $password)
 			->willReturn($account);
@@ -283,26 +282,26 @@ class AccountsControllerTest extends TestCase {
 		$response = $this->controller->create($accountName, $email, $password, null, null, null, null, null, null, null, null,
 			null, null, true);
 
-		$expectedResponse = new JSONResponse($account, Http::STATUS_CREATED);
+		$expectedResponse = \OCA\Mail\Http\JsonResponse::success($account, Http::STATUS_CREATED);
 
-		$this->assertEquals($expectedResponse, $response);
+		self::assertEquals($expectedResponse, $response);
 	}
 
-	public function testUpdateAutoDetectFailure() {
+	public function testUpdateAutoDetectFailure(): void {
 		$email = 'john@example.com';
 		$password = '123456';
 		$accountName = 'John Doe';
-		$this->setupService->expects($this->once())
+		$this->setupService->expects(self::once())
 			->method('createNewAutoconfiguredAccount')
 			->with($accountName, $email, $password)
-			->willThrowException(new Exception());
+			->willThrowException(new ClientException());
 		$this->expectException(ClientException::class);
 
 		$this->controller->create($accountName, $email, $password, null, null, null, null, null, null, null, null,
 			null, null, true);
 	}
 
-	public function testCreateManualSuccess() {
+	public function testCreateManualSuccess(): void {
 		$autoDetect = false;
 		$email = 'user@domain.tld';
 		$password = 'mypassword';
@@ -318,19 +317,19 @@ class AccountsControllerTest extends TestCase {
 		$smtpUser = 'user@domain.tld';
 		$smtpPassword = 'mypassword';
 		$account = $this->createMock(Account::class);
-		$this->setupService->expects($this->once())
+		$this->setupService->expects(self::once())
 			->method('createNewAccount')
 			->with($accountName, $email, $imapHost, $imapPort, $imapSslMode, $imapUser, $imapPassword, $smtpHost, $smtpPort, $smtpSslMode, $smtpUser, $smtpPassword, $this->userId)
 			->willReturn($account);
 
 		$response = $this->controller->create($accountName, $email, $password, $imapHost, $imapPort, $imapSslMode, $imapUser, $imapPassword, $smtpHost, $smtpPort, $smtpSslMode, $smtpUser, $smtpPassword, $autoDetect);
 
-		$expectedResponse = new JSONResponse($account, Http::STATUS_CREATED);
+		$expectedResponse = \OCA\Mail\Http\JsonResponse::success($account, Http::STATUS_CREATED);
 
-		$this->assertEquals($expectedResponse, $response);
+		self::assertEquals($expectedResponse, $response);
 	}
 
-	public function testCreateManualFailure() {
+	public function testCreateManualFailure(): void {
 		$autoDetect = false;
 		$email = 'user@domain.tld';
 		$password = 'mypassword';
@@ -345,16 +344,16 @@ class AccountsControllerTest extends TestCase {
 		$smtpSslMode = 'none';
 		$smtpUser = 'user@domain.tld';
 		$smtpPassword = 'mypassword';
-		$this->setupService->expects($this->once())
+		$this->setupService->expects(self::once())
 			->method('createNewAccount')
 			->with($accountName, $email, $imapHost, $imapPort, $imapSslMode, $imapUser, $imapPassword, $smtpHost, $smtpPort, $smtpSslMode, $smtpUser, $smtpPassword, $this->userId)
-			->willThrowException(new Exception());
+			->willThrowException(new ClientException());
 		$this->expectException(ClientException::class);
 
 		$this->controller->create($accountName, $email, $password, $imapHost, $imapPort, $imapSslMode, $imapUser, $imapPassword, $smtpHost, $smtpPort, $smtpSslMode, $smtpUser, $smtpPassword, $autoDetect);
 	}
 
-	public function testUpdateManualSuccess() {
+	public function testUpdateManualSuccess(): void {
 		$autoDetect = false;
 		$id = 135;
 		$email = 'user@domain.tld';
@@ -371,7 +370,7 @@ class AccountsControllerTest extends TestCase {
 		$smtpUser = 'user@domain.tld';
 		$smtpPassword = 'mypassword';
 		$account = $this->createMock(Account::class);
-		$this->setupService->expects($this->once())
+		$this->setupService->expects(self::once())
 			->method('createNewAccount')
 			->with($accountName, $email, $imapHost, $imapPort, $imapSslMode, $imapUser, $imapPassword, $smtpHost, $smtpPort, $smtpSslMode, $smtpUser, $smtpPassword, $this->userId, $id)
 			->willReturn($account);
@@ -380,10 +379,10 @@ class AccountsControllerTest extends TestCase {
 
 		$expectedResponse = new JSONResponse($account);
 
-		$this->assertEquals($expectedResponse, $response);
+		self::assertEquals($expectedResponse, $response);
 	}
 
-	public function testUpdateManualFailure() {
+	public function testUpdateManualFailure(): void {
 		$autoDetect = false;
 		$id = 135;
 		$email = 'user@domain.tld';
@@ -399,38 +398,38 @@ class AccountsControllerTest extends TestCase {
 		$smtpSslMode = 'none';
 		$smtpUser = 'user@domain.tld';
 		$smtpPassword = 'mypassword';
-		$this->setupService->expects($this->once())
+		$this->setupService->expects(self::once())
 			->method('createNewAccount')
 			->with($accountName, $email, $imapHost, $imapPort, $imapSslMode, $imapUser, $imapPassword, $smtpHost, $smtpPort, $smtpSslMode, $smtpUser, $smtpPassword, $this->userId, $id)
-			->willThrowException(new Exception());
+			->willThrowException(new ClientException());
 		$this->expectException(ClientException::class);
 
 		$this->controller->update($id, $autoDetect, $accountName, $email, $password, $imapHost, $imapPort, $imapSslMode, $imapUser, $imapPassword, $smtpHost, $smtpPort, $smtpSslMode, $smtpUser, $smtpPassword);
 	}
 
-	public function testSendNewMessage() {
+	public function testSendNewMessage(): void {
 		$account = $this->createMock(Account::class);
-		$this->accountService->expects($this->once())
+		$this->accountService->expects(self::once())
 			->method('find')
 			->willReturn($account);
 		$messageData = NewMessageData::fromRequest($account, 'to@d.com', '', '', 'sub', 'bod', []);
-		$this->transmission->expects($this->once())
+		$this->transmission->expects(self::once())
 			->method('sendMessage')
 			->with($messageData, null, null, null);
 		$expected = new JSONResponse();
 
 		$resp = $this->controller->send(13, 'sub', 'bod', 'to@d.com', '', '');
 
-		$this->assertEquals($expected, $resp);
+		self::assertEquals($expected, $resp);
 	}
 
-	public function testSendingError() {
+	public function testSendingError(): void {
 		$account = $this->createMock(Account::class);
-		$this->accountService->expects($this->once())
+		$this->accountService->expects(self::once())
 			->method('find')
 			->willReturn($account);
 		$messageData = NewMessageData::fromRequest($account, 'to@d.com', '', '', 'sub', 'bod', []);
-		$this->transmission->expects($this->once())
+		$this->transmission->expects(self::once())
 			->method('sendMessage')
 			->with($messageData, null, null, null)
 			->willThrowException(new Horde_Exception('error'));
@@ -439,7 +438,7 @@ class AccountsControllerTest extends TestCase {
 		$this->controller->send(13, 'sub', 'bod', 'to@d.com', '', '');
 	}
 
-	public function testSendingManyRecipientsError() {
+	public function testSendingManyRecipientsError(): void {
 		$this->expectException(ManyRecipientsException::class);
 
 		$recipients = [];
@@ -451,7 +450,7 @@ class AccountsControllerTest extends TestCase {
 		$this->controller->send(13, 'sub', 'bod', $recipients, '', '');
 	}
 
-	public function testSendingManyRecipientsCcError() {
+	public function testSendingManyRecipientsCcError(): void {
 		$this->expectException(ManyRecipientsException::class);
 
 		$recipients = [];
@@ -463,20 +462,20 @@ class AccountsControllerTest extends TestCase {
 		$this->controller->send(13, 'sub', 'bod', '', $recipients, '');
 	}
 
-	public function testSendReply() {
+	public function testSendReply(): void {
 		$account = $this->createMock(Account::class);
 		$replyMessage = new Message();
 		$messageId = 1234;
-		$this->accountService->expects($this->once())
+		$this->accountService->expects(self::once())
 			->method('find')
 			->willReturn($account);
-		$this->mailManager->expects($this->once())
+		$this->mailManager->expects(self::once())
 			->method('getMessage')
 			->with($this->userId, $messageId)
 			->willReturn($replyMessage);
 		$messageData = NewMessageData::fromRequest($account, 'to@d.com', '', '', 'sub', 'bod', []);
 		$replyData = new RepliedMessageData($account, $replyMessage);
-		$this->transmission->expects($this->once())
+		$this->transmission->expects(self::once())
 			->method('sendMessage')
 			->with($messageData, $replyData, null, null);
 		$expected = new JSONResponse();
@@ -495,7 +494,7 @@ class AccountsControllerTest extends TestCase {
 			[],
 			null);
 
-		$this->assertEquals($expected, $resp);
+		self::assertEquals($expected, $resp);
 	}
 
 	public function draftDataProvider(): array {
@@ -518,14 +517,14 @@ class AccountsControllerTest extends TestCase {
 		$newUid = 124;
 		$account = $this->createMock(Account::class);
 		$mailbox = new Mailbox();
-		$this->accountService->expects($this->once())
+		$this->accountService->expects(self::once())
 			->method('find')
 			->with($this->userId, $this->accountId)
-			->will($this->returnValue($this->account));
-		$this->transmission->expects($this->once())
+			->will(self::returnValue($this->account));
+		$this->transmission->expects(self::once())
 			->method('saveDraft')
 			->willReturn([$account, $mailbox, $newUid]);
-		$this->mailManager->expects($this->once())
+		$this->mailManager->expects(self::once())
 			->method('getMessageIdForUid')
 			->willReturn($newId);
 
@@ -534,6 +533,6 @@ class AccountsControllerTest extends TestCase {
 		$expected = new JSONResponse([
 			'id' => $newId,
 		]);
-		$this->assertEquals($expected, $actual);
+		self::assertEquals($expected, $actual);
 	}
 }
diff --git a/tests/Unit/Controller/SieveControllerTest.php b/tests/Unit/Controller/SieveControllerTest.php
index 2d80afcbb..bad9ff61d 100644
--- a/tests/Unit/Controller/SieveControllerTest.php
+++ b/tests/Unit/Controller/SieveControllerTest.php
@@ -28,6 +28,7 @@ use Horde\ManageSieve\Exception;
 use OCA\Mail\Account;
 use OCA\Mail\Controller\SieveController;
 use OCA\Mail\Db\MailAccount;
+use OCA\Mail\Exception\ClientException;
 use OCA\Mail\Exception\CouldNotConnectException;
 use OCA\Mail\Tests\Integration\TestCase;
 
@@ -130,7 +131,7 @@ class SieveControllerTest extends TestCase {
 	}
 
 	public function testGetActiveScriptNoSieve(): void {
-		$this->expectException(CouldNotConnectException::class);
+		$this->expectException(ClientException::class);
 		$this->expectExceptionMessage('ManageSieve is disabled');
 
 		$mailAccount = new MailAccount();