feat: tracking issues with calendar subscriptions - #63298
Conversation
|
Nice! Another direction would be to save the date of last failed sync and numbers of retries so that retries have a exponential backoff and sync eventually stop trying for nothing. |
6d87b22 to
2aeb5a3
Compare
Signed-off-by: Roberto Guido <info@madbob.org>
2aeb5a3 to
f0cd11b
Compare
| } 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 { |
There was a problem hiding this comment.
We should only save a error identifier not the string, if the intent is to show the user the error, otherwise this would needed to be translated before its saved.
I think we might be over complicating this... can we just send the user a notification?
Yes this would be a good option, and then just auto remove the subscription if its failed x amount of times or for x amount of days... basically automated clean up |
|
A notification is permanent, a connectivity issue may be temporary. Once the notification is issued, it stays on the tray. I'm not really persuaded about the idea to arbitrarily delete user's data. |
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
joshtrichards
added
bug
3. to review
4 participants
Summary
Targeting nextcloud/calendar#7068
As calendar subscriptions' refresh happens asyncronously to the web client, it is not possible to immediately inform the user about issues (error in connection, in parsing, in configuration and so on). On the other side, the actual implementation fails almost silently, occasionally leaving a few traces in the system log.
The proposed change involves a new database column where to save the status of the latest refresh, so to retrieve it and deliver to the client at his next request (this has to be implemented in Calendar's interface).
This PR is not complete: the detail is saved in the database, but not incapsulated into the DAV response about the status of calendars. I have still to figure out which lovecraftian demon have to invoke to pack this in the Sabre-generated payload, and before I have to guess which part of the DAV schema is the more appropriate to carry this kind of information.
Feedback about this approach, and hints about the Sabre/DAV black magic ritual to fulfill, are both appreciated.
Checklist
3. to review, feature component)stable32)