Enhance getEvents with logging and error handling#81
Enhance getEvents with logging and error handling#81hismayilov wants to merge 2 commits intoLJPc-solutions:mainfrom
Conversation
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.
Lars-
left a comment
There was a problem hiding this comment.
First of all, thank you for the pull request! There are a couple of small things that could be improved. Can you change those?
Http/Helpers/CalDAV.php
Outdated
| 'Depth' => '1', | ||
| ] ); | ||
|
|
||
| \Log::info('CalDAV REPORT response', ['status' => $response['statusCode'] ?? 'no status', 'body_length' => strlen($response['body'] ?? '')]); |
There was a problem hiding this comment.
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.
Http/Helpers/CalDAV.php
Outdated
| \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 ); |
There was a problem hiding this comment.
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 );
Http/Helpers/CalDAV.php
Outdated
| ' </C:filter>' . "\n" . | ||
| '</C:calendar-query>'; | ||
|
|
||
| $response = $this->client->request( 'REPORT', $calendarUrl, $xmlBody, [ |
There was a problem hiding this comment.
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.
SummaryThis PR fixes CalDAV event retrieval for providers (notably Zoho) that return Fixes CalDAV event sync for Zoho-like servers that return REPORT results without inline What changed
Why this is neededSome CalDAV servers return valid event references ( Scope
Testing performed
Risk / impact
|
Lars-
left a comment
There was a problem hiding this comment.
2 small changes, then it's good!
|
|
||
| libxml_use_internal_errors( true ); | ||
| $document = new \DOMDocument(); | ||
| if ( $document->loadXML( $responseBody ) ) { |
There was a problem hiding this comment.
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' ); |
There was a problem hiding this comment.
Doesn't $node->nodeValue already returns decoded text?
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