Add logging of general exception to error log in federated share polling

apps/federatedfilesharing/lib/AppInfo/Application.php

@@ -177,6 +177,7 @@ function ($c) use ($server) {
 					$server->getDatabaseConnection(),
 					$server->getUserManager(),
 					\OC\Files\Filesystem::getLoader(),
+					$server->getLogger(),
 					$externalManager,
 					$externalMountProvider
 				);

apps/federatedfilesharing/lib/Command/PollIncomingShares.php

@@ -32,6 +32,7 @@
 use OCP\Files\StorageInvalidException;
 use OCP\Files\StorageNotAvailableException;
 use OCP\IDBConnection;
+use OCP\ILogger;
 use OCP\IUserManager;
 use OCP\Lock\LockedException;
 use Symfony\Component\Console\Command\Command;
@@ -48,6 +49,9 @@ class PollIncomingShares extends Command {
 	/** @var IStorageFactory */
 	private $loader;
 
+	/** @var ILogger */
+	private $logger;
+
 	/** @var Manager */
 	private $externalManager;
 
@@ -60,14 +64,17 @@ class PollIncomingShares extends Command {
 	 *
 	 * @param IDBConnection $dbConnection
 	 * @param IUserManager $userManager
-	 * @param MountProvider $externalMountProvider
 	 * @param IStorageFactory $loader
+	 * @param ILogger $logger
+	 * @param Manager|null $externalManager
+	 * @param MountProvider|null $externalMountProvider
 	 */
-	public function __construct(IDBConnection $dbConnection, IUserManager $userManager, IStorageFactory $loader, Manager $externalManager = null, MountProvider $externalMountProvider = null) {
+	public function __construct(IDBConnection $dbConnection, IUserManager $userManager, IStorageFactory $loader, ILogger $logger, Manager $externalManager = null, MountProvider $externalMountProvider = null) {
 		parent::__construct();
 		$this->dbConnection = $dbConnection;
 		$this->userManager = $userManager;
 		$this->loader = $loader;
+		$this->logger = $logger;
 		$this->externalManager = $externalManager;
 		$this->externalMountProvider = $externalMountProvider;
 	}
@@ -124,13 +131,20 @@ public function execute(InputInterface $input, OutputInterface $output) {
 					$output->writeln(
 						"Remote \"$remote\" reports that external share with id \"$entryId\" no longer exists. Removing it.."
 					);
-				} catch (\Exception $e) {
+				} catch (StorageNotAvailableException $e) {
 					$entryId = $shareData['id'];
 					$remote = $shareData['remote'];
 					$reason = $e->getMessage();
 					$output->writeln(
-						"Skipping external share with id \"$entryId\" from remote \"$remote\". Reason: \"$reason\""
+						"Skipping external share with id \"$entryId\" from remote \"$remote\" as share is unreachable. Reason: \"$reason\""
+					);
+				} catch (\Exception $e) {
+					$entryId = $shareData['id'];
+					$remote = $shareData['remote'];
+					$output->writeln(
+						"Skipping external share with id \"$entryId\" from remote \"$remote\" due to internal server error"
 					);
+					$this->logger->logException($e, ['app' => 'federatedfilesharing']);
 				}
 			}
 		}

apps/federatedfilesharing/tests/Command/PollIncomingSharesTest.php

@@ -33,6 +33,7 @@
 use OCP\Files\Storage\IStorageFactory;
 use OCP\Files\StorageNotAvailableException;
 use OCP\IDBConnection;
+use OCP\ILogger;
 use OCP\IUser;
 use OCP\IUserManager;
 use PHPUnit\Framework\MockObject\MockObject;
@@ -61,6 +62,9 @@ class PollIncomingSharesTest extends TestCase {
 	/** @var IStorageFactory | MockObject */
 	private $loader;
 
+	/** @var IStorageFactory | MockObject */
+	private $logger;
+
 	/**
 	 * @var Manager | MockObject
 	 */
@@ -71,9 +75,10 @@ protected function setUp(): void {
 		$this->dbConnection = $this->createMock(IDBConnection::class);
 		$this->userManager = $this->createMock(IUserManager::class);
 		$this->loader = $this->createMock(IStorageFactory::class);
+		$this->logger = $this->createMock(ILogger::class);
 		$this->externalManager = $this->createMock(Manager::class);
 		$this->externalMountProvider = $this->createMock(MountProvider::class);
-		$command = new PollIncomingShares($this->dbConnection, $this->userManager, $this->loader, $this->externalManager, $this->externalMountProvider);
+		$command = new PollIncomingShares($this->dbConnection, $this->userManager, $this->loader, $this->logger, $this->externalManager, $this->externalMountProvider);
 		$this->commandTester = new CommandTester($command);
 	}
 
@@ -103,7 +108,7 @@ public function testNoSharesPoll() {
 	}
 
 	public function testWithFilesSharingDisabled() {
-		$command = new PollIncomingShares($this->dbConnection, $this->userManager, $this->loader, $this->externalManager, null);
+		$command = new PollIncomingShares($this->dbConnection, $this->userManager, $this->loader, $this->logger, $this->externalManager, null);
 		$this->commandTester = new CommandTester($command);
 		$this->commandTester->execute([]);
 		$output = $this->commandTester->getDisplay();
@@ -148,7 +153,47 @@ public function testUnavailableStorage() {
 		$this->commandTester->execute([]);
 		$output = $this->commandTester->getDisplay();
 		$this->assertStringContainsString(
-			'Skipping external share with id "50" from remote "example.org". Reason: "Ooops"',
+			'Skipping external share with id "50" from remote "example.org" as share is unreachable. Reason: "Ooops"',
+			$output
+		);
+	}
+
+	public function testInternalServerError() {
+		$uid = 'foo';
+		$exprBuilder = $this->createMock(IExpressionBuilder::class);
+		$statementMock = $this->createMock(Statement::class);
+		$statementMock->method('fetch')->willReturnOnConsecutiveCalls(
+			['user' => $uid],
+			['id' => 50, 'remote' => 'example.org'],
+			false
+		);
+		$qbMock = $this->createMock(IQueryBuilder::class);
+		$qbMock->method('select')->willReturnSelf();
+		$qbMock->method('selectDistinct')->willReturnSelf();
+		$qbMock->method('from')->willReturnSelf();
+		$qbMock->method('where')->willReturnSelf();
+		$qbMock->method('expr')->willReturn($exprBuilder);
+		$qbMock->method('execute')->willReturn($statementMock);
+
+		$userMock = $this->createMock(IUser::class);
+		$this->userManager->expects($this->once())->method('get')
+			->with($uid)->willReturn($userMock);
+
+		$storage = $this->createMock(Storage::class);
+		$storage->method('hasUpdated')->willThrowException(new \Exception(''));
+		$storage->method('getRemote')->willReturn('example.org');
+
+		$mount = $this->createMock(\OCA\Files_Sharing\External\Mount::class);
+		$mount->method('getStorage')->willReturn($storage);
+		$mount->method('getMountPoint')->willReturn("/$uid/files/point");
+		$this->externalMountProvider->expects($this->once())->method('getMountsForUser')
+			->willReturn([$mount]);
+
+		$this->dbConnection->method('getQueryBuilder')->willReturn($qbMock);
+		$this->commandTester->execute([]);
+		$output = $this->commandTester->getDisplay();
+		$this->assertStringContainsString(
+			'Skipping external share with id "50" from remote "example.org" due to internal server error',
 			$output
 		);
 	}