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.

Sign up for GitHub

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

Jump to bottom

request-server: don't return EOF if there is an unexpected error #392

Merged
merged 3 commits into from
Nov 18, 2020
Merged

request-server: don't return EOF if there is an unexpected error #392

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6af3f0a
request-server: don't return EOF if there is an unexpected error
drakkan Nov 5, 2020
8637833
request-server: add test cases for server errors
drakkan Nov 5, 2020
75eca30
request-server test: set size for the error channel to 1
drakkan Nov 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions request-server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,9 @@ func (rs *RequestServer) Serve() error {
// make sure all open requests are properly closed
// (eg. possible on dropped connections, client crashes, etc.)
for handle, req := range rs.openRequests {
if err == io.EOF {
err = io.ErrUnexpectedEOF
}
req.transferError(err)

delete(rs.openRequests, handle)
Expand Down
43 changes: 39 additions & 4 deletions request-server_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ import (
var _ = fmt.Print

type csPair struct {
cli *Client
svr *RequestServer
cli *Client
svr *RequestServer
svrResult chan error
}

// these must be closed in order, else client.Close will hang
Expand All @@ -39,6 +40,9 @@ func clientRequestServerPair(t *testing.T) *csPair {
skipIfPlan9(t)
ready := make(chan bool)
os.Remove(sock) // either this or signal handling
pair := &csPair{
svrResult: make(chan error),
drakkan marked this conversation as resolved.
Show resolved Hide resolved
}
var server *RequestServer
go func() {
l, err := net.Listen("unix", sock)
Expand All @@ -55,7 +59,8 @@ func clientRequestServerPair(t *testing.T) *csPair {
options = append(options, WithRSAllocator())
}
server = NewRequestServer(fd, handlers, options...)
server.Serve()
err = server.Serve()
pair.svrResult <- err
}()
<-ready
defer os.Remove(sock)
Expand All @@ -65,7 +70,9 @@ func clientRequestServerPair(t *testing.T) *csPair {
if err != nil {
t.Fatalf("%+v\n", err)
}
return &csPair{client, server}
pair.svr = server
pair.cli = client
return pair
}

func checkRequestServerAllocator(t *testing.T, p *csPair) {
Expand Down Expand Up @@ -718,6 +725,34 @@ func TestRequestReaddir(t *testing.T) {
checkRequestServerAllocator(t, p)
}

func TestCleanDisconnect(t *testing.T) {
p := clientRequestServerPair(t)
defer p.Close()

err := p.cli.conn.Close()
require.NoError(t, err)
// server must return io.EOF after a clean client close
// with no pending open requests
err = <-p.svrResult
require.EqualError(t, err, io.EOF.Error())
checkRequestServerAllocator(t, p)
}

func TestUncleanDisconnect(t *testing.T) {
p := clientRequestServerPair(t)
defer p.Close()

foo := NewRequest("", "foo")
p.svr.nextRequest(foo)
err := p.cli.conn.Close()
require.NoError(t, err)
// the foo request above is still open after the client disconnects
// so the server will convert io.EOF to io.ErrUnexpectedEOF
err = <-p.svrResult
require.EqualError(t, err, io.ErrUnexpectedEOF.Error())
checkRequestServerAllocator(t, p)
}

func TestCleanPath(t *testing.T) {
assert.Equal(t, "/", cleanPath("/"))
assert.Equal(t, "/", cleanPath("."))
Expand Down