fix(effect): stop Schedule.cron crashing on a non-finite clock#6296
Open
eldardada wants to merge 1 commit into
Open
fix(effect): stop Schedule.cron crashing on a non-finite clock#6296eldardada wants to merge 1 commit into
eldardada wants to merge 1 commit into
Conversation
When the clock is advanced past the end of time (e.g. TestClock.adjust(Infinity)), the cron schedule received a non-finite now. new Date(now) is then an Invalid Date, which made the internal Cron.match/Cron.next calls throw IllegalArgumentException and crash the fiber. Finish the schedule instead, matching how Schedule.fixed and Schedule.spaced already behave in that case.
🦋 Changeset detectedLatest commit: 63211d3 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #6292.
Problem
Advancing the clock past the end of time, e.g.
TestClock.adjust(Infinity), crashes a runningSchedule.cronwith an uncaughtIllegalArgumentException: Invalid datethat kills the fiber:Cause
The cron schedule step turns the current time into a
Dateand runs calendar math on it:When
nowis non-finite,new Date(now)is an Invalid Date.Cron.match/Cron.nextthen calldateTime.unsafeMakeZoned, whosedate.getTime()isNaN, andunsafeFromDatethrows by design. So the real bug is thatSchedule.cronhands an invalid instant to the cron helpers.Time-based neighbours like
Schedule.fixedandSchedule.spaceddo not crash here: they work with plainnow + delayarithmetic and never build aDate, so a non-finitenowjust flows through.Fix
Guard the cron step: when
nowis non-finite there is no instant to match, so finish the schedule (ScheduleDecision.done) instead of throwing. This brings cron in line with how the other time-based schedules behave when the clock runs past the end of time, and leaves all finite behaviour untouched.Verification
Reproduced the crash deterministically (cron fails while
fixed/spacedsucceed under the samenow = Infinity), confirmed the fix turns the failure into a cleanSuccess, and added a regression test in the cron suite.pnpm check(tsc),eslint, and the fullScheduletest suite (83 tests) all pass. Changeset included (effect: patch).