-
Notifications
You must be signed in to change notification settings - Fork 15
Mitigate issues with missing awaits in state method calls #253
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
Conversation
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
…oper state call handling
…and set() methods
… in get() and set() methods; remove GlobalExecutionContext class
…ration handling; replace promise chaining with a queue system for better management of pending operations
…e class; refactor get() and set() methods to utilize the new queue system
Member
|
Not really the approach we were going for, closing in favor of a future implementation |
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.
This PR creates a queue-like mechanism for the state methods with a method that can be awaited until all the queued promises are resolved. This method is then called after a runtime method executes, which makes is so that these state operations are executed in the proper runtime moment (and in the order they are queued) even if the user misses to await them.
This helps mitigate the issue of missing awaits to state methods (#199), ensuring these are properly executed if they are runtime methods top level calls, but not if the runtime method calls some other method with state calls and this method is not awaited, because then the runtime method execution can finish before the other method had the chance to queue the operations. An example of such behavior would be:
However, if we deconstruct the
mintmethod intothis code will now work despite the missing
awaits in the state set calls.I also tested adding a 50ms delay timeout to the queue
onCompletedmethod in an attempt to help these other missingawaits catchup, but ended up not adding it as I felt that it might not work if the complexity of the runtime methods increases as well because this doesn't ensure the linearity of these calls.