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

Create new websocket function websocket_check_pingpong #1744

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

danidee10
Copy link

@danidee10 danidee10 commented Feb 18, 2018

This adds a new method called websocket_check_pingpong #1717

Right now, It's merely a wrapper around uwsgi_websocket_recv_do (like websocket_recv and websocket_recv_nb) and the non-blocking behaviour is hardcoded.

I tried to call the C function uwsgi_websockets_check_pingpong but I got errors and since the internals of uWSGI isn't documented, I didn't really know what to do. I believe some things must be done on the wsgi_request (e.g uwsgi_websocket_parse_header(wsgi_req);) before trying to send a ping or receive a pong.

The only solution was to copy most of the code from uwsgi_websocket_recv_do but that doesn't seem right.

I'm submitting this PR (Not to be merged in its current state) because I don't really know how to proceed and how to handle non-blocking and blocking mode that's an argument to uwsgi_websocket_recv_do (It's only used in one place in the function body so I feel it can easily be removed) .

Obviously there are no tests yet 😄

@danidee10
Copy link
Author

This is loosely related to #1533 because with the new function I still experience the same No PONG received in 3 seconds and currently the only workaround is to kill the uWSGI process which forces it to restart (provided uWSGI is started with it's --master process). and on the client side try and re-establish a connection.

If there's a separate function for handling pings and pongs it'll be easier to solve the problem since we won't be debugging code that has to deal with receiving of packets.

@maxim-s-barabash
Copy link

the proposed solution #1261

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants