-
Notifications
You must be signed in to change notification settings - Fork 35
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
switch to websockets #358
base: master
Are you sure you want to change the base?
switch to websockets #358
Conversation
✅ Deploy Preview for cheery-moxie-4f1121 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@@ -107,20 +107,159 @@ export class Network { | |||
} | |||
} | |||
|
|||
/** | |||
* | |||
*/ | |||
export class ExplorerNetwork extends Network { |
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.
I think we should keep both classes, for explorers that may not support websockets, or just to give the option to users
async awaitWebSocketStatus(status, errMessage) { | ||
for (let i = 0; i < 100; i++) { | ||
if (this.ws.readyState === status) { | ||
break; | ||
} | ||
await sleep(100); | ||
} | ||
if (this.ws.readyState !== status) { | ||
throw new Error(errMessage); | ||
} | ||
} |
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.
Use onopen event
async sendAndWaitForAnswer(method, params) { | ||
let attempt = 1; | ||
const maxAttempts = 10; | ||
// milliseconds | ||
const maxAwaitTime = 600 * 1000; | ||
const frequency = 100; | ||
while (attempt <= maxAttempts) { | ||
let receivedInvalidAnswer = false; | ||
const [id, ws_that_sent] = await this.send(method, params); | ||
for (let i = 0; i < Math.floor(maxAwaitTime / frequency); i++) { | ||
const res = this.cachedResults[id]; | ||
if (res !== undefined) { | ||
delete this.cachedResults[id]; | ||
if (res.error) { | ||
await sleep(1000); | ||
receivedInvalidAnswer = true; | ||
break; | ||
} | ||
return res; | ||
} | ||
await sleep(frequency); | ||
// If connection got closed while sending do not wait until timeout | ||
if (ws_that_sent !== this.ws) { | ||
break; | ||
} | ||
} | ||
debugWarn( | ||
DebugTopics.NET, | ||
'Failed send attempt for ' + method, | ||
'attempt ' + | ||
attempt + | ||
'/' + | ||
maxAttempts + | ||
' received an invalid answer: ' + | ||
receivedInvalidAnswer | ||
); | ||
attempt += 1; | ||
} | ||
throw new Error('Failed to communicate with the explorer'); | ||
} |
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.
Instead of making heavy use of sleep
, this should use some form of observer/event and be refactored in another class
let attempt = 1; | ||
const maxAttempts = 10; | ||
// milliseconds | ||
const maxAwaitTime = 600 * 1000; |
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.
We shouldn't have a maxAwaitTime, we should only abort if the websocket closes
Abstract
Replace the legacy HTTP requests based system with a websocket implementation.
Testing
At the moment this is testable only on mainnet (Duddino's testnet explorer needs to be updated still).
What I suggest for testing: