Skip to content

Enhance getEvents with logging and error handling#81

Open
hismayilov wants to merge 2 commits intoLJPc-solutions:mainfrom
hismayilov:patch-1
Open

Enhance getEvents with logging and error handling#81
hismayilov wants to merge 2 commits intoLJPc-solutions:mainfrom
hismayilov:patch-1

Conversation

@hismayilov
Copy link
Copy Markdown

@hismayilov hismayilov commented Mar 7, 2026

The issue was that the getEvents method was using propFind, which doesn't retrieve calendar event data correctly in CalDAV. I've updated it to use the proper REPORT method with a calendar-query to fetch all events from the calendar.

This should now work with Zoho Calendar (and other CalDAV servers). The method sends a REPORT request to query for VEVENT components and extracts the calendar data from the XML response.

Added logging for CalDAV events retrieval process and improved error handling for the REPORT request.

Previous issue. #77

The issue was that the getEvents method was using propFind, which doesn't retrieve calendar event data correctly in CalDAV. I've updated it to use the proper REPORT method with a calendar-query to fetch all events from the calendar.

This should now work with Zoho Calendar (and other CalDAV servers). The method sends a REPORT request to query for VEVENT components and extracts the calendar data from the XML response.

Added logging for CalDAV events retrieval process and improved error handling for the REPORT request.
Copy link
Copy Markdown
Contributor

@Lars- Lars- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all, thank you for the pull request! There are a couple of small things that could be improved. Can you change those?

'Depth' => '1',
] );

\Log::info('CalDAV REPORT response', ['status' => $response['statusCode'] ?? 'no status', 'body_length' => strlen($response['body'] ?? '')]);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and the other \Log::info() calls will fire on every scheduled sync (potentially every minute). Please downgrade to \Log::debug() or remove them. The URL could also contain credentials in some CalDAV setups.

\Log::info('CalDAV response body preview', ['body' => substr($response['body'], 0, 500)]);
// Parse the XML response to extract calendar-data
$matches = [];
preg_match_all( '/<(?:C:)?calendar-data[^>]*>(.*?)<\/(?:C:)?calendar-data>/s', $response['body'], $matches );
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This regex misses the cal: namespace prefix, which many CalDAV servers use. Your existing getEventByUid method handles cal:, C:, and unqualifie, this should match. Suggested fix: preg_match_all( '/<(?:cal:|C:)?calendar-data[^>]*>(.*?)<\/(?:cal:|C:)?calendar-data>/s', $response['body'], $matches );

' </C:filter>' . "\n" .
'</C:calendar-query>';

$response = $this->client->request( 'REPORT', $calendarUrl, $xmlBody, [
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Client::request() can throw ClientException on network/HTTP errors. Wrap this in a try-catch like getEventByUid does, otherwise a transient failure will crash the cron job instead of returning an empty array.

Handle REPORT responses without calendar-data by fetching each event .ics via href, reduce noisy/sensitive logging, and preserve ICS separators so imported events parse correctly.
@hismayilov
Copy link
Copy Markdown
Author

Summary

This PR fixes CalDAV event retrieval for providers (notably Zoho) that return 207 Multi-Status responses without embedded calendar-data nodes.

Fixes CalDAV event sync for Zoho-like servers that return REPORT results without inline calendar-data.
Adds a fallback to parse event hrefs and fetch .ics payloads via GET when needed.
Also preserves valid ICS boundaries by joining event payloads with \r\n in external content assembly.
Logging was reduced to safer debug output (no large/sensitive response previews).
Low-risk change: existing behavior remains the same when calendar-data is already present.

What changed

  • Added a fallback flow in CalDAV::getEvents():
    • First, try extracting calendar-data from the REPORT XML response.
    • If none is present, extract event href entries and fetch each event via GET (.ics payloads).
  • Improved XML parsing robustness:
    • Parse calendar-data and href with namespace-agnostic DOM/XPath.
    • Keep regex parsing as a fallback when XML parsing cannot extract data.
  • Reduced noisy/sensitive logs:
    • Changed several info/error logs to safer debug logs and removed large body dumps.
  • Fixed ICS concatenation in Calendar::getExternalContent():
    • Join event payloads with "\r\n" separators instead of an empty string to preserve valid ICS boundaries.

Why this is needed

Some CalDAV servers return valid event references (href) but omit inline calendar-data in REPORT results.
Before this change, those calendars could appear empty or partially imported.
With this fallback, events are retrieved reliably across more CalDAV providers (including Zoho) while preserving correct ICS formatting.

Scope

  • Http/Helpers/CalDAV.php
  • Entities/Calendar.php

Testing performed

  • Verified normal REPORT parsing still returns events when calendar-data is present.
  • Verified fallback path retrieves events by href when calendar-data is absent.
  • Confirmed resulting ICS payload remains parseable after concatenation with CRLF separators.
  • Confirmed no sensitive response body preview is logged at info/error level.

Risk / impact

  • Low to medium: retrieval logic expanded with a defensive fallback path.
  • Behavior is unchanged for providers already returning calendar-data.
  • Additional GET requests only occur when REPORT does not include event payloads.

db961ee

Copy link
Copy Markdown
Contributor

@Lars- Lars- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

2 small changes, then it's good!


libxml_use_internal_errors( true );
$document = new \DOMDocument();
if ( $document->loadXML( $responseBody ) ) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DOMDocument::loadXML() without LIBXML_NONET allows external entity loading on PHP < 8.0 (XXE risk). FreeScout supports PHP 7.x. Use:

$document->loadXML( $responseBody, LIBXML_NONET );

Same applies to extractEventHrefsFromMultistatus below.

foreach ( $nodes as $node ) {
$data = trim( $node->nodeValue );
if ( $data !== '' ) {
$events[] = html_entity_decode( $data, ENT_XML1, 'UTF-8' );
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't $node->nodeValue already returns decoded text?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants