feat: throw instead of silently rewriting invalid cron expressions (#937

) Co-authored-by: Brandon der Blätter <[email protected]>
kelektiv · Jan 10, 2025 · dcc5b93 · dcc5b93
1 parent cd7ee9f
commit dcc5b93
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 80 deletions.
diff --git a/src/constants.ts b/src/constants.ts
@@ -6,20 +6,6 @@ export const CONSTRAINTS = Object.freeze({
 	month: [1, 12],
 	dayOfWeek: [0, 7]
 } as const);
-export const MONTH_CONSTRAINTS = Object.freeze({
-	1: 31,
-	2: 29, // support leap year...not perfect
-	3: 31,
-	4: 30,
-	5: 31,
-	6: 30,
-	7: 31,
-	8: 31,
-	9: 30,
-	10: 31,
-	11: 30,
-	12: 31
-} as const);
 export const PARSE_DEFAULTS = Object.freeze({
 	second: '0',
 	minute: '*',

diff --git a/src/time.ts b/src/time.ts
@@ -3,7 +3,6 @@ import { DateTime, Zone } from 'luxon';
 import {
 	ALIASES,
 	CONSTRAINTS,
-	MONTH_CONSTRAINTS,
 	PARSE_DEFAULTS,
 	PRESETS,
 	RE_RANGE,
@@ -15,13 +14,10 @@ import {
 import { CronError, ExclusiveParametersError } from './errors';
 import {
 	CronJobParams,
-	DayOfMonthRange,
-	MonthRange,
 	Ranges,
 	TimeUnit,
 	TimeUnitField
 } from './types/cron.types';
-import { getRecordKeys } from './utils';
 
 export class CronTime {
 	source: string | DateTime;
@@ -76,59 +72,13 @@ export class CronTime {
 		} else {
 			this.source = source;
 			this._parse(this.source);
-			this._verifyParse();
 		}
 	}
 
 	private _getWeekDay(date: DateTime) {
 		return date.weekday === 7 ? 0 : date.weekday;
 	}
 
-	/**
-	 * Ensure that the syntax parsed correctly and correct the specified values if needed.
-	 */
-	private _verifyParse() {
-		const months = getRecordKeys(this.month);
-		const daysOfMonth = getRecordKeys(this.dayOfMonth);
-
-		let isOk = false;
-
-		/**
-		 * if a dayOfMonth is not found in all months, we only need to fix the last
-		 * wrong month to prevent infinite loop
-		 */
-		let lastWrongMonth: MonthRange | null = null;
-		for (const m of months) {
-			const con = MONTH_CONSTRAINTS[m];
-
-			for (const day of daysOfMonth) {
-				if (day <= con) {
-					isOk = true;
-				}
-			}
-
-			if (!isOk) {
-				// save the month in order to be fixed if all months fails (infinite loop)
-				lastWrongMonth = m;
-				console.warn(`Month '${m}' is limited to '${con}' days.`);
-			}
-		}
-
-		// infinite loop detected (dayOfMonth is not found in all months)
-		// eslint-disable-next-line @typescript-eslint/no-unnecessary-condition
-		if (!isOk && lastWrongMonth !== null) {
-			const notOkCon = MONTH_CONSTRAINTS[lastWrongMonth];
-			for (const notOkDay of daysOfMonth) {
-				if (notOkDay > notOkCon) {
-					// eslint-disable-next-line @typescript-eslint/no-dynamic-delete
-					delete this.dayOfMonth[notOkDay];
-					const fixedDay = (notOkDay % notOkCon) as DayOfMonthRange;
-					this.dayOfMonth[fixedDay] = true;
-				}
-			}
-		}
-	}
-
 	/**
 	 * Calculate the "next" scheduled time
 	 */

diff --git a/tests/cron.test.ts b/tests/cron.test.ts
@@ -616,28 +616,24 @@ describe('cron', () => {
 
 	it('should not get into an infinite loop on invalid times', () => {
 		expect(() => {
-			new CronJob(
-				'* 60 * * * *',
-				() => {
-					expect(true).toBe(true);
-				},
-				null,
-				true
-			);
+			new CronJob('* 60 * * * *', () => {}, null, true);
 		}).toThrow();
 
 		expect(() => {
-			new CronJob(
-				'* * 24 * * *',
-				() => {
-					expect(true).toBe(true);
-				},
-				null,
-				true
-			);
+			new CronJob('* * 24 * * *', () => {}, null, true);
+		}).toThrow();
+
+		expect(() => {
+			new CronJob('0 0 30 FEB *', callback, null, true);
 		}).toThrow();
 	});
 
+	it('should not throw if at least one time is valid', () => {
+		expect(() => {
+			new CronJob('0 0 30 JAN,FEB *', callback, null, true);
+		}).not.toThrow();
+	});
+
 	it('should test start of month', () => {
 		const d = new Date('12/31/2014');
 		d.setSeconds(59);
@@ -856,6 +852,7 @@ describe('cron', () => {
 
 		// tick by 1 day
 		clock.tick(24 * 60 * 60 * 1000);
+		clock.restore();
 		job.stop();
 		expect(callback).toHaveBeenCalledTimes(1);
 	});