-
Notifications
You must be signed in to change notification settings - Fork 662
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 and Add Logging for StateMachine websocket close #537
Fix and Add Logging for StateMachine websocket close #537
Conversation
Codecov Report
@@ Coverage Diff @@
## master #537 +/- ##
=======================================
Coverage 80.28% 80.28%
=======================================
Files 6 6
Lines 279 279
Branches 43 43
=======================================
Hits 224 224
Misses 38 38
Partials 17 17 Continue to review full report at Codecov.
|
this.logger.debug(`disconnected after unexpected close ${context.eventPayload.reason} | ||
${context.eventPayload.code} with isMonitoring set to ${this.keepAlive.isMonitoring} | ||
and recommendReconnect set to ${this.keepAlive.recommendReconnect}`); | ||
// this transition circumvents the 'disconnecting' state (since the websocket is already closed), |
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 attention to detail in the comments!
@shanedewael i know there's no testing rig for RTMClient at the moment, but it would be useful to know if you're done manual testing, and what that manual testing code might have looked like. |
@aoberoi I manually hacked together a way to reproduce the bugs by emitting a handshake event in |
Summary
Addresses #531 and #527. Adds flags to
KeepAlive
and logs in client on unexpected websocket close duringhandshaking
state. Also addresses race condition that could cause this unexpected close.Requirements (place an
x
in each[ ]
)