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

[feature] Set timeout like 10 seconds to a connection #770

Open
Zhaoxun opened this issue Mar 18, 2022 · 4 comments
Open

[feature] Set timeout like 10 seconds to a connection #770

Zhaoxun opened this issue Mar 18, 2022 · 4 comments

Comments

@Zhaoxun
Copy link

Zhaoxun commented Mar 18, 2022

Is your feature request related to a problem? Please describe.

A clear and concise description of what the problem is - e.g. "I'm always frustrated when [...]"

Most long lasting websocket connections need heartbeats between two peers. And it is a common practice to acknowledge the corruption of the connection after a silence for too long. So "Timeout" feature is common in websocket or TCP connections.

However, I find this timeout equivalent to be unusable.

https://pkg.go.dev/github.com/gorilla/websocket#Conn

func (c *Conn) SetReadDeadline(t time.Time) error

SetReadDeadline sets the read deadline on the underlying network connection. After a read has timed out, the websocket connection state is corrupt and all future reads will return an error. A zero value for t means reads will not time out.

Since time.Time only represents a particular time point like time.Now(), this methods means that the Deadline is one given future time, rather than a recurring tick.

Describe the solution you'd like

I think it is better to classify t as time.Duration and implement that feature.

Describe alternatives you've considered

Are there alternatives you've tried, and/or workarounds in-place?

@Zhaoxun
Copy link
Author

Zhaoxun commented Mar 21, 2022

Call c.SetDeadline(time.Now().Add(10 * time.Second)) before the operations that you want to timeout within 10 seconds.

The standard net package originally used timeouts, but was modified to use deadlines instead. The commit message for the change explains why. I think the reasoning applies to this package as well.

"Previously, a timeout (in int64 nanoseconds) applied to a granularity even smaller than one operation: a 100 byte read with a 1 second timeout could take 100 seconds, if the bytes all arrived on the network 1 second apart. This was confusing."

That post 10 yrs ago seems to get around the bug by cutting the features other languages take for granted. I don't think that workaround on http protocol fits the requirements of a long TCP connection such as websocket.

@Zhaoxun
Copy link
Author

Zhaoxun commented Mar 22, 2022

What is the granularity for your proposed timeout feature? Does it apply to each call to the connection or to the entire message? How does a pong handler called from NextReader extend the timeout?

I am not talking about a temporary connection to download like 10 MB file. On this occasion, http or ftp would satisfy the demand. I am talking about websocket connections that last ast least for hours, even for months. The routers along the connection would simply break it if silence remain for 1 minute. Therefore some heartbeat mechanism (yes it could be ping pong or custom made heartbeat) is necessary to keep the routers from shutting the connection down.

On top of the heartbeat one peer would simply judge the connection corrupt, there comes the timeout. If the heartbeat is once per 3 seconds, I could safely set the timeout as 4 seconds, and the program would break the connection by throwing out an error after 4 seconds without getting one incoming message including the heartbeat. This simple mechanism is widely used on automated websocket servers for High Availability. Other languages like Java and Python has already implemented this feature basic and solid. Without this feature, the disconnection event could be as late as half a minute delay by notification of the operating system.

@ghost
Copy link

ghost commented Mar 22, 2022

Oh, you are asking for a heartbeat or keep-alive feature.

I got off track because because of the discussion of deadlines vs. timeouts. I thought you were saying that deadline does not allow the application to implement heartbeats, but timeout does. How the application specifies when to fail an operation on the connection is different from a heartbeat.

See the chat, command and file watch examples in this repository. These examples implement ping/ping heartbeats.

@thavlik
Copy link

thavlik commented May 3, 2022

While you can always implement heartbeats at the application level - which what we're all most likely doing now - this really is a protocol-level feature. It would be awesome if gorilla had a way of handling it internally, though client compatibility may be such that it's significantly easier to implement at the application level.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Development

No branches or pull requests

2 participants