-
Notifications
You must be signed in to change notification settings - Fork 12
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
Mitigate issues with missing awaits in state method calls #253
Open
joaosreis
wants to merge
28
commits into
develop
Choose a base branch
from
feature/state-execution-linearity
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
This file contains 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
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
mint
method intothis code will now work despite the missing
await
s in the state set calls.I also tested adding a 50ms delay timeout to the queue
onCompleted
method in an attempt to help these other missingawait
s 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.