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

noVNC does not expose WebSocket status code on failed connection. #491

Closed
MOZGIII opened this issue Jun 6, 2015 · 13 comments
Closed

noVNC does not expose WebSocket status code on failed connection. #491

MOZGIII opened this issue Jun 6, 2015 · 13 comments
Labels

Comments

@MOZGIII
Copy link
Contributor

MOZGIII commented Jun 6, 2015

It would be useful if I could get the HTTP response code returned from the server during WebSocket request in case it fails before handshake.

In example, in case of 503 code I cound reconnect after some time, but that makes sense only if the response code is 503.

A nice place to catch those kind of thing would be updateState function, so I guess, the way to implement this is to carry on this information with the state update or save it somewhere in the RFB object.

@DirectXMan12
Copy link
Member

👍 Sounds like a good idea.

@MOZGIII
Copy link
Contributor Author

MOZGIII commented Jun 10, 2015

Does noVNC have any internal state that represents the "last error occured" on any level? I ask because it really depends on a design principles, and if there is already something like that, we could just add to it or add another internal state to memorize last error.
The solution is not perfect though, much better way would be to pass the full error around.

@DirectXMan12
Copy link
Member

hmm... so after a bit of investigation, it doesn't look like the WebSocket API actually provides an easy way to get the status code from the handshake. the CloseEvent provides a WebSocket status code, but this does not correspond to the HTTP status codes. Did you have a method that you knew of for obtaining this information?

@MOZGIII
Copy link
Contributor Author

MOZGIII commented Jun 10, 2015

Well, if websocket connection closes before the handshake, at least in chrome, I get a error in the log with the error code.

WebSocket connection to 'ws://website/tunnel' failed: Error during WebSocket handshake: Unexpected response code: 404

The idea is to catch the errors that occur after HTTP connection is established but before the WebSocket handshake happends, in this case the status code is immediately returned by the server (instead of 100 or whatever it is to continue/upgrade).

@MOZGIII
Copy link
Contributor Author

MOZGIII commented Jun 10, 2015

So, maybe try/catch would work? It seems like error acually happends during new WebSocket() call.

@kanaka
Copy link
Member

kanaka commented Jun 10, 2015

My understanding is that for security reasons, WebSocket error statuses are fairly restricted to limit the ability of malicious JS to probe an internal network. We certainly might be able to do better, but we will always be a bit limited in terms of how much we can know about the failure condition.

As an aside, in addition to restricting the information provided in error codes, most browsers also throttle the number of WebSocket connections a single page (probaby domain) is allowed to make within a certain amount of time.

@MOZGIII
Copy link
Contributor Author

MOZGIII commented Jun 10, 2015

Yeah, I guess there is really no good way to fetch status code...

So, any advice on how to implement reconnection after a certain amount of time? Probing in a loop with log scale intervals is what I have not, but this is, obviously, a terrible solution.

I want this behaviour for when my backend knows that the connection is legit to be opened, but the other side (VNC server) is just not ready yet.

Maybe something with headers? I think it would be inappropriate to introduce some internal protocol over WebSocket but before noVNC/RFB.

@DirectXMan12
Copy link
Member

@MOZGIII : you could modify websockify to send one of the status codes reserved for application use. noVNC currently just places the code in the status string that updateState is called with, but it might be a good idea to actually place the code in a separate argument as well (thoughts, @kanaka ?)

@MOZGIII
Copy link
Contributor Author

MOZGIII commented Jun 10, 2015

I am using my own backend, not websocksify, and it is already set up like that, though it does not work. I'm currently using code 503, but it's just a matter of choice, i.e. I used to have 418 I'm a teapot jff there. However niether of those codes are passed to the app... Any ideas of where to lookup for status codes that are hadled differently (non 1006 error)? I guess that would work.
Should I try to pass this problem to WebSocket spec authors somehow?

@MOZGIII
Copy link
Contributor Author

MOZGIII commented Jun 10, 2015

Speaking of the code, I think it would be great to just pass the error itself.

@DirectXMan12
Copy link
Member

I'm currently using code 503

So, my suggestion was that you should accept the connection, and then immediately send the WebSocket close frame with one of the WebSocket close codes (see the Mozilla Developer Center Documentation or RFC 6455 Section 7.4.2). Those closure codes get provided to the WebSocket onclose handler as part of the CloseEvent object. Currently, noVNC then places the code into the msg argument of onUpdateState, but we could change it so that it was also provided as a separate argument.

@MOZGIII
Copy link
Contributor Author

MOZGIII commented Jun 10, 2015

Oh, got it, this may do the trick, thanks.

@MOZGIII
Copy link
Contributor Author

MOZGIII commented Jun 10, 2015

By the way, @kanaka mentioned reconnection limit, and I updated to new version of noVNC and not it fails to reconnect, even though on previous version it worked. I believe there can be such thing as connection limitting, but I guess current version of noVNC, nonetheless, has a bug around related to it. See #495.

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

No branches or pull requests

3 participants