Skip to content

Commit 9834b7a

Browse files
madbobmadbob
committed
feat: tracking issues with calendar subscriptions
Signed-off-by: Roberto Guido <info@madbob.org>
1 parent d3aba32 commit 9834b7a

9 files changed

Lines changed: 195 additions & 74 deletions

File tree

‎apps/dav/lib/CalDAV/CalDavBackend.php‎

Lines changed: 71 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,13 @@
1111
use DateTime;
1212
use DateTimeImmutable;
1313
use DateTimeInterface;
14+
use Exception as NativeException;
1415
use Generator;
1516
use OCA\DAV\AppInfo\Application;
1617
use OCA\DAV\CalDAV\Federation\FederatedCalendarEntity;
1718
use OCA\DAV\CalDAV\Federation\FederatedCalendarMapper;
1819
use OCA\DAV\CalDAV\Sharing\Backend;
20+
use OCA\DAV\CalDAV\WebcalCaching\RefreshWebcalService;
1921
use OCA\DAV\Connector\Sabre\Principal;
2022
use OCA\DAV\DAV\Sharing\IShareable;
2123
use OCA\DAV\Events\CachedCalendarObjectCreatedEvent;
@@ -49,6 +51,7 @@
4951
use OCP\ICacheFactory;
5052
use OCP\IConfig;
5153
use OCP\IDBConnection;
54+
use OCP\IL10N;
5255
use OCP\IUserManager;
5356
use OCP\Security\ISecureRandom;
5457
use Psr\Log\LoggerInterface;
@@ -218,6 +221,7 @@ public function __construct(
218221
private IConfig $config,
219222
private Sharing\Backend $calendarSharingBackend,
220223
private FederatedCalendarMapper $federatedCalendarMapper,
224+
private IL10N $l10n,
221225
ICacheFactory $cacheFactory,
222226
private bool $legacyEndpoint = false,
223227
) {
@@ -735,6 +739,7 @@ public function getSubscriptionById($subscriptionId) {
735739
$fields[] = 'synctoken';
736740
$fields[] = 'principaluri';
737741
$fields[] = 'lastmodified';
742+
$fields[] = 'lasterror';
738743

739744
$query = $this->db->getQueryBuilder();
740745
$query->select($fields)
@@ -749,18 +754,7 @@ public function getSubscriptionById($subscriptionId) {
749754
return null;
750755
}
751756

752-
$row['principaluri'] = (string)$row['principaluri'];
753-
$subscription = [
754-
'id' => $row['id'],
755-
'uri' => $row['uri'],
756-
'principaluri' => $row['principaluri'],
757-
'source' => $row['source'],
758-
'lastmodified' => $row['lastmodified'],
759-
'{' . Plugin::NS_CALDAV . '}supported-calendar-component-set' => new SupportedCalendarComponentSet(['VTODO', 'VEVENT']),
760-
'{http://sabredav.org/ns}sync-token' => $row['synctoken'] ?: '0',
761-
];
762-
763-
return $this->rowToSubscription($row, $subscription);
757+
return $this->rowToSubscription($row);
764758
}
765759

766760
public function getSubscriptionByUri(string $principal, string $uri): ?array {
@@ -771,6 +765,7 @@ public function getSubscriptionByUri(string $principal, string $uri): ?array {
771765
$fields[] = 'synctoken';
772766
$fields[] = 'principaluri';
773767
$fields[] = 'lastmodified';
768+
$fields[] = 'lasterror';
774769

775770
$query = $this->db->getQueryBuilder();
776771
$query->select($fields)
@@ -786,18 +781,7 @@ public function getSubscriptionByUri(string $principal, string $uri): ?array {
786781
return null;
787782
}
788783

789-
$row['principaluri'] = (string)$row['principaluri'];
790-
$subscription = [
791-
'id' => $row['id'],
792-
'uri' => $row['uri'],
793-
'principaluri' => $row['principaluri'],
794-
'source' => $row['source'],
795-
'lastmodified' => $row['lastmodified'],
796-
'{' . Plugin::NS_CALDAV . '}supported-calendar-component-set' => new SupportedCalendarComponentSet(['VTODO', 'VEVENT']),
797-
'{http://sabredav.org/ns}sync-token' => $row['synctoken'] ?: '0',
798-
];
799-
800-
return $this->rowToSubscription($row, $subscription);
784+
return $this->rowToSubscription($row);
801785
}
802786

803787
/**
@@ -2987,6 +2971,7 @@ public function getSubscriptionsForUser($principalUri) {
29872971
$fields[] = 'principaluri';
29882972
$fields[] = 'lastmodified';
29892973
$fields[] = 'synctoken';
2974+
$fields[] = 'lasterror';
29902975

29912976
$query = $this->db->getQueryBuilder();
29922977
$query->select($fields)
@@ -2997,18 +2982,7 @@ public function getSubscriptionsForUser($principalUri) {
29972982

29982983
$subscriptions = [];
29992984
while ($row = $stmt->fetchAssociative()) {
3000-
$subscription = [
3001-
'id' => $row['id'],
3002-
'uri' => $row['uri'],
3003-
'principaluri' => $row['principaluri'],
3004-
'source' => $row['source'],
3005-
'lastmodified' => $row['lastmodified'],
3006-
3007-
'{' . Plugin::NS_CALDAV . '}supported-calendar-component-set' => new SupportedCalendarComponentSet(['VTODO', 'VEVENT']),
3008-
'{http://sabredav.org/ns}sync-token' => $row['synctoken'] ?: '0',
3009-
];
3010-
3011-
$subscriptions[] = $this->rowToSubscription($row, $subscription);
2985+
$subscriptions[] = $this->rowToSubscription($row);
30122986
}
30132987

30142988
return $subscriptions;
@@ -3160,6 +3134,29 @@ public function deleteSubscription($subscriptionId) {
31603134
}, $this->db);
31613135
}
31623136

3137+
/**
3138+
* Update the error status of a subscription
3139+
*
3140+
* @param mixed $subscriptionId
3141+
* @param null|NativeException $exception
3142+
* @param null|string $error
3143+
* @return void
3144+
*/
3145+
public function trackSubscriptionError($subscriptionId, $exception, $error): void {
3146+
$query = $this->db->getQueryBuilder();
3147+
$query->update('calendarsubscriptions')
3148+
->set('lasterror', $query->createNamedParameter($error))
3149+
->executeStatement();
3150+
3151+
if ($error) {
3152+
$this->logger->error('Subscription {subscriptionId} could not be refreshed: {error}', [
3153+
'exception' => $exception,
3154+
'subscriptionId' => $subscriptionId,
3155+
'error' => $error,
3156+
]);
3157+
}
3158+
}
3159+
31633160
/**
31643161
* Returns a single scheduling object for the inbox collection.
31653162
*
@@ -4000,11 +3997,47 @@ private function rowToCalendar($row, array $calendar): array {
40003997
* Amend the subscription info with database row data
40013998
*
40023999
* @param array $row
4003-
* @param array $subscription
40044000
*
40054001
* @return array
40064002
*/
4007-
private function rowToSubscription($row, array $subscription): array {
4003+
private function rowToSubscription($row): array {
4004+
$row['principaluri'] = (string)$row['principaluri'];
4005+
4006+
$subscription = [
4007+
'id' => $row['id'],
4008+
'uri' => $row['uri'],
4009+
'principaluri' => $row['principaluri'],
4010+
'source' => $row['source'],
4011+
'lastmodified' => $row['lastmodified'],
4012+
'{' . Plugin::NS_CALDAV . '}supported-calendar-component-set' => new SupportedCalendarComponentSet(['VTODO', 'VEVENT']),
4013+
'{http://sabredav.org/ns}sync-token' => $row['synctoken'] ?: '0',
4014+
];
4015+
4016+
if ($row['lasterror'] !== null) {
4017+
switch($row['lasterror']) {
4018+
case RefreshWebcalService::ERROR_PARSING:
4019+
$lasterror = $this->l10n->t('Parsing error');
4020+
break;
4021+
case RefreshWebcalService::ERROR_ACCESS:
4022+
$lasterror = $this->l10n->t('Subscription violates local access rules');
4023+
break;
4024+
case RefreshWebcalService::ERROR_URL:
4025+
$lasterror = $this->l10n->t('Invalid URL');
4026+
break;
4027+
case RefreshWebcalService::ERROR_CONTENTS:
4028+
$lasterror = $this->l10n->t('Invalid contents');
4029+
break;
4030+
case RefreshWebcalService::ERROR_NETWORK:
4031+
$lasterror = $this->l10n->t('Network error');
4032+
break;
4033+
default:
4034+
$lasterror = $this->l10n->t('Unknown error');
4035+
break;
4036+
}
4037+
4038+
$subscription['{' . \OCA\DAV\DAV\Sharing\Plugin::NS_NEXTCLOUD . '}subscription-error'] = $lasterror;
4039+
}
4040+
40084041
foreach ($this->subscriptionPropertyMap as $xmlName => [$dbName, $type]) {
40094042
$value = $row[$dbName];
40104043
if ($value !== null) {

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

Lines changed: 6 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -9,18 +9,16 @@
99

1010
namespace OCA\DAV\CalDAV\WebcalCaching;
1111

12-
use Exception;
1312
use GuzzleHttp\RequestOptions;
13+
use OCA\DAV\Exception\InvalidSubscriptionPayload;
14+
use OCA\DAV\Exception\InvalidSubscriptionUrl;
1415
use OCP\Http\Client\IClientService;
15-
use OCP\Http\Client\LocalServerException;
1616
use OCP\IAppConfig;
17-
use Psr\Log\LoggerInterface;
1817

1918
class Connection {
2019
public function __construct(
2120
private IClientService $clientService,
2221
private IAppConfig $config,
23-
private LoggerInterface $logger,
2422
) {
2523
}
2624

@@ -33,7 +31,7 @@ public function queryWebcalFeed(array $subscription): ?array {
3331
$subscriptionId = $subscription['id'];
3432
$url = $this->cleanURL($subscription['source']);
3533
if ($url === null) {
36-
return null;
34+
throw new InvalidSubscriptionUrl('Wrong URL: ' . $subscription['source']);
3735
}
3836

3937
// 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 {
6563
$params[RequestOptions::AUTH] = [$user, $pass];
6664
}
6765

68-
try {
69-
$client = $this->clientService->newClient();
70-
$response = $client->get($url, $params);
71-
} catch (LocalServerException $ex) {
72-
$this->logger->warning("Subscription $subscriptionId was not refreshed because it violates local access rules", [
73-
'exception' => $ex,
74-
]);
75-
return null;
76-
} catch (Exception $ex) {
77-
$this->logger->warning("Subscription $subscriptionId could not be refreshed due to a network error", [
78-
'exception' => $ex,
79-
]);
80-
return null;
81-
}
66+
$client = $this->clientService->newClient();
67+
$response = $client->get($url, $params);
8268

8369
$contentType = $response->getHeader('Content-Type');
8470
$contentType = explode(';', $contentType, 2)[0];
@@ -92,7 +78,7 @@ public function queryWebcalFeed(array $subscription): ?array {
9278
// With 'stream' => true, getBody() returns the underlying stream resource
9379
$stream = $response->getBody();
9480
if (!is_resource($stream)) {
95-
return null;
81+
throw new InvalidSubscriptionPayload('Cannot open a stream for ' . $url);
9682
}
9783

9884
return ['data' => $stream, 'format' => $format];

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

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,13 @@
99

1010
namespace OCA\DAV\CalDAV\WebcalCaching;
1111

12+
use Exception;
1213
use OCA\DAV\CalDAV\CalDavBackend;
1314
use OCA\DAV\CalDAV\Import\ImportService;
15+
use OCA\DAV\Exception\InvalidSubscriptionPayload;
16+
use OCA\DAV\Exception\InvalidSubscriptionUrl;
1417
use OCP\AppFramework\Utility\ITimeFactory;
18+
use OCP\Http\Client\LocalServerException;
1519
use Psr\Log\LoggerInterface;
1620
use Sabre\DAV\PropPatch;
1721
use Sabre\VObject\Component;
@@ -28,6 +32,12 @@ class RefreshWebcalService {
2832
public const STRIP_ATTACHMENTS = '{http://calendarserver.org/ns/}subscribed-strip-attachments';
2933
public const STRIP_TODOS = '{http://calendarserver.org/ns/}subscribed-strip-todos';
3034

35+
public const ERROR_PARSING = 'parsing';
36+
public const ERROR_ACCESS = 'access';
37+
public const ERROR_URL = 'url';
38+
public const ERROR_CONTENTS = 'contents';
39+
public const ERROR_NETWORK = 'network';
40+
3141
public function __construct(
3242
private CalDavBackend $calDavBackend,
3343
private LoggerInterface $logger,
@@ -54,19 +64,18 @@ public function refreshSubscription(string $principalUri, string $uri) {
5464
}
5565
}
5666

57-
$result = $this->connection->queryWebcalFeed($subscription);
58-
if (!$result) {
59-
return;
60-
}
67+
$data = null;
6168

62-
$data = $result['data'];
63-
$format = $result['format'];
69+
try {
70+
$result = $this->connection->queryWebcalFeed($subscription);
6471

65-
$stripTodos = ($subscription[self::STRIP_TODOS] ?? 1) === 1;
66-
$stripAlarms = ($subscription[self::STRIP_ALARMS] ?? 1) === 1;
67-
$stripAttachments = ($subscription[self::STRIP_ATTACHMENTS] ?? 1) === 1;
72+
$data = $result['data'];
73+
$format = $result['format'];
74+
75+
$stripTodos = ($subscription[self::STRIP_TODOS] ?? 1) === 1;
76+
$stripAlarms = ($subscription[self::STRIP_ALARMS] ?? 1) === 1;
77+
$stripAttachments = ($subscription[self::STRIP_ATTACHMENTS] ?? 1) === 1;
6878

69-
try {
7079
$existingObjects = $this->calDavBackend->getLimitedCalendarObjects((int)$subscription['id'], CalDavBackend::CALENDAR_TYPE_SUBSCRIPTION, ['id', 'uid', 'etag', 'uri']);
7180

7281
$generator = match ($format) {
@@ -158,11 +167,21 @@ public function refreshSubscription(string $principalUri, string $uri) {
158167
if (isset($vObject)) {
159168
$this->updateRefreshRate($subscription, $vObject);
160169
}
170+
171+
$this->calDavBackend->trackSubscriptionError($subscription['id'], null, null);
161172
} catch (ParseException $ex) {
162-
$this->logger->error('Subscription {subscriptionId} could not be refreshed due to a parsing error', ['exception' => $ex, 'subscriptionId' => $subscription['id']]);
173+
$this->calDavBackend->trackSubscriptionError($subscription['id'], $ex, self::ERROR_PARSING);
174+
} catch (LocalServerException $ex) {
175+
$this->calDavBackend->trackSubscriptionError($subscription['id'], $ex, self::ERROR_ACCESS);
176+
} catch (InvalidSubscriptionUrl $ex) {
177+
$this->calDavBackend->trackSubscriptionError($subscription['id'], $ex, self::ERROR_URL);
178+
} catch (InvalidSubscriptionPayload $ex) {
179+
$this->calDavBackend->trackSubscriptionError($subscription['id'], $ex, self::ERROR_CONTENTS);
180+
} catch (Exception $ex) {
181+
$this->calDavBackend->trackSubscriptionError($subscription['id'], $ex, self::ERROR_NETWORK);
163182
} finally {
164183
// Close the data stream to free resources
165-
if (is_resource($data)) {
184+
if ($data && is_resource($data)) {
166185
fclose($data);
167186
}
168187
}

‎apps/dav/lib/Exception/InvalidSubscriptionPayload.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\DAV\Exception;
11+
12+
use Exception;
13+
14+
class InvalidSubscriptionPayload extends Exception {
15+
}

‎apps/dav/lib/Exception/InvalidSubscriptionUrl.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
/**
6+
* SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
7+
* SPDX-License-Identifier: AGPL-3.0-or-later
8+
*/
9+
10+
namespace OCA\DAV\Exception;
11+
12+
use Exception;
13+
14+
class InvalidSubscriptionUrl extends Exception {
15+
}

0 commit comments

Comments
 (0)