fix(app): queue notification refetch if mqtt topic update occurs while refetching #18694
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.
Closes RABR-781
Overview
If MQTT informs the app that it should refetch a particular topic while the app is actively refetching that topic, the app doesn't perform an additional refetch after the first refetch completes. This creates a potential race condition.
To fix, the app always needs to refetch again if MQTT tells the app an update is available while the app is in the process of refetching.
This bug is most noticeable on the
robot-server/runs/commands_links
when the commands in question are extremely short lived commands such ascomment
commands. This protocol repros the issue rather readily:Test Plan and Hands on Testing
Reran the above protocol multiple times:
Changelog
Review requests
edge
with these changes given the risk level, but let me know if we want them elsewhere. (After some convos, we are targeting 8.5 for this fix).Risk assessment
low-med: This change is only additive. If anything in the app was dependent on stale data, this bug was still a race condition at the end of the day, and we would have visibly seen the unwanted behavior by now. It's still a networking change though, so there's inherent risk involved.