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

Web socket timeouts in 0.9.5 #1384

Closed
splack opened this issue Jan 27, 2017 · 9 comments
Closed

Web socket timeouts in 0.9.5 #1384

splack opened this issue Jan 27, 2017 · 9 comments

Comments

@splack
Copy link

splack commented Jan 27, 2017

1. What version of Caddy are you running (caddy -version)?

0.9.5

2. What are you trying to do?

Reverse proxy drone.io server web interface. I noticed that websockets were being closed and the web UI was not updating after a small time.

3. What is your entire Caddyfile?

Not working:

localhost:2015 {
  proxy /ws localhost:12345 {
    websocket
  }
}

localhost:12345 {
  websocket /bin/cat
}

Working:

localhost:2015 {
  timeouts {
    read 0
  }
  proxy /ws localhost:12345 {
    websocket
  }
}

localhost:12345 {
  websocket /bin/cat
}

Test html:

<html>
<head></head>
<body>
    <h1>WebSocket Echo Test</h1>
    <form>
        <p>
            Message: <input id="message" type="text" value="Hello, world!">
        </p>
    </form>
    <button id="sendButton" disabled="true" onclick="send();">Send Message</button>
    <button id="openButton" onclick="logPre.innerHTML='';">Open WebSocket</button>
    <pre id="log" />
    <script type="text/javascript">
        var state = ["CONNECTING", "OPEN", "CLOSING", "CLOSED"]
        var sock = null;
        var wsuri = ("https:" === window.location.protocol ? "wss:" : "ws:") +
            "//" + window.location.host + "/ws/echo"
        var logPre = document.getElementById("log");
        var sendButton = document.getElementById("sendButton");
        var openButton = document.getElementById("openButton");
        function log(msg) {
            console.log(msg);
            var d = new Date().getTime() / 1000;
            logPre.innerHTML = logPre.innerHTML + d + "\t" + "sock is " +
                state[sock.readyState] + "\t" + msg + "\n";
        }
        function send() {
            var msg = document.getElementById("message").value;
            log("sending: " + msg);
            try {
                sock.send(msg);
            } catch (e) {
                log(e)
            }
        };
        window.onload = function() {
            console.log("onload");
            sock = new WebSocket(wsuri);
            send();
            sock.onopen = function() {
                sendButton.disabled = false;
                openButton.disabled = true;
                log("connected to " + wsuri);
                for (i = 1000; i <= 12000; i += 1000) {
                    window.setTimeout(send, i);
                }
            }
            sock.onclose = function(e) {
                sendButton.disabled = true;
                openButton.disabled = false;
                log("connection closed (" + e.code + ")");
            }
            sock.onmessage = function(e) {
                log("received: " + e.data);
            }
        };
    </script>
</body>
</html>

4. How did you run Caddy (give the full command and describe the execution environment)?

./caddy

5. What did you expect to see?

Open web sockets do not close after 10 seconds.

I think I've narrowed it down to the new default 10s read timeout. Should web sockets bypass the read timeout since the socket state is known?

6. What did you see instead (give full error messages and/or log)?

Web socket is being closed by caddy after 10 seconds.

7. How can someone who is starting from scratch reproduce this behavior as minimally as possible?

Use the files and html above.

@mholt
Copy link
Member

mholt commented Jan 27, 2017

Yes, if you require longer connections, you should disable the default timeouts (or extend them to some arbitrarily longer value).

timeouts none

The release notes describe this: https://github.com/mholt/caddy/releases/tag/v0.9.5

@mholt mholt closed this as completed Jan 27, 2017
@splack
Copy link
Author

splack commented Jan 27, 2017

I have read the release notes but I disagree that the read timeout should apply to websockets. The other use cases such as long polling are an abuse of http and would need a workaround but websockets are the standardised way to achieve bi directional communication.

https://html.spec.whatwg.org/multipage/comms.html#ping-and-pong-frames defines the behaviour of ping pong specifically to keep the connection alive.

The scope of timeout is per vhost and I would like it to not apply to websockets or have another timeout specific to webwockets.

Another approach could be to disable the timeout whenever a websocket connection is established or switch to a websocket timeout. I'd be happy to implement this option and send a PR.

@mholt
Copy link
Member

mholt commented Jan 27, 2017

@splack

The scope of timeout is per vhost

Unfortunately, that's not the case. https://caddyserver.com/docs/timeouts:

Because timeouts apply to an entire HTTP server which may serve multiple sites defined in your Caddyfile, the timeout values for each site will be reduced to their minimum values (with 0, or disabled, being the lowest) across all sites that share that server.

Go lets us define timeouts at the HTTP server level, not for each connection.

@splack
Copy link
Author

splack commented Jan 27, 2017

Understood. I just read golang/go#16100 and can see the issues.

@mholt
Copy link
Member

mholt commented Jan 27, 2017

Yup, that's the main rub. I'm really interested in making things "just work" here. There's probably some way of doing it by subverting the standard library but... I'm not sure that's a good idea.

@alfiepates
Copy link

@mholt

The release notes describe this: https://github.com/mholt/caddy/releases/tag/v0.9.5

The documentation does not. I don't read the release notes to figure out why my websockets aren't working, I read the websockets documentation.

Please do not make "breaking" changes like this without updating the documentation.

@mholt
Copy link
Member

mholt commented Mar 20, 2017

Hey Alfie, this is in the documentation: https://caddyserver.com/docs/timeouts

screen shot 2017-03-20 at 3 37 16 pm

I don't read the release notes to figure out why my websockets aren't working

You should always read release notes before upgrading, though. Especially before 1.0.

Please do not make "breaking" changes like this without updating the documentation.

I know this is inconvenient, sorry about that. But I do reserve the right to make breaking changes intentionally before version 1.0:

Caddy is not yet 1.0, meaning that individual releases are deemed stable, but Caddy may change between releases in a way that is not backward-compatible.

The post-1.0 compatibility guarantee is still being developed.

As this is a security feature, I deemed it important enough to impose on users by default, as it follows Caddy's goal to be secure by default. It's being removed in the next release, though, because of some complex bugs/edge cases related to timeouts upstream in Go's standard library.

Thanks for understanding. And for your patience. 😇

@windymindy
Copy link

@mholt Hi.
Regarding #1416 .
Don't you think, that it is up to proxied-to service to decide whom to timeout? On the other hand, setting basic http timeouts is essential.
As far as I understand, there are several difficulties on the way to implement timeouts as proper.

  1. Go std api permits setting timeouts per server instead of per connection.
  2. Apparently, There are bugs in Go std related to timeouts.

Given these two, can we speculate about implementing custom timeouts without tweaking std?

P.S. I am certainly having perfomance issues and random disconnections using caddy websocket proxy, but I am not ready to create a reproducable bug report (version is 0.9.5, timeouts set to none).

@mholt
Copy link
Member

mholt commented Apr 18, 2017

@windymindy Timeouts will be disabled in the next release coming out in 2 days. And yes, those two problems are the main hangups right now (pun intended) with timeouts. Go 1.9 or even sooner releases might have some of the fixes we'll need to make timeouts more robust/useful.

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

No branches or pull requests

4 participants