[core] Add max iteration count for detecting infinite loops #5430
Conversation
- Introduced `MachineOptions` interface to define runtime options, including `maxIterations`. - Updated `StateMachine` class to accept and initialize options from the configuration. - Modified `macrostep` function to utilize the `maxIterations` option, allowing for dynamic iteration limits.
🦋 Changeset detectedLatest commit: e500583 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
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 |
- Changed default value of `maxIterations` in `MachineOptions` to `Infinity` for no limit. - Enhanced `macrostep` function to support dynamic iteration limits, throwing an error for infinite loops only when a valid limit is set. - Updated tests to reflect new `maxIterations` behavior.
| options: { | ||
| maxIterations: 100 | ||
| } |
There was a problem hiding this comment.
It's yet another place when some options are accepted. I'm not saying this is inherently bad but I can see how this might not be that discoverable given this is the only option accepted this way now.
There was a problem hiding this comment.
I also wonder, shouldn't this kinda be a global option really? As a user, I don't see myself using this everywhere - and currently I'd have to because it's opt-in.
Some of other libraries (like even React) just implements such protections automatically and they don't allow the user to confgure the "depth count" that triggers the error.
| this.version = this.config.version; | ||
| this.schemas = this.config.schemas; | ||
| this.options = { | ||
| maxIterations: 1000, |
There was a problem hiding this comment.
The changeset file mentions:
The default is
Infinity(no limit) to avoid breaking existing machines.
but clearly it's not true as the default here is actually set to 1000 at runtime.
|
|
||
| let shouldSelectEventlessTransitions = true; | ||
| const maxIterations = snapshot.machine.options?.maxIterations ?? Infinity; | ||
| const hasMaxIterations = maxIterations !== Infinity && maxIterations !== -1; |
There was a problem hiding this comment.
I don't think handling 2 values like this makes sense. If you need a value representing some special situation then make one value represent that situation. And also - if -1 means "disabled"... what about any other negative number?
There was a problem hiding this comment.
I'll stick to infinity
Add
maxIterationsoption to configure the maximum number of microsteps allowed before throwing an infinite loop error. The default isInfinity(no limit) to avoid breaking existing machines.You can configure it when creating a machine: