Skip to content
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

[ws transport]: soketto occasionally returns UnexpectedOpCode(Continue) when reading empty WebSocket message #154

Closed
niklasad1 opened this issue Nov 3, 2020 · 3 comments
Labels
bug Something isn't working

Comments

@niklasad1
Copy link
Member

niklasad1 commented Nov 3, 2020

I have noticed a difference between soketto v0.3 and soketto v0.4 where it sometimes reads an empty WebSocket message and returns UnexpectedOpCode(Continue).

For the client it doesn't make much difference than just that ERROR jsonrpsee::client::ws::client] Client Error: Inner(Ws(UnexpectedOpCode(Continue))) would be logged, still quite annoying.

For the server, it's critical because it would terminate the connection but I haven't seen it yet at least.... it might just soketto::Client related but receive_data and send_data seems to have the same implemetation.

Note: I tried to downgrade soketto v0.3.2 but it didn't manage to get the ws client/server to work.

@niklasad1 niklasad1 added the bug Something isn't working label Nov 3, 2020
@niklasad1
Copy link
Member Author

niklasad1 commented Nov 9, 2020

As Toralf described,

The futures in soketto::receive_data is not cancelable and currently, those futures are spawned in a loop from client -> raw client::next_event -> ws transport::next_response. Thus, it's possible that futures are dropped if the from_front futures finishes first and the client_event/soketto::receive_data future gets dropped.

It was fixed in the WebSocket server: #114 and similar fix should be done in the client to verify if that's issue here (most likely)

However, it will require some refactoring to convert the future into a stream because the client is used for other things too and stream::unfold takes ownership of the closure AFAIK.

@niklasad1
Copy link
Member Author

//cc @maciejhirsz could it be related to "bad performance" i.e, spawning unnecessary futures (bloat the executor thread pool)?

@niklasad1
Copy link
Member Author

Fixed by #180

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant