Skip to content

Conversation

@Yoann-Abbes
Copy link
Contributor

What does this PR do?

Fixes the ping clearTimeout

How should this be manually tested?

  • Step 1 :
  • Step 2 :
  • Step 3 :
    ...

Other changes

Boyscout

@codecov
Copy link

codecov bot commented Feb 25, 2021

Codecov Report

Merging #607 (13d7092) into master (9eaf648) will increase coverage by 0.20%.
The diff coverage is 77.77%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #607      +/-   ##
==========================================
+ Coverage   86.54%   86.75%   +0.20%     
==========================================
  Files          33       33              
  Lines        1501     1502       +1     
  Branches      262      262              
==========================================
+ Hits         1299     1303       +4     
+ Misses        149      146       -3     
  Partials       53       53              
Impacted Files Coverage Δ
src/protocols/abstract/Realtime.ts 97.14% <ø> (+2.54%) ⬆️
src/protocols/WebSocket.ts 79.12% <77.77%> (+2.98%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9eaf648...73dce99. Read the comment docs.

Copy link
Contributor

@Aschen Aschen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add basic unit test for this please?

* Called when the client's connection is closed
*/
clientDisconnected () {
clearInterval(this.pingIntervalId);
Copy link
Contributor

@scottinet scottinet Feb 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Realtime object is a generic class that does not know anything about those properties, since they are specific to the websocket protocol

What you can do is to add an overload to these Realtime methods in the WebSocket class, clear the timers there, and then invoke the overloaded methods

@Yoann-Abbes Yoann-Abbes merged commit fce607b into master Feb 25, 2021
@Yoann-Abbes Yoann-Abbes deleted the fix-ping-pong-timeout-clear branch February 25, 2021 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants