feat: reconnect refactor

[SSebo]

@nshaaban-cPacket This PR is based on your POC 👍 @nshaaban-cPacket @1c3t3a PTAL

[codecov[bot]]

Codecov Report

Merging #229 (76193b7) into main (7d5bbb4) will increase coverage by 1.06%. The diff coverage is 89.68%.

@@            Coverage Diff             @@
##             main     #229      +/-   ##
==========================================
+ Coverage   86.38%   87.44%   +1.06%     
==========================================
  Files          30       31       +1     
  Lines        2350     2478     +128     
==========================================
+ Hits         2030     2167     +137     
+ Misses        320      311       -9     

Impacted Files	Coverage Δ
socketio/src/client/callback.rs	37.50% <ø> (ø)
socketio/src/error.rs	100.00% <ø> (ø)
socketio/src/socket.rs	78.49% <66.66%> (+4.02%)	:arrow_up:
socketio/src/client/raw_client.rs	87.17% <87.17%> (ø)
socketio/src/client/client.rs	94.91% <94.59%> (+9.05%)	:arrow_up:
engineio/src/client/client.rs	94.60% <100.00%> (+0.71%)	:arrow_up:
engineio/src/packet.rs	97.95% <100.00%> (ø)
engineio/src/socket.rs	77.77% <100.00%> (+3.70%)	:arrow_up:
socketio/src/client/builder.rs	92.95% <100.00%> (+2.18%)	:arrow_up:
socketio/src/lib.rs	100.00% <100.00%> (ø)
... and 6 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[SSebo]

Test case is stable now by checking Open and Close callback.

[SSebo]

@nshaaban-cPacket looks like not review the latest code? I force pushed code 3 days ago.

[SSebo]

solution three: rename ReconnctingClient to Client (and take it's place), and rename existing client to InternalClient

@nshaaban-cPacket I tried, found that InternalClient is hard to hide from user because of the callback parameter. Compiler told me that trait is not object safe if I move emit function with generic type in.

[SSebo]

@1c3t3a Compiler does not complain until I create a trait object Box<dyn ClientTrait>:

--> socketio/src/client/reconnect.rs:40:8 | 14 | pub trait ClientTrait { | ----------- this trait cannot be made into an object... ... 40 | fn emit<E, D>(&self, event: E, data: D) -> Result<()> | ^^^^ ...because method emit has generic type parameters ... 80 | fn emit_with_ack<F, E, D>( | ^^^^^^^^^^^^^ ...because method emit_with_ack has generic type parameters = help: consider moving emit to another trait = help: consider moving emit_with_ack to another trait

For more information about this error, try rustc --explain E0038. error: could not compile rust_socketio due to previous error

[ctrlaltf24]

If this is a breaking change anyways, we could expose BOTH ReconnectingClient and Client in the public interface

[1c3t3a]

@1c3t3a Compiler does not complain until I create a trait object Box<dyn ClientTrait>:

--> socketio/src/client/reconnect.rs:40:8 | 14 | pub trait ClientTrait { | ----------- this trait cannot be made into an object... ... 40 | fn emit<E, D>(&self, event: E, data: D) -> Result<()> | ^^^^ ...because method emit has generic type parameters ... 80 | fn emit_with_ack<F, E, D>( | ^^^^^^^^^^^^^ ...because method emit_with_ack has generic type parameters = help: consider moving emit to another trait = help: consider moving emit_with_ack to another trait

For more information about this error, try rustc --explain E0038. error: could not compile rust_socketio due to previous error

What is your definition of ClientTrait, and why do we necessarily need it? Anyways I agree with what @nshaaban-cPacket mentions and expose both versions of the client.

[SSebo]

@1c3t3a Compiler does not complain until I create a trait object Box<dyn ClientTrait>: --> socketio/src/client/reconnect.rs:40:8 | 14 | pub trait ClientTrait { | ----------- this trait cannot be made into an object... ... 40 | fn emit<E, D>(&self, event: E, data: D) -> Result<()> | ^^^^ ...because method emit has generic type parameters ... 80 | fn emit_with_ack<F, E, D>( | ^^^^^^^^^^^^^ ...because method emit_with_ack has generic type parameters = help: consider moving emit to another trait = help: consider moving emit_with_ack to another trait For more information about this error, try rustc --explain E0038. error: could not compile rust_socketio due to previous error

What is your definition of ClientTrait, and why do we necessarily need it? Anyways I agree with what @nshaaban-cPacket mentions and expose both versions of the client.

@1c3t3a If we expose both versions of client，we do not need this.

[milandekruijf]

I've been testing this for a while now and it works great. I did notice that reconnects don't happen when losing internet connection, it just simply does nothing and stops emitting events.

[SSebo]

I've been testing this for a while now and it works great. I did notice that reconnects don't happen when losing internet connection, it just simply does nothing and stops emitting events.

@milandekruijf @1c3t3a @nshaaban-cPacket I found the js version of engine.io client will close connection if ping time out. https://github.com/socketio/engine.io-client/blob/dfee8ded722a8c4f9f773505d0c77b4561569863/lib/socket.ts#L645. Suggest @milandekruijf to open a new issue, because the change will be in engine.io layer

[TomieAi]

when this feature gonna be available? pls

[ctrlaltf24]

What's left to do here:

• [x] expose reconnecting client as Client in external API (returned from connect function on the ClientBuilder)
• [x] expose Client as RawClient (or something along those lines) in public API
• [ ] (optional) expose second function on builder called connect_raw or something along those lines.
• [x] bump MINOR version
• [ ] MERGE!

@SSebo

(Listen to @1c3t3a over me if he says anything different)

[1c3t3a]

Sounds good! From my experience it was never bad advice to listen to @nshaaban-cPacket :) I would accept any PR that fixes the existing questions.

[SSebo]

@1c3t3a @nshaaban-cPacket PTAL