|
1 | 1 | --- |
2 | | -"@objectstack/plugin-reports": patch |
| 2 | +"@objectstack/plugin-reports": minor |
3 | 3 | --- |
4 | 4 |
|
5 | | -`ReportService` no longer discards a report schedule's cron expression when its `timezone` is not a real IANA zone — and when it does fall back to interval, it says which of the two inputs failed. |
| 5 | +fix(plugin-reports): a non-member schedule `timezone` no longer discards the cron expression, and a schedule already holding one stops instead of firing on a cadence nobody asked for (#16291) |
| 6 | + |
| 7 | +**BREAKING** for a deployment that already stores a report schedule with a cron expression and a `timezone` that is not an IANA member. Such a schedule is delivering today, on the wrong cadence; after this change it does not deliver at all until a human corrects the zone. It ships as `minor` under the lockstep launch-window convention (`scripts/check-changeset-no-major.mjs` refuses `major`); the version number is not the signal here, this entry is. |
| 8 | + |
| 9 | +<!-- adr-0087: not-required (no-migration-prescription) No metadata moves. No key is retired, no def is unpublished, no schema shape changes, and `sys_report_schedule`'s declaration is untouched apart from a comment — so `objectstack migrate meta`, `spec-changes.json` and the upgrade guide have nothing to rewrite and no ledger entry would have anything to say. The obligation this change creates is DATA-SIDE and operational, not a code rewrite: an operator checks `sys_report_schedule.last_status` for `failed` and corrects the row's `timezone`, and the schedule resumes on the next sweep by itself. That channel is the row the operator is already looking at, which is strictly more precise than a migration-chain entry about a metadata surface that did not change. --> |
| 10 | + |
| 11 | +## What an upgrading operator has to do, and how to find out |
| 12 | + |
| 13 | +If `sys_report_schedule` holds a row whose `timezone` is not a real IANA zone **and** whose `cron_expression` is set, the sweep now marks it `last_status: 'failed'` with a `last_error` naming the zone, and stops running it. Correct the `timezone` on that row; the schedule resumes on the next sweep with no re-enable and no second action, because `active` and the past `next_run_at` are deliberately left alone. |
| 14 | + |
| 15 | +Only rows written **before** `valueDomain: 'iana_time_zone'` landed on that column can be in this state, and the set cannot grow: measured on a real kernel with a real SQLite driver, `insert` into `sys_report_schedule` with `timezone: 'Mars/Olympus'` is already refused today — `VALIDATION_FAILED · Timezone must be a valid IANA time zone identifier, e.g. Europe/Zurich (got "Mars/Olympus")`. A set that cannot grow is still not an empty one, which is why this carries a banner rather than a shrug. |
| 16 | + |
| 17 | +## The defect |
6 | 18 |
|
7 | 19 | croner (10.0.1) answers a non-member zone in three different ways, and only the middle one was ever reached here: `new Cron(expr, { timezone })` **without a callback** validates the expression and lets any zone through, `nextRun()` on that instance then throws a `CronDate` conversion `TypeError`, and the callback form throws at construction. `scheduleReport`'s eager guard used the callback-less form, so the timezone half of its own input passed straight under a guard whose stated purpose was "a clear error at schedule time instead of a schedule that silently falls back to interval on sweep" — and `nextRunAt` caught that deferred throw and returned `from + interval_minutes`. A schedule authored as "every weekday 09:00 Asia/Shanghai" became "every 1440 minutes, forever", re-derived on every sweep, logged only as a complaint about a cron expression that was perfectly good. |
8 | 20 |
|
9 | | -- **The create-time guard now asks the right question.** `scheduleReport` consults `isValueDomainMember('iana_time_zone', …)` from `@objectstack/spec/shared` — the same predicate `sys_report_schedule.timezone`'s `valueDomain` declaration enforces on write — and refuses a non-member with `VALIDATION_FAILED: invalid timezone '<zone>': not a member of the 'iana_time_zone' value domain`. One answer at both doors, so this one cannot accept what the storage gate refuses; it only says so earlier and names the input that is actually wrong. It applies whether or not a `cron_expression` is set, because the storage gate does too. |
| 21 | +## What changed |
| 22 | + |
| 23 | +- **The create-time guard now asks the right question.** `scheduleReport` consults `isValueDomainMember('iana_time_zone', …)` from `@objectstack/spec/shared` — the same predicate `sys_report_schedule.timezone`'s `valueDomain` declaration enforces on write — and refuses a non-member with `VALIDATION_FAILED: invalid timezone '<zone>': not a member of the 'iana_time_zone' value domain`. One answer at both doors, so this one cannot accept what the storage door refuses; it says so earlier and names the input that is actually wrong. It applies whether or not a `cron_expression` is set, because the storage gate does too. **This is not what makes the change breaking:** the storage door already refuses the same value today, so no reachable accept set narrows — what moves is which door answers and how clearly. |
10 | 24 | - **The row now stores the string the scheduler evaluates.** An empty `timezone` was stored verbatim while every `new Cron` call site read it as `UTC`; it is normalised to `UTC` on the way in. |
11 | | -- **A schedule already holding an unusable zone is stopped, not rescheduled.** `valueDomain` is written-values-only, so rows stored before that declaration are never re-validated and no refusal reaches them. On a sweep, a schedule with a `cron_expression` whose zone is not a member is now not run and its `next_run_at` is not advanced; `last_status` becomes `failed` and `last_error` names the zone, the expression and what to do. `active` stays set and `next_run_at` stays in the past deliberately — the same posture this loop already takes for a schedule whose report has vanished — so correcting the zone resumes the schedule on the next sweep with no second action. Interval-only schedules are untouched: interval arithmetic never consults the zone, so a legacy bad value there still delivers on the cadence its author asked for. |
| 25 | +- **A schedule already holding an unusable zone is stopped, not rescheduled.** It is not run and its `next_run_at` is not advanced; `last_status` / `last_error` carry the reason. Repairing the value automatically was rejected: the intended zone is not recoverable from a typo, and rewriting it to `UTC` would deliver at yet another set of wrong instants while the row looked healthy. Interval-only schedules are untouched — interval arithmetic never consults the zone, so a legacy bad value there still delivers on the cadence its author asked for. |
12 | 26 | - **Both fall-back warnings name both inputs.** The "no next occurrence" and the former "invalid cron" lines each mentioned only the expression, so either of them on a timezone fault sent an investigator to audit the half that was fine. They now carry the expression *and* the zone, and the second no longer asserts the expression is the broken one. |
0 commit comments