Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 18 additions & 16 deletions packages/lib/CalendarService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -463,23 +463,25 @@ export default abstract class BaseCalendarService implements Calendar {
: undefined;

// We create the event directly on iCal
const targetCalendars = mainHostDestinationCalendar?.externalId
? calendars.filter((c) => c.externalId === mainHostDestinationCalendar.externalId)
: calendars.slice(0, 1);

if (targetCalendars.length === 0) {
throw new Error("No target calendars found to create CalDAV calendar entry");
}
Comment on lines +466 to +472
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.

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

🌐 Web query:

Does the CalDAV/WebDAV PROPFIND response for calendar-home-set guarantee the primary/default calendar is listed first? Is there a standard property (e.g., CALDAV:schedule-default-calendar-URL) to identify the default calendar?

💡 Result:

No, the CalDAV/WebDAV PROPFIND response for calendar-home-set does not guarantee that the primary/default calendar is listed first. RFC 4791 defines CALDAV:calendar-home-set as containing zero or more DAV:href elements identifying collections with the principal's calendars, with no specification on order (RFC 4791 Section 6.2.1; sources 1,11). Implementations may vary in ordering, and clients cannot rely on position (e.g., GitHub issue on Stalwart notes clients like Apple do not consistently pick first; source 5). Yes, there is a standard property: CALDAV:schedule-default-calendar-URL (urn:ietf:params:xml:ns:caldav), defined in RFC 6638 Section 9.2. It MAY be defined on the scheduling Inbox collection (discovered via CALDAV:schedule-inbox-URL on the principal) and, if present with a DAV:href, points to the default calendar where new scheduling objects are created (sources 6,7,9,10). Servers SHOULD use this for new scheduling objects (RFC 6638 Section 4.3). Some implementations expose it on the principal too, but standard location is the inbox (source 9).

Citations:


🏁 Script executed:

# Check lines around 471 in CalendarService.ts to verify the Error handling
head -n 480 packages/lib/CalendarService.ts | tail -n 30

Repository: calcom/cal.diy

Length of output: 1332


🏁 Script executed:

# Check imports and error handling patterns in CalendarService.ts
head -n 50 packages/lib/CalendarService.ts | grep -E "^import|ErrorWithCode|TRPCError"

Repository: calcom/cal.diy

Length of output: 664


🏁 Script executed:

# Search for ErrorWithCode usage in the codebase to confirm the pattern
rg "ErrorWithCode" --type ts --type tsx -A 2 -B 2 | head -n 30

Repository: calcom/cal.diy

Length of output: 85


🏁 Script executed:

# Search for ErrorWithCode import pattern and usage
rg "ErrorWithCode" --type ts -A 2 -B 2 | head -n 50

Repository: calcom/cal.diy

Length of output: 3255


🏁 Script executed:

# Check if ErrorWithCode is used in lib files (non-tRPC context)
rg "ErrorWithCode" packages/lib --type ts

Repository: calcom/cal.diy

Length of output: 2295


🏁 Script executed:

# Verify if CalendarService.ts uses ErrorWithCode anywhere
grep -n "ErrorWithCode\|import.*Error" packages/lib/CalendarService.ts

Repository: calcom/cal.diy

Length of output: 40


🏁 Script executed:

# Check how other errors are thrown in CalendarService.ts
rg "throw new Error|throw.*Error" packages/lib/CalendarService.ts | head -n 20

Repository: calcom/cal.diy

Length of output: 261


Use ErrorWithCode instead of new Error, and prefer the standard CALDAV:schedule-default-calendar-URL property for identifying the primary calendar.

The fix correctly avoids writing to all discovered collections, but has two issues:

  1. Error type: Line 471 throws new Error(...), violating the guideline to use ErrorWithCode in non-tRPC lib files. Refactor to ErrorWithCode with an appropriate error code.

  2. Calendar selection fallback: calendars.slice(0, 1) assumes the server returns the primary calendar first, but RFC 4791 (CalDAV) does not guarantee PROPFIND response order. Per RFC 6638 Section 9.2, servers expose the default calendar via the CALDAV:schedule-default-calendar-URL property (typically on the scheduling inbox collection discovered via CALDAV:schedule-inbox-URL). If tsdav surfaces this property, prefer it over positional fallback. If not available, at least log which calendar URL was chosen when falling back to the first one, so misrouted writes are debuggable.

Note: The behavior change is intentional — previously an unmatched externalId produced an empty list and Promise.all([]) resolved silently; now it throws and triggers the compensating-update pattern (per RegularBookingService), potentially flipping a booking to PENDING. More correct, but user-visible.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/lib/CalendarService.ts` around lines 466 - 472, Replace the thrown
plain Error with an ErrorWithCode (e.g., new ErrorWithCode("No target calendars
found to create CalDAV calendar entry", "CALDAV:no-target-calendar")) where the
current check on targetCalendars fails; when selecting the primary calendar
prefer the CALDAV:schedule-default-calendar-URL property if exposed by tsdav
(inspect the collection metadata returned for a schedule-default-calendar-URL
and match that URL against calendars to build targetCalendars) and only if that
property is not available fall back to picking the first calendar from calendars
but emit a debug/info log that includes the chosen calendar URL (log the
selected calendar's href/externalId) so misrouted writes are traceable; update
the logic around mainHostDestinationCalendar, calendars and targetCalendars to
implement these checks and the ErrorWithCode replacement.


const responses = await Promise.all(
calendars
.filter((c) =>
mainHostDestinationCalendar?.externalId
? c.externalId === mainHostDestinationCalendar.externalId
: true
)
.map((calendar) =>
createCalendarObject({
calendar: {
url: calendar.externalId,
},
filename: `${uid}.ics`,
iCalString: injectScheduleAgent(iCalStringWithTimezone),
headers: this.headers,
})
)
targetCalendars.map((calendar) =>
createCalendarObject({
calendar: {
url: calendar.externalId,
},
filename: `${uid}.ics`,
iCalString: injectScheduleAgent(iCalStringWithTimezone),
headers: this.headers,
})
)
);

if (responses.some((r) => !r.ok)) {
Expand Down
Loading