-
-
Notifications
You must be signed in to change notification settings - Fork 139
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
Fix node connection issues #1504
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -38,12 +38,18 @@ export default class NodeInfoStore { | |
this.loading = true; | ||
}; | ||
|
||
private static currentRequest: any; | ||
|
||
@action | ||
public getNodeInfo = () => { | ||
this.errorMsg = ''; | ||
this.loading = true; | ||
const currentRequest = (NodeInfoStore.currentRequest = {}); | ||
BackendUtils.getMyNodeInfo() | ||
.then((data: any) => { | ||
if (NodeInfoStore.currentRequest !== currentRequest) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could you explain the logic here? I don't see currentRequest getting redefined. Just on line 47 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
return; | ||
} | ||
const nodeInfo = new NodeInfo(data); | ||
this.nodeInfo = nodeInfo; | ||
this.testnet = nodeInfo.isTestNet; | ||
|
@@ -52,6 +58,9 @@ export default class NodeInfoStore { | |
this.error = false; | ||
}) | ||
.catch((error: any) => { | ||
if (NodeInfoStore.currentRequest !== currentRequest) { | ||
return; | ||
} | ||
// handle error | ||
this.errorMsg = ErrorUtils.errorToUserFriendly( | ||
error.toString() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1026,6 +1026,7 @@ export default class SettingsStore { | |
if (status) { | ||
this.error = false; | ||
this.errorMsg = ''; | ||
BackendUtils.clearCachedCalls(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we should probably check first if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is not necessary since the call function of BackendUtils already checks this. |
||
} | ||
this.connecting = status; | ||
return this.connecting; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't
this.currentRequest
suffice in place ofNodeInfoStore.currentRequest
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is
NodeInfoStore.currentRequest
because the property is static. Static properties cannot be accessed usingthis.
. My idea was to make it static to discard responses even if there are multiple instances ofNodeInfoStore
(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.