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

Document how to have a separate idle and read timeout #87

Closed
nhooyr opened this issue Jun 3, 2019 · 5 comments
Closed

Document how to have a separate idle and read timeout #87

nhooyr opened this issue Jun 3, 2019 · 5 comments
Labels
Milestone

Comments

@nhooyr
Copy link
Contributor

nhooyr commented Jun 3, 2019

func read(ctx context.Context, c *websocket.Conn) ([]byte, error) {
	ctx, cancel := context.WithTimeout(ctx, time.Minute*30)
	defer cancel()

	_, r, err := c.Reader(ctx)
	if err != nil {
		return nil, err
	}

	// One minute max to read the received message.
	time.AfterFunc(time.Minute, cancel)

	return ioutil.ReadAll(r)
}

That code should be good in an example under Reader.

Related #86

@nhooyr nhooyr added the docs label Jun 3, 2019
@nhooyr
Copy link
Contributor Author

nhooyr commented Jun 3, 2019

If enough people need this, I can add a method to the conn c.SetReadTimeout that controls the maximum time spent reading each message. Then the context passed in would effectively act as the idle timeout.

nhooyr added a commit that referenced this issue Jun 5, 2019
nhooyr added a commit that referenced this issue Jun 5, 2019
@nhooyr nhooyr closed this as completed in 22638e0 Jun 5, 2019
@nhooyr nhooyr mentioned this issue Jun 24, 2019
@mikestead
Copy link

mikestead commented Sep 13, 2019

@nhooyr thanks for all the work on this library. Add me as a vote for this option

Specifically it currently looks like I'd have to unwrap the logic encapsulated in wsjson.Read if I wanted to have that functionality with this option. Let me know if not.

@nhooyr
Copy link
Contributor Author

nhooyr commented Sep 13, 2019

@nhooyr thanks for all the work on this library. Add me as a vote for this option

Thanks :)

Specifically it currently looks like I'd have to unwrap the logic encapsulated in wsjson.Read if I wanted to have that functionality with this option. Let me know if not.

Yes, unfortunately you would need to unwrap the logic in wsjson. For now, I would recommend going that route as no one else has requested this option.

Is there a reason you can't just bound the entire wsjson read to something like a few minutes? As long as you have a read limit, things should be fine.

@mikestead
Copy link

No problem.

It was with the aim to have checks over the two different steps. A general longer term read wait timeout and a specific timeout once we actually have something to read.

@nhooyr
Copy link
Contributor Author

nhooyr commented Nov 26, 2020

Looks like this was lost somehow, will need to investigate why it isn't in master anymore.

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

2 participants