feat: tracking issues with calendar subscriptions

Signed-off-by: Roberto Guido <info@madbob.org>

Author: madbob

apps/dav/lib/CalDAV/CalDavBackend.php

@@ -3160,6 +3160,29 @@ public function deleteSubscription($subscriptionId) {
 		}, $this->db);
 	}
 
+	/**
+	 * Update the error status of a subscription
+	 *
+	 * @param mixed $subscriptionId
+	 * @param null|Exception $exception
+	 * @param null|string $error
+	 * @return void
+	 */
+	public function trackSubscriptionError($subscriptionId, $exception, $error): void {
+		$query = $this->db->getQueryBuilder();
+		$query->update('calendarsubscriptions')
+			->set('lasterror', $query->createNamedParameter($error))
+			->executeStatement();
+
+		if ($error) {
+			$this->logger->error('Subscription {subscriptionId} could not be refreshed: {error}', [
+				'exception' => $exception,
+				'subscriptionId' => $subscriptionId,
+				'error' => $error,
+			]);
+		}
+	}
+
 	/**
 	 * Returns a single scheduling object for the inbox collection.
 	 *

apps/dav/lib/CalDAV/WebcalCaching/Connection.php

@@ -9,18 +9,16 @@
 
 namespace OCA\DAV\CalDAV\WebcalCaching;
 
-use Exception;
 use GuzzleHttp\RequestOptions;
+use OCA\DAV\Exception\InvalidSubscriptionPayload;
+use OCA\DAV\Exception\InvalidSubscriptionUrl;
 use OCP\Http\Client\IClientService;
-use OCP\Http\Client\LocalServerException;
 use OCP\IAppConfig;
-use Psr\Log\LoggerInterface;
 
 class Connection {
 	public function __construct(
 		private IClientService $clientService,
 		private IAppConfig $config,
-		private LoggerInterface $logger,
 	) {
 	}
 
@@ -33,7 +31,7 @@ public function queryWebcalFeed(array $subscription): ?array {
 		$subscriptionId = $subscription['id'];
 		$url = $this->cleanURL($subscription['source']);
 		if ($url === null) {
-			return null;
+			throw new InvalidSubscriptionUrl('Wrong URL: ' . $subscription['source']);
 		}
 
 		// ICS feeds hosted on O365 can return HTTP 500 when the UA string isn't satisfactory
@@ -65,20 +63,8 @@ public function queryWebcalFeed(array $subscription): ?array {
 			$params[RequestOptions::AUTH] = [$user, $pass];
 		}
 
-		try {
-			$client = $this->clientService->newClient();
-			$response = $client->get($url, $params);
-		} catch (LocalServerException $ex) {
-			$this->logger->warning("Subscription $subscriptionId was not refreshed because it violates local access rules", [
-				'exception' => $ex,
-			]);
-			return null;
-		} catch (Exception $ex) {
-			$this->logger->warning("Subscription $subscriptionId could not be refreshed due to a network error", [
-				'exception' => $ex,
-			]);
-			return null;
-		}
+		$client = $this->clientService->newClient();
+		$response = $client->get($url, $params);
 
 		$contentType = $response->getHeader('Content-Type');
 		$contentType = explode(';', $contentType, 2)[0];
@@ -92,7 +78,7 @@ public function queryWebcalFeed(array $subscription): ?array {
 		// With 'stream' => true, getBody() returns the underlying stream resource
 		$stream = $response->getBody();
 		if (!is_resource($stream)) {
-			return null;
+			throw new InvalidSubscriptionPayload('Cannot open a stream for ' . $url);
 		}
 
 		return ['data' => $stream, 'format' => $format];

apps/dav/lib/CalDAV/WebcalCaching/RefreshWebcalService.php

@@ -9,9 +9,13 @@
 
 namespace OCA\DAV\CalDAV\WebcalCaching;
 
+use Exception;
 use OCA\DAV\CalDAV\CalDavBackend;
 use OCA\DAV\CalDAV\Import\ImportService;
+use OCA\DAV\Exception\InvalidSubscriptionPayload;
+use OCA\DAV\Exception\InvalidSubscriptionUrl;
 use OCP\AppFramework\Utility\ITimeFactory;
+use OCP\Http\Client\LocalServerException;
 use Psr\Log\LoggerInterface;
 use Sabre\DAV\PropPatch;
 use Sabre\VObject\Component;
@@ -54,19 +58,18 @@ public function refreshSubscription(string $principalUri, string $uri) {
 			}
 		}
 
-		$result = $this->connection->queryWebcalFeed($subscription);
-		if (!$result) {
-			return;
-		}
+		$data = null;
+
+		try {
+			$result = $this->connection->queryWebcalFeed($subscription);
 
-		$data = $result['data'];
-		$format = $result['format'];
+			$data = $result['data'];
+			$format = $result['format'];
 
-		$stripTodos = ($subscription[self::STRIP_TODOS] ?? 1) === 1;
-		$stripAlarms = ($subscription[self::STRIP_ALARMS] ?? 1) === 1;
-		$stripAttachments = ($subscription[self::STRIP_ATTACHMENTS] ?? 1) === 1;
+			$stripTodos = ($subscription[self::STRIP_TODOS] ?? 1) === 1;
+			$stripAlarms = ($subscription[self::STRIP_ALARMS] ?? 1) === 1;
+			$stripAttachments = ($subscription[self::STRIP_ATTACHMENTS] ?? 1) === 1;
 
-		try {
 			$existingObjects = $this->calDavBackend->getLimitedCalendarObjects((int)$subscription['id'], CalDavBackend::CALENDAR_TYPE_SUBSCRIPTION, ['id', 'uid', 'etag', 'uri']);
 
 			$generator = match ($format) {
@@ -158,11 +161,21 @@ public function refreshSubscription(string $principalUri, string $uri) {
 			if (isset($vObject)) {
 				$this->updateRefreshRate($subscription, $vObject);
 			}
+
+			$this->calDavBackend->trackSubscriptionError($subscription['id'], null, null);
 		} catch (ParseException $ex) {
-			$this->logger->error('Subscription {subscriptionId} could not be refreshed due to a parsing error', ['exception' => $ex, 'subscriptionId' => $subscription['id']]);
+			$this->calDavBackend->trackSubscriptionError($subscription['id'], $ex, 'Parsing error');
+		} catch (LocalServerException $ex) {
+			$this->calDavBackend->trackSubscriptionError($subscription['id'], $ex, 'Subscription violates local access rules');
+		} catch (InvalidSubscriptionUrl $ex) {
+			$this->calDavBackend->trackSubscriptionError($subscription['id'], $ex, 'Invalid URL');
+		} catch (InvalidSubscriptionPayload $ex) {
+			$this->calDavBackend->trackSubscriptionError($subscription['id'], $ex, 'Invalid contents');
+		} catch (Exception $ex) {
+			$this->calDavBackend->trackSubscriptionError($subscription['id'], $ex, 'Network error');
 		} finally {
 			// Close the data stream to free resources
-			if (is_resource($data)) {
+			if ($data && is_resource($data)) {
 				fclose($data);
 			}
 		}

apps/dav/lib/Exception/InvalidSubscriptionPayload.php

@@ -0,0 +1,15 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\DAV\Exception;
+
+use Exception;
+
+class InvalidSubscriptionPayload extends Exception {
+}

apps/dav/lib/Exception/InvalidSubscriptionUrl.php

@@ -0,0 +1,15 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\DAV\Exception;
+
+use Exception;
+
+class InvalidSubscriptionUrl extends Exception {
+}

apps/dav/lib/Migration/Version2000Date20260815202617.php

@@ -0,0 +1,45 @@
+<?php
+
+declare(strict_types=1);
+
+/**
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\DAV\Migration;
+
+use Closure;
+use OCP\DB\ISchemaWrapper;
+use OCP\DB\Types;
+use OCP\Migration\IOutput;
+use OCP\Migration\SimpleMigrationStep;
+
+/**
+ * FIXME Auto-generated migration step: Please modify to your needs!
+ */
+class Version2000Date20260815202617 extends SimpleMigrationStep {
+	/**
+	 * @param IOutput $output
+	 * @param Closure(): ISchemaWrapper $schemaClosure
+	 * @param array $options
+	 * @return null|ISchemaWrapper
+	 */
+	#[Override]
+	public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper {
+		/** @var ISchemaWrapper $schema */
+		$schema = $schemaClosure();
+
+		$table = $schema->getTable('calendarsubscriptions');
+
+		if (!$table->hasColumn('lasterror')) {
+			$table->addColumn('lasterror', Types::STRING, [
+				'notnull' => false,
+				'length' => 255,
+				'default' => null,
+			]);
+		}
+
+		return $schema;
+	}
+}

apps/dav/tests/unit/CalDAV/WebcalCaching/ConnectionTest.php

@@ -9,6 +9,8 @@
 namespace OCA\DAV\Tests\unit\CalDAV\WebcalCaching;
 
 use OCA\DAV\CalDAV\WebcalCaching\Connection;
+use OCA\DAV\Exception\InvalidSubscriptionPayload;
+use OCA\DAV\Exception\InvalidSubscriptionUrl;
 use OCP\Http\Client\IClient;
 use OCP\Http\Client\IClientService;
 use OCP\Http\Client\IResponse;
@@ -60,10 +62,8 @@ public function testLocalUrl($source): void {
 		$client->expects(self::once())
 			->method('get')
 			->willThrowException($localServerException);
-		$this->logger->expects(self::once())
-			->method('warning')
-			->with('Subscription 42 was not refreshed because it violates local access rules', ['exception' => $localServerException]);
 
+		$this->expectException(LocalServerException::class);
 		$this->connection->queryWebcalFeed($subscription);
 	}
 
@@ -85,6 +85,7 @@ public function testInvalidUrl(): void {
 		$client->expects(self::never())
 			->method('get');
 
+		$this->expectException(InvalidSubscriptionUrl::class);
 		$this->connection->queryWebcalFeed($subscription);
 
 	}
@@ -186,6 +187,7 @@ public function testConnectionReturnsNullWhenBodyIsNotResource(): void {
 			->method('getBody')
 			->willReturn('not a resource');
 
+		$this->expectException(InvalidSubscriptionPayload::class);
 		$output = $this->connection->queryWebcalFeed($subscription);
 
 		$this->assertNull($output);

apps/dav/tests/unit/CalDAV/WebcalCaching/RefreshWebcalServiceTest.php

@@ -12,6 +12,7 @@
 use OCA\DAV\CalDAV\Import\ImportService;
 use OCA\DAV\CalDAV\WebcalCaching\Connection;
 use OCA\DAV\CalDAV\WebcalCaching\RefreshWebcalService;
+use OCA\DAV\Exception\InvalidSubscriptionUrl;
 use OCP\AppFramework\Utility\ITimeFactory;
 use PHPUnit\Framework\MockObject\MockObject;
 use Psr\Log\LoggerInterface;
@@ -92,6 +93,10 @@ public function testRun(string $body, string $format, string $result): void {
 			->method('queryWebcalFeed')
 			->willReturn(['data' => $stream, 'format' => $format]);
 
+		$this->caldavBackend->expects(self::once())
+			->method('trackSubscriptionError')
+			->with('42', null, null);
+
 		$this->caldavBackend->expects(self::once())
 			->method('getLimitedCalendarObjects')
 			->willReturn([]);
@@ -226,9 +231,15 @@ public function testConnectionReturnsNull(): void {
 				],
 			]);
 
+		$exception = new InvalidSubscriptionUrl('broken');
+
 		$this->connection->expects(self::once())
 			->method('queryWebcalFeed')
-			->willReturn(null);
+			->willThrowException($exception);
+
+		$this->caldavBackend->expects(self::once())
+			->method('trackSubscriptionError')
+			->with('42', $exception, 'Invalid URL');
 
 		$this->importService->expects(self::never())
 			->method('importText');