Skip to content

fix(cron): coerce null runHistory ms fields from jobs.json - #4985

Merged
Re-bin merged 2 commits into
HKUDS:mainfrom
santhreal:fix/cron-runhistory-null-ms
Jul 21, 2026
Merged

Re-bin merged 2 commits into
HKUDS:mainfrom
santhreal:fix/cron-runhistory-null-ms

Conversation

@santhreal

Copy link
Copy Markdown
Contributor

jobs.json can store runAtMs/durationMs (and createdAtMs/updatedAtMs) as explicit JSON null. get_camel_snake returns that null, so bare int() raised TypeError and quarantined the store. Treat null/blank like a missing key (0), same idea as the dual-case and string-ms load hardening.

Repro: put "runAtMs": null in a runHistory row, then load jobs.

Explicit JSON null for runAtMs/durationMs bypassed the missing-key
default and raised TypeError on load. Treat null/blank like missing.
Same present-null footgun as runHistory; route all required store
ints through _store_int.
@chengyongru chengyongru added priority: p1 Important user-impacting issue or project priority with bounded impact or a reasonable workaround. bug Something isn't working fix test labels Jul 19, 2026

@Re-bin Re-bin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. This fixes the persisted cron compatibility bug at the deserialization boundary without changing scheduling behavior.

The current head is green, merges cleanly with the latest main, and the focused cron regression suite passes.

@Re-bin
Re-bin merged commit b6156fd into HKUDS:main Jul 21, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working fix priority: p1 Important user-impacting issue or project priority with bounded impact or a reasonable workaround. test

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants