Skip nil-anchor events in non_recurring instead of crashing

[tilthouse]

Summary

Event#non_recurring does arithmetic on self['duration'], self['sdate'], and self['edate'] without nil checks. Real Calendar.app data occasionally contains rows where these fields are missing — likely malformed subscribed calendars, partial syncs from CalDAV servers, or post-Sequoia migration rows.

When that happens, icalpal aborts the entire output with one of:

NoMethodError: undefined method '/' for nil:NilClass
    (lib/event.rb:120 — `(self['duration'] / 86_400).to_i`)

ArgumentError: comparison of NilClass with ICalPal::RDT failed
    (lib/event.rb:321 — `[ s, e ].max >= $opts[:from]` with nil e)

NoMethodError: undefined method 'to_a' for nil:NilClass
    (lib/event.rb:126 — `@self['sdate'].to_a` with nil sdate)

In my case a single malformed row in a subscribed calendar takes the whole events / eventsToday / week output to zero events. I've been working around it locally with a Module#prepend patch in a downstream tool (calq/remq), but it really belongs upstream.

The fix

Three lines at the top of non_recurring:

return events if self['sdate'].nil?

self['duration'] ||= 0
self['edate']    ||= self['sdate']

• nil sdate is unrecoverable (no anchor) → skip the row, return [].
• nil duration is treated as a zero-length event → nDays = 0, runs once.
• nil edate falls back to sdate → in_window?'s [s, e].max no longer raises.

Order matters: the sdate guard must come first so the subsequent ||= self['sdate'] is safe.

Tests

Adds test/event_nil_safe_test.rb — 4 cases using stdlib minitest:

• nil sdate returns [] without raising NoMethodError
• nil duration is coerced to 0
• nil edate is coerced to sdate (avoids the in_window? ArgumentError)
• well-formed in-window event is still returned (sanity)

The test class subclasses ICalPal::Event and overrides initialize to bypass the JSON.parse / date-conversion machinery, isolating the test surface to non_recurring itself.

Run with:

ruby test/event_nil_safe_test.rb

I confirmed the suite catches the regressions — reverting the patch produces three errors with the exact line numbers cited in the documentation comment.

Notes

• No changes to runtime dependencies, gemspec, or executable surface.
• The Reminder path doesn't reach non_recurring (icalpal's bin/icalPal only calls it on Events), so no test there.
• Filing context: this is the third of three patches I had stacked locally. The other two are Preserve id for reminders in sectioned lists #53 (RestoreIdField, merged in 4.2.0) and Skip nil input and memoize plconvert #55 (plconvert nil-skip + memoize, currently open). Combined with this one, my downstream tool's lib/.../icalpal_patch.rb becomes empty and the icalPal version pin can be bumped.

[ajrosen]

Do you have a corrupted start_date, a missing start_date, or something else?

If icalPal ignored any events it thinks are malformed, it looks like the result would be the same: those events would simply not be included in the output.

I'd like to add sanity checks to initialize:

# Basic sanity checks
%w[ start_date end_date ].each do |s|
  raise TypeError.new("event has no #{s}") unless @self[s]
end

...and handle the exception in bin/icalPal:

begin
  # Instantiate an item
  item = klass.new(row)
rescue StandardError => e
  $log.error("#{e.class}: #{e}")
  $log.error(row)
  next
end

[tilthouse]

Quick note since I'm spamming you with PRs at the moment. This applies to all of them:

So while I'm a software engineer with a CS degree (from another millennium ... ow my back hurts), I should be upfront, if it's not obvious, that I'm working on a couple projects with Claude Code here and that's where the write-ups are coming from.

I'm basically working on some convenience / QoL command line utilities for reading both Apple Calendar and Apple Reminders data. The performance tweaks came about because reading reminders was taking unreasonably long.

I think the case I have is probably not uncommon for daily users of Reminders: family has a shared reminders list, cleverly called "Shopping List". Because it is shared, we just use the same list indefinitely. It has hundreds, maybe thousands, of completed reminder items. We will probably never delete it because the whole sharing process is clunky. And while you can clear out completed items, you have to go out of your way to do so. Nothing will really ever remind you to.

As for this PR, here’s what’s going on on my end:

The two rows in question have start_date = 0.0 (not NULL) and end_date = NULL. NULL propagates into duration because the SQL is CAST(end_date - start_date AS INT). So the actual nil fields downstream are edate and duration; sdate is non-nil but nonsensical (Jan 1 2001 — Apple's epoch reference).

Source data: two rows summarized Genius Bar - iPad — likely a CalDAV / Apple Business calendar invite where END was missing or got stripped in some sync round-trip. They've been in the DB for years.

Your initialize-level check is functionally fine for this case because end_date IS NULL would still trip it. It would also be cleaner than my approach: it logs the offending row (which my silent prepend workaround doesn't), centralizes the validation, and avoids whack-a-mole in recurring / monthly / etc.

One correction to my own PR description while I'm here: I cited "missing start_date" as one of the observed failure modes, but in this database that's not actually what's happening — start_date is 0.0, not NULL. The real-world crash sites are line 120 (nil duration) and line 132 (nil edate via in_window?). The sdate.nil? guard in my patch is defensive but never triggers on this data; the load-bearing parts are duration ||= 0 and edate ||= sdate.

One small note on your proposed sanity check: I'd keep it to literal nil? on start_date / end_date. Rejecting start_date == 0 would filter technically-valid (just-very-old) rows, which feels like a downstream filter concern rather than icalpal's job.

Happy to close this PR in favor of your initialize + rescue approach, or rework mine to match if you'd prefer to land it under this PR number — your call.