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

Server stream does not detect client's disconnection #57

Closed
osechet opened this issue Jun 19, 2017 · 8 comments
Closed

Server stream does not detect client's disconnection #57

osechet opened this issue Jun 19, 2017 · 8 comments

Comments

@osechet
Copy link

osechet commented Jun 19, 2017

In my application, the client requests data that is returned through a stream. But if the client crashes, the server keeps writing into the stream without any error.

@MarcusLongmuir
Copy link
Contributor

Thanks for raising. Can you provide a repro please?

@atamgp
Copy link

atamgp commented Oct 25, 2017

Hi, I have the same problem. In the stream example a servers pushes and ends the stream. But in this context (which i also have) the server has many clients and sends endless. Clients come and go.
Current behaviour is indeed that when clients are killed the server still sends without getting an error

err := stream.Send(rate)

It should give an error even when a client abruptly gets killed with no clean Close()

Thanks for fixing..

@jsmouret
Copy link

Starting from the example server, replace the method in https://github.com/improbable-eng/grpc-web/blob/master/example/go/exampleserver/exampleserver.go with:

func (s *bookService) QueryBooks(bookQuery *library.QueryBooksRequest, stream library.BookService_QueryBooksServer) error {
	for {
		grpclog.Info("sending to", stream)
		err := stream.Send(books[0])
		if err != nil {
			grpclog.Error("got error", err, "from", stream)
			break
		}
		time.Sleep(time.Second)
	}
	grpclog.Info("done with", stream)
	return nil
}

The idea is to send a message every second.

The output from the server looks like:

exampleserver: 2017/11/28 03:29:06 Starting server. http port: 9090, with TLS: false
exampleserver: 2017/11/28 03:29:14 sending to&{0xc0421b4180}
exampleserver: 2017/11/28 03:29:15 sending to&{0xc0421b4180}
exampleserver: 2017/11/28 03:29:16 sending to&{0xc0421b4180}
exampleserver: 2017/11/28 03:29:17 sending to&{0xc0421b4180}  # Reloading the webpage
exampleserver: 2017/11/28 03:29:17 sending to&{0xc0421d8180}  # Note that there are now 2 different clients
exampleserver: 2017/11/28 03:29:18 sending to&{0xc0421b4180}
exampleserver: 2017/11/28 03:29:18 sending to&{0xc0421d8180}
exampleserver: 2017/11/28 03:29:19 sending to&{0xc0421b4180}  # Reloading again
exampleserver: 2017/11/28 03:29:19 sending to&{0xc0421d8180}  # 3 clients and no errors
exampleserver: 2017/11/28 03:29:19 sending to&{0xc04222a280}
exampleserver: 2017/11/28 03:29:20 sending to&{0xc0421b4180}
exampleserver: 2017/11/28 03:29:20 sending to&{0xc0421d8180}
exampleserver: 2017/11/28 03:29:20 sending to&{0xc04222a280}
exampleserver: 2017/11/28 03:29:21 sending to&{0xc0421b4180}
exampleserver: 2017/11/28 03:29:21 sending to&{0xc0421d8180}
exampleserver: 2017/11/28 03:29:21 sending to&{0xc04222a280}
exampleserver: 2017/11/28 03:29:22 sending to&{0xc0421b4180}
exampleserver: 2017/11/28 03:29:22 sending to&{0xc0421d8180}
exampleserver: 2017/11/28 03:29:22 sending to&{0xc04222a280}

Thank you

@jsmouret
Copy link

jsmouret commented Nov 27, 2017

Workaround is to check the stream context:

err = stream.Context().Err()
if err != nil {
	break
}

@easyCZ
Copy link
Contributor

easyCZ commented Mar 31, 2018

Closing due to inactivity. Please comment if you'd like to revisit this issue.

@easyCZ easyCZ closed this as completed Mar 31, 2018
@lukaszx0
Copy link

The workaround provided by @jsmouret does indeed work, but shouldn't err := stream.Send(e); return error when browser closed connection and there's nothing to write to as stream should be closed at this point? Reproducing it should be very simple using @jsmouret ticker example on server side and opening a browser tab which opens the stream and then closing the tab - server will continue writing to stream, without rising an error unless you'll check Context() explicit.

@easyCZ thoughts?

@raliste
Copy link

raliste commented Jun 7, 2018

Anything like http CloseNotify?

@Virtual-felix
Copy link

I have the exact same issue but I would like to handle it in the proxy server, in which I don't have access to the stream handler so I can't put this work around.
I tried to create a grpc.StreamServerInterceptor, in which I wrapped the grpc.ServerStream in a wrapper that override the method SendMsg, I wanted to get the context from this method and do something but the context doesn't return any errors even if the client has crashed or closed the tab.
Any suggestions ?

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

No branches or pull requests

8 participants