Only catch invalid recurrence rule errors

kewisch · Apr 17, 2024 · 377646b · 377646b
1 parent 36a3f3e
commit 377646b
Show file tree

Hide file tree

Showing 3 changed files with 35 additions and 16 deletions.
diff --git a/lib/ical/recur_iterator.js b/lib/ical/recur_iterator.js
@@ -182,10 +182,15 @@ class RecurIterator {
       try {
         this.init();
       } catch (e) {
-        // Init may error if there are no possible recurrence instances from the
-        // rule, but we don't want to bubble this error up. Instead, we create
-        // an empty iterator.
-        this.completed = true;
+        if (e instanceof InvalidRecurrenceRuleError) {
+          // Init may error if there are no possible recurrence instances from
+          // the rule, but we don't want to bubble this error up. Instead, we
+          // create an empty iterator.
+          this.completed = true;
+        } else {
+          // Propagate other errors to consumers.
+          throw e;
+        }
       }
     }
   }
@@ -277,7 +282,7 @@ class RecurIterator {
       }
 
       if (this.days.length == 0) {
-        throw new Error("No possible occurrences");
+        throw new InvalidRecurrenceRuleError();
       }
 
       this._nextByYearDay();
@@ -1378,4 +1383,11 @@ class RecurIterator {
     return result;
   }
 }
+
+class InvalidRecurrenceRuleError extends Error {
+  constructor() {
+    super("Recurrence rule has no valid occurrences");
+  }
+}
+
 export default RecurIterator;
diff --git a/test/recur_iterator_test.js b/test/recur_iterator_test.js
@@ -144,6 +144,13 @@ suite('recur_iterator', function() {
       let start = ICAL.Time.fromString(options.dtStart);
       let recur = ICAL.Recur.fromString(ruleString);
 
+      if (options.throws) {
+        assert.throws(function() {
+          recur.iterator(start);
+        });
+        return;
+      }
+
       let iterator = recur.iterator(start);
 
       if (options.noInstance) {
@@ -688,7 +695,7 @@ suite('recur_iterator', function() {
       // Invalid rule. There's never a 31st of Feburary, check that this fails.
       testRRULE('FREQ=MONTHLY;INTERVAL=12;BYMONTHDAY=31', {
         dtStart: '2022-02-01T08:00:00',
-        noInstance: true,
+        throws: true,
       });
 
       // monthly + by month

diff --git a/test/recur_test.js b/test/recur_test.js
@@ -26,56 +26,56 @@ suite('recur', function() {
       });
     }
 
-    function checkNoInstance(data, expectedMessage, dtstart, stack) {
+    function checkThrow(data, expectedMessage, dtstart, stack) {
       test(expectedMessage, function() {
         let recur = new ICAL.Recur(data);
         if (dtstart) {
           dtstart = ICAL.Time.fromString(dtstart);
         } else {
           dtstart = ICAL.Time.epochTime.clone();
         }
-
-        const iter = recur.iterator(dtstart);
-        assert.equal(iter.next(), null);
+        assert.throws(function() {
+          recur.iterator(dtstart);
+        }, expectedMessage);
       });
     }
 
-    checkNoInstance({
+    checkThrow({
       parts: {
         BYYEARDAY: [3, 4, 5],
         BYMONTH: [2]
       }
     }, 'Invalid BYYEARDAY rule');
 
-    checkNoInstance({
+    checkThrow({
       parts: {
         BYWEEKNO: [3],
         BYMONTHDAY: [2]
       }
    }, 'BYWEEKNO does not fit to BYMONTHDAY');
 
-    checkNoInstance({
+    checkThrow({
       freq: 'MONTHLY',
       parts: {
         BYWEEKNO: [30]
       }
     }, 'For MONTHLY recurrences neither BYYEARDAY nor BYWEEKNO may appear');
 
-    checkNoInstance({
+    checkThrow({
       freq: 'WEEKLY',
       parts: {
         BYMONTHDAY: [20]
       }
     }, 'For WEEKLY recurrences neither BYMONTHDAY nor BYYEARDAY may appear');
 
-    checkNoInstance({
+    checkThrow({
       freq: 'DAILY',
       parts: {
         BYYEARDAY: [200]
       }
     }, 'BYYEARDAY may only appear in YEARLY rules');
 
-    checkNoInstance({
+    checkThrow({
       freq: 'MONTHLY',
       parts: {
         BYDAY: ['-6TH']