Skip to content

Commit

Permalink
Handle reads from closed sockets and pipes better.
Browse files Browse the repository at this point in the history
After closing a pipe or socket, a read will return an internal error whose text
contains "use of closed network connection". Unfortunately, that appears to be
the only way to detect that error, and golang/go#4373
suggests it's not going to improve on its own.

Do what ~everyone else seems to have done in this case, check for the string.
That gives us nicer diagnostics when a client or server shuts down in the
normal course of its connections being terminated.
  • Loading branch information
creachadair committed Jun 19, 2018
1 parent d8632ec commit 7eb9cc7
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 3 deletions.
9 changes: 7 additions & 2 deletions client.go
Expand Up @@ -5,6 +5,7 @@ import (
"encoding/json"
"errors"
"fmt"
"io"
"strconv"
"sync"

Expand Down Expand Up @@ -68,8 +69,12 @@ func (c *Client) accept(ch channel.Receiver) error {
c.log("Recoverable decoding error: %v", err)
return nil
} else if err != nil {
c.log("Unrecoverable decoding error: %v", err)
c.stop(err)
if err == io.EOF || isErrClosing(err) {
c.stop(nil) // don't remark on this as a failure
} else {
c.log("Unrecoverable decoding error: %v", err)
c.stop(err)
}
return err
}

Expand Down
9 changes: 9 additions & 0 deletions error.go
Expand Up @@ -4,6 +4,7 @@ import (
"encoding/json"
"errors"
"fmt"
"strings"

"bitbucket.org/creachadair/jrpc2/code"
)
Expand Down Expand Up @@ -70,3 +71,11 @@ func DataErrorf(code code.Code, v interface{}, msg string, args ...interface{})
}
return e
}

// isErrClosing detects the internal error returned by a read from a pipe or
// socket that is closed.
func isErrClosing(err error) bool {
// That we must check the string here appears to be working as intended, or at least
// there is no intent to make it better. https://github.com/golang/go/issues/4373
return err != nil && strings.Contains(err.Error(), "use of closed network connection")
}
7 changes: 6 additions & 1 deletion server.go
Expand Up @@ -416,7 +416,12 @@ func (s *Server) read(ch channel.Receiver) {
} else if isRecoverableJSONError(err) {
s.pushError(nil, jerrorf(code.ParseError, "invalid JSON request message"))
} else {
s.stop(err)
// Don't remark on EOF or a closed pipe as a failure.
if isErrClosing(err) {
s.stop(io.EOF)
} else {
s.stop(err)
}
s.mu.Unlock()
return
}
Expand Down

0 comments on commit 7eb9cc7

Please sign in to comment.