Fix node connection issues #1504
Conversation
|
Just checked again with v0.7.7-beta2 and #1479 is still reproducible. Did you follow the steps to reproduce 100% @shubhamkmr04 ? In my opinion this PR still solves both issues and should be merged. |
| @@ -1026,6 +1026,7 @@ export default class SettingsStore { | |||
| if (status) { | |||
| this.error = false; | |||
| this.errorMsg = ''; | |||
| BackendUtils.clearCachedCalls(); | |||
There was a problem hiding this comment.
we should probably check first if clearCachedCalls is defined as it isn't defined for all backends
There was a problem hiding this comment.
That is not necessary since the call function of BackendUtils already checks this.
stores/NodeInfoStore.ts
Outdated
| BackendUtils.getMyNodeInfo() | ||
| .then((data: any) => { | ||
| if (NodeInfoStore.currentRequest !== currentRequest) { |
There was a problem hiding this comment.
could you explain the logic here? I don't see currentRequest getting redefined. Just on line 47
There was a problem hiding this comment.
Yes, it is only defined on line 47. If the response for a request is processed after a newer request is sent, the currentRequest objects don't match and thus the response is discarded.
stores/NodeInfoStore.ts
Outdated
| @action | ||
| public getNodeInfo = () => { | ||
| this.errorMsg = ''; | ||
| this.loading = true; | ||
| const currentRequest = (NodeInfoStore.currentRequest = {}); |
There was a problem hiding this comment.
shouldn't this.currentRequest suffice in place of NodeInfoStore.currentRequest?
There was a problem hiding this comment.
It is NodeInfoStore.currentRequest because the property is static. Static properties cannot be accessed using this.. My idea was to make it static to discard responses even if there are multiple instances of NodeInfoStore (which should never be the case).
But I realized that this makes no sense since even if there were multiple instances, they all have their own state and then they should also have their own currentRequest object.
Description
This fixes #1492 and fixes #1479.
#1479 was caused by cached backend calls being used although the node config was changed. So the calls are now removed when reconnecting to a node.
#1492 was due to results of older requests being received and processed after the results of newer requests. Now, results of non-latest requests are generally discarded.
This pull request is categorized as a:
Checklist
yarn run tscand made sure my code compiles correctlyyarn run lintand made sure my code didn’t contain any problematic patternsyarn run prettierand made sure my code is formatted correctlyyarn run testand made sure all of the tests passTesting
If you modified or added a utility file, did you add new unit tests?
I have tested this PR on the following platforms (please specify OS version and phone model/VM):
I have tested this PR with the following types of nodes (please specify node version and API version where appropriate):
Locales
Third Party Dependencies and Packages
yarnafter this PR is merged inpackage.jsonandyarn.lockhave been properly updatedOther: