Improve cron exception handling (#4553)

Signed-off-by: Jimmy Tanagra <jcode@tanagra.id.au>
pull/4556/head
jimtng 2025-01-11 18:14:54 +10:00 committed by GitHub
parent f8d34d9882
commit 229d87263f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 18 additions and 11 deletions

View File

@ -68,9 +68,13 @@ public class GenericCronTriggerHandler extends BaseTriggerModuleHandler
}
private void scheduleJob() {
schedule = scheduler.schedule(this, expression);
logger.debug("Scheduled cron job '{}' for trigger '{}'.", module.getConfiguration().get(CFG_CRON_EXPRESSION),
module.getId());
try {
schedule = scheduler.schedule(this, expression);
logger.debug("Scheduled cron job '{}' for trigger '{}'.",
module.getConfiguration().get(CFG_CRON_EXPRESSION), module.getId());
} catch (IllegalArgumentException e) { // Catch exception from CronAdjuster
logger.warn("Failed to schedule job for trigger '{}'. {}", module.getId(), e.getMessage());
}
}
@Override

View File

@ -12,6 +12,7 @@
*/
package org.openhab.core.scheduler;
import java.time.DateTimeException;
import java.time.DayOfWeek;
import java.time.temporal.ChronoField;
import java.time.temporal.ChronoUnit;
@ -71,7 +72,6 @@ public class CronAdjuster implements SchedulerTemporalAdjuster {
.collect(Collectors.toMap(Entry::getKey, Entry::getValue));
private final List<Field> fields = new ArrayList<>(7);
private String cronExpression;
private final Map<String, String> environmentMap;
private final boolean reboot;
@ -83,7 +83,6 @@ public class CronAdjuster implements SchedulerTemporalAdjuster {
environmentMap = parseEnvironment(entries);
String cronExpression = entries[entries.length - 1].trim();
this.cronExpression = cronExpression;
reboot = "@reboot".equals(cronExpression);
@ -108,6 +107,14 @@ public class CronAdjuster implements SchedulerTemporalAdjuster {
parseAndAdd(cronExpression, parts[2], ChronoField.HOUR_OF_DAY);
parseAndAdd(cronExpression, parts[1], ChronoField.MINUTE_OF_HOUR);
parseAndAdd(cronExpression, parts[0], ChronoField.SECOND_OF_MINUTE);
try {
// Test the cron expression in action to make sure it won't cause too many restarts
adjustInto(java.time.ZonedDateTime.now());
} catch (final DateTimeException e) {
throw new IllegalArgumentException(
String.format("Invalid cron expression '%s': %s", cronExpression, e.getMessage()));
}
}
/**
@ -529,8 +536,7 @@ public class CronAdjuster implements SchedulerTemporalAdjuster {
index++;
} else {
if (restarts++ > 1000) {
throw new IllegalArgumentException(
String.format("Cron expression '%s' is not valid, too many restarts", cronExpression));
throw new DateTimeException("Conditions not satisfied.");
}
ret = out;
index = 0;

View File

@ -203,9 +203,6 @@ public class CronAdjusterTest {
@ParameterizedTest
@ValueSource(strings = { "0 0 0 31 2 *", "* * *", "80 * * * * *" })
public void testInvalidCronExpression(String cron) {
assertThrows(IllegalArgumentException.class, () -> {
final CronAdjuster cronAdjuster = new CronAdjuster(cron);
cronAdjuster.adjustInto(java.time.ZonedDateTime.now());
});
assertThrows(IllegalArgumentException.class, () -> new CronAdjuster(cron));
}
}