eschutho opened a new pull request, #42486:
URL: https://github.com/apache/superset/pull/42486
### SUMMARY
A crontab expression that's syntactically valid but can never match a real
calendar date (e.g. `0 0 30 2 *` for Feb 30th, or `0 0 31 4 *` for April 31st —
April only has 30 days) passes API schema validation on report/alert
create/update, because `superset/reports/schemas.py::validate_crontab()` only
checks `croniter.is_valid()`, which is a purely syntactic check.
Once such a schedule is saved as active, the celery-beat task
`reports.scheduler` (`superset/tasks/scheduler.py`) iterates over **every**
active `ReportSchedule` and calls `cron_schedule_window()`
(`superset/tasks/cron_util.py`) for each, with no per-schedule isolation:
```python
for active_schedule in active_schedules:
for schedule in cron_schedule_window(
triggered_at, active_schedule.crontab, active_schedule.timezone
):
```
Inside `cron_schedule_window`, iterating `crons.all_next(datetime)` raises
`croniter.CroniterBadDateError` for a bad-date cron (croniter's internal
max-year search gives up), uncaught anywhere in the chain. Because the loop has
no per-schedule isolation, **one** report/alert with a
bad-but-syntactically-valid crontab crashes the scheduler task for every other
active schedule too — on every celery-beat tick, across the whole instance —
until someone finds and fixes/deletes the offending schedule directly in the
database.
### PROBLEM
```pycon
>>> from croniter import croniter
>>> from datetime import datetime, timezone
>>> croniter.is_valid("0 0 30 2 *")
True
>>> next(iter(croniter("0 0 30 2 *", datetime(2026, 7, 27,
tzinfo=timezone.utc)).all_next(datetime)))
croniter.croniter.CroniterBadDateError: failed to find next date
```
`CroniterBadDateError` is a subclass of `CroniterError`, itself a subclass
of `ValueError` — a raw library exception with no Superset-level handling on
this path.
### FIX
`cron_schedule_window()` already has an established precedent for degrading
gracefully on bad per-schedule input a few lines above this bug: it catches
`UnknownTimeZoneError` for an invalid timezone string and falls back to UTC
with a `logger.warning`. This PR follows the same convention for the cron
expression itself: the `crons.all_next(datetime)` iteration is now wrapped in
`try/except CroniterBadDateError`, logging an error (this case is not a
graceful degrade like the timezone fallback — it means the schedule can never
fire until someone fixes it, so it's logged loudly) and letting the generator
end instead of propagating and crashing the whole scheduler run.
This is a background celery-task loop, not an HTTP request/response path, so
there's no 4xx/500 status to map to — the fix intentionally does not introduce
a new `superset/exceptions.py` subclass (unlike the API-layer fixes in the
related PRs below).
### TESTING INSTRUCTIONS
New parametrized test `test_cron_schedule_window_invalid_cron_date` added to
`tests/unit_tests/tasks/test_cron_util.py`, covering `"0 0 30 2 *"` (Feb 30th)
and `"0 0 31 4 *"` (April 31st), asserting `cron_schedule_window()` returns
`[]` instead of raising.
Verified empirically both ways:
- **Pre-fix** (fix stashed, test kept): both new cases fail with an uncaught
`croniter.croniter.CroniterBadDateError: failed to find next date` propagating
out of the test.
- **Post-fix**: full `tests/unit_tests/tasks/test_cron_util.py` — 36 passed,
no regressions to the timezone-fallback or normal-schedule cases.
`ruff check` / `ruff format --check` clean on both changed files (verified
against both the repo's ambient ruff and the CI-pinned `ruff==0.9.7` via
`uvx`). `mypy` shows the same pre-existing missing-stub errors (`croniter`,
`pytz`, `freezegun.api.FakeDatetime`) before and after — no new errors
introduced.
### Tradeoffs
Additive-only: a cron that can never fire now logs an error and produces
zero scheduled executions instead of crashing the whole scheduler task. No
change to the failure mode for any valid crontab.
### ADDITIONAL INFORMATION
Same bug class / cleanup series as apache/superset#42366,
apache/superset#42401, apache/superset#42426, apache/superset#42442 (raw
system/library exception reaching a live code path instead of being handled) —
this one surfaces in the report-scheduling background task rather than an API
request path.
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
