From 8a1cb77730725944498dac3d243cdda01152501f Mon Sep 17 00:00:00 2001
From: Victor Lap <victorlap@outlook.com>
Date: Fri, 12 Apr 2024 14:14:32 +0200
Subject: [PATCH] fix: Reduce log messages when no model found

Signed-off-by: Victor Lap <victorlap@outlook.com>
Signed-off-by: Richard Steinmetz <richard@steinmetz.cloud>
Signed-off-by: Christoph Wurst <christoph@winzerhof-wurst.at>
---
 lib/Command/Predict.php                  | 26 +++++++++++++++++-------
 lib/Exception/ModelNotFoundException.php | 13 ++++++++++++
 lib/Service/EstimatorService.php         |  2 ++
 lib/Service/LoginClassifier.php          |  9 ++++++--
 lib/Service/ModelStore.php               | 13 ++++++++----
 5 files changed, 50 insertions(+), 13 deletions(-)
 create mode 100644 lib/Exception/ModelNotFoundException.php

diff --git a/lib/Command/Predict.php b/lib/Command/Predict.php
index df365286..adcbd04d 100644
--- a/lib/Command/Predict.php
+++ b/lib/Command/Predict.php
@@ -9,6 +9,8 @@
 
 namespace OCA\SuspiciousLogin\Command;
 
+use OCA\SuspiciousLogin\Exception\ModelNotFoundException;
+use OCA\SuspiciousLogin\Exception\ServiceException;
 use OCA\SuspiciousLogin\Service\EstimatorService;
 use OCA\SuspiciousLogin\Service\Ipv4Strategy;
 use OCA\SuspiciousLogin\Service\IpV6Strategy;
@@ -55,14 +57,24 @@ protected function execute(InputInterface $input, OutputInterface $output): int
 		$uid = $input->getArgument('uid');
 		$ip = $input->getArgument('ip');
 		$modelId = $input->getArgument('model');
-		if (!$this->estimatorService->predict(
-			$uid,
-			$ip,
-			$input->getOption('v6') ? new IpV6Strategy() : new Ipv4Strategy(),
-			$modelId ? (int)$modelId : null)) {
-			$output->writeln("WARN: IP $ip is suspicious");
-			return 1;
+
+		try {
+			if (!$this->estimatorService->predict(
+				$uid,
+				$ip,
+				$input->getOption('v6') ? new IpV6Strategy() : new Ipv4Strategy(),
+				$modelId ? (int)$modelId : null)) {
+				$output->writeln("WARN: IP $ip is suspicious");
+				return 1;
+			}
+		} catch (ModelNotFoundException $ex) {
+			$output->writeln('<error>Could not predict suspiciousness: ' . $ex->getMessage() . '</error>');
+			return 2;
+		} catch (ServiceException $ex) {
+			$output->writeln('<error>Could not predict suspiciousness: ' . $ex->getMessage() . '</error>');
+			return 3;
 		}
+
 		$output->writeln("OK:   IP $ip is not suspicious");
 		return 0;
 	}
diff --git a/lib/Exception/ModelNotFoundException.php b/lib/Exception/ModelNotFoundException.php
new file mode 100644
index 00000000..61ffe929
--- /dev/null
+++ b/lib/Exception/ModelNotFoundException.php
@@ -0,0 +1,13 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2025 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\SuspiciousLogin\Exception;
+
+class ModelNotFoundException extends ServiceException {
+}
diff --git a/lib/Service/EstimatorService.php b/lib/Service/EstimatorService.php
index ada3f458..7d47b229 100644
--- a/lib/Service/EstimatorService.php
+++ b/lib/Service/EstimatorService.php
@@ -9,6 +9,7 @@
 
 namespace OCA\SuspiciousLogin\Service;
 
+use OCA\SuspiciousLogin\Exception\ModelNotFoundException;
 use OCA\SuspiciousLogin\Exception\ServiceException;
 use Psr\Log\LoggerInterface;
 use Rubix\ML\Datasets\Unlabeled;
@@ -28,6 +29,7 @@ public function __construct(
 	 * @param int|null $modelId
 	 *
 	 * @return bool
+	 * @throws ModelNotFoundException
 	 * @throws ServiceException
 	 */
 	public function predict(string $uid, string $ip, AClassificationStrategy $strategy, ?int $modelId = null): bool {
diff --git a/lib/Service/LoginClassifier.php b/lib/Service/LoginClassifier.php
index 92a37690..90567e4f 100644
--- a/lib/Service/LoginClassifier.php
+++ b/lib/Service/LoginClassifier.php
@@ -12,6 +12,7 @@
 use OCA\SuspiciousLogin\Db\SuspiciousLogin;
 use OCA\SuspiciousLogin\Db\SuspiciousLoginMapper;
 use OCA\SuspiciousLogin\Event\SuspiciousLoginEvent;
+use OCA\SuspiciousLogin\Exception\ModelNotFoundException;
 use OCA\SuspiciousLogin\Exception\ServiceException;
 use OCA\SuspiciousLogin\Util\AddressClassifier;
 use OCP\AppFramework\Utility\ITimeFactory;
@@ -75,10 +76,14 @@ public function process(string $uid, string $ip) {
 				// All good, carry on!
 				return;
 			}
-		} catch (ServiceException $ex) {
-			$this->logger->debug("Could not predict suspiciousness: " . $ex->getMessage());
+		} catch (ModelNotFoundException $ex) {
+			$this->logger->debug('Could not predict suspiciousness: ' . $ex->getMessage());
 			// This most likely means there is no trained model yet, so we return early here
 			return;
+		} catch (ServiceException $ex) {
+			$this->logger->warning("Could not predict suspiciousness: " . $ex->getMessage());
+			// There was an error loading the model, so we return early here
+			return;
 		}
 
 		$this->logger->info("Detected a login from a suspicious login. user=$uid ip=$ip strategy=" . $strategy::getTypeName());
diff --git a/lib/Service/ModelStore.php b/lib/Service/ModelStore.php
index bc6b3406..389a17c7 100644
--- a/lib/Service/ModelStore.php
+++ b/lib/Service/ModelStore.php
@@ -12,7 +12,7 @@
 use OCA\SuspiciousLogin\AppInfo\Application;
 use OCA\SuspiciousLogin\Db\Model;
 use OCA\SuspiciousLogin\Db\ModelMapper;
-use OCA\SuspiciousLogin\Exception\ServiceException;
+use OCA\SuspiciousLogin\Exception\ModelNotFoundException;
 use OCP\App\IAppManager;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\Files\IAppData;
@@ -47,14 +47,14 @@ public function __construct(
 	/**
 	 * @return Estimator
 	 * @throws RuntimeException
-	 * @throws ServiceException
+	 * @throws ModelNotFoundException
 	 */
 	public function loadLatest(AClassificationStrategy $strategy): Estimator {
 		try {
 			$latestModel = $this->modelMapper->findLatest($strategy::getTypeName());
 		} catch (DoesNotExistException $e) {
 			$this->logger->debug("No models found. Can't load latest");
-			throw new ServiceException("No models found", 0, $e);
+			throw new ModelNotFoundException("No models found", 0, $e);
 		}
 		return $this->load($latestModel->getId());
 	}
@@ -82,6 +82,11 @@ private function cache(int $id, string $serialized): void {
 		$cache->set($this->getCacheKey($id), $serialized);
 	}
 
+	/**
+	 * @return Estimator
+	 * @throws RuntimeException
+	 * @throws ModelNotFoundException
+	 */
 	public function load(int $id): Estimator {
 		$cached = $this->getCached($id);
 		if ($cached !== null) {
@@ -96,7 +101,7 @@ public function load(int $id): Estimator {
 				$modelFile = $modelsFolder->getFile((string)$id);
 			} catch (NotFoundException $e) {
 				$this->logger->error("Could not load classifier model $id: " . $e->getMessage());
-				throw new ServiceException("Could not load model $id", 0, $e);
+				throw new ModelNotFoundException("Could not load model $id", 0, $e);
 			}
 
 			$serialized = $modelFile->getContent();