fix(schedule): validate the cron expression itself, not just its arity

ScheduleSpec.__post_init__ only counted fields, so five fields of
garbage ("x x x x x", out-of-range values) constructed successfully and
surfaced later in next_after as a croniter exception outside the
ScheduleError family -- turning a 422-mappable input error into an
unclassified 500. Probe the normalized expression with croniter at
construction and wrap the failure in InvalidScheduleError.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
rayhpeng 2026-07-31 16:50:08 +08:00
parent c72bccb916
commit 681c774f32
2 changed files with 26 additions and 2 deletions

View File

@ -4,7 +4,7 @@ from dataclasses import dataclass
from datetime import UTC, datetime
from zoneinfo import ZoneInfo, ZoneInfoNotFoundError
from croniter import croniter
from croniter import CroniterError, croniter
from deerflow.domain.schedule.exceptions import InvalidScheduleError
from deerflow.domain.schedule.model.enums import ScheduleType
@ -75,7 +75,16 @@ class ScheduleSpec:
fields = [part for part in self.cron.split() if part]
if len(fields) != CRON_FIELD_COUNT:
raise InvalidScheduleError(f"Cron expression must contain exactly {CRON_FIELD_COUNT} fields")
object.__setattr__(self, "cron", " ".join(fields))
normalized = " ".join(fields)
# Counting fields is not parsing: five fields of garbage would
# otherwise construct fine and surface later, deep in next_after,
# as a croniter error outside the ScheduleError family the router
# maps -- an input error turned unclassified 500.
try:
croniter(normalized)
except CroniterError as exc:
raise InvalidScheduleError(f"Invalid cron expression: {normalized}") from exc
object.__setattr__(self, "cron", normalized)
if self.schedule_type is ScheduleType.ONCE:
if self.run_at is None:

View File

@ -94,6 +94,21 @@ class TestScheduleSpecInvariants:
def test_cron_whitespace_is_normalized_on_construction(self):
assert ScheduleSpec(ScheduleType.CRON, "UTC", cron=" 0 9 * * * ").cron == "0 9 * * *"
@pytest.mark.parametrize("expr", ["x x x x x", "60 * * * *", "* * * * 99"])
def test_a_five_field_expression_the_parser_rejects_is_invalid_at_construction(self, expr):
"""Counting fields is not parsing: garbage with five fields used to
construct successfully and only blow up later, deep in `next_after`,
as a croniter exception outside the ScheduleError family -- turning a
422-mappable input error into an unclassified 500."""
with pytest.raises(InvalidScheduleError, match="Invalid cron expression"):
ScheduleSpec.cron_schedule(expr, "UTC")
@pytest.mark.parametrize("expr", ["*/5 * * * *", "0 9 * * mon-fri", "0 0 1,15 * *"])
def test_real_cron_vocabulary_still_constructs(self, expr):
"""The validity probe must not over-reject: steps, ranges, lists, and
named weekdays are everyday croniter vocabulary."""
assert ScheduleSpec.cron_schedule(expr, "UTC").cron == expr
def test_naive_run_at_is_localized_to_the_schedule_timezone(self):
spec = ScheduleSpec.once_at(datetime(2026, 8, 1, 9, 0), "Asia/Shanghai") # noqa: DTZ001 -- naive input is the subject
assert spec.run_at.utcoffset() == timedelta(hours=8)