-
Notifications
You must be signed in to change notification settings - Fork 720
[DO NOT MERGE] Spencer/sdk/3.9.6 #1658
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
base: arjun/3.9.5
Are you sure you want to change the base?
Conversation
🟡 Heimdall Review Status
|
e1ca664 to
fc22bb4
Compare
fc22bb4 to
05676cc
Compare
05676cc to
f0ec246
Compare
|
|
||
| // Check if WebSocket is actually open before sending | ||
| if (webSocket.readyState !== WebSocket.OPEN) { | ||
| this.pendingData.push(data); |
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.
maybe i dont fully understand the life cycle of this class, but if we are constructing new WLWS for new connection, think we are losing this object level pendingData?
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.
The WalletLinkWebsocket is kind of a wrapper around the actual websocket
so
WalletLinkWebsocket
has a:
- websocket
- pendingData array
and only the internal websocket is reconstructed
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.
Ty for correcting me here - there are places we re-instantiate the whole class. Discussed a path forward.
fan-zhang-sv
left a comment
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.
LGTM! thx Spencer for tackling this issue!
Summary
Connection Management:
Heartbeat Improvements:
Browser Event Handling:
How did you test your changes?
Failure 1 (before fixes)
commercefail1.mp4
Failure 2 (before fixes)
commercefail2.mp4
Success with fixes
commercesuccess1.mp4