-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Implement error codes spec #2927
base: master
Are you sure you want to change the base?
Changes from all commits
aa8b049
58f5b89
e5acd28
1ad628f
0e1eb18
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -27,4 +27,8 @@ type Stream interface { | |
|
||
// Scope returns the user's view of this stream's resource scope | ||
Scope() StreamScope | ||
|
||
// ResetWithError closes both ends of the stream with errCode. The errCode is sent | ||
// to the peer. | ||
ResetWithError(errCode StreamErrorCode) error | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,8 @@ | ||
package yamux | ||
|
||
import ( | ||
"errors" | ||
"fmt" | ||
"time" | ||
|
||
"github.com/libp2p/go-libp2p/core/network" | ||
|
@@ -13,22 +15,33 @@ type stream yamux.Stream | |
|
||
var _ network.MuxedStream = &stream{} | ||
|
||
func (s *stream) Read(b []byte) (n int, err error) { | ||
n, err = s.yamux().Read(b) | ||
if err == yamux.ErrStreamReset { | ||
err = network.ErrReset | ||
func parseResetError(err error) error { | ||
if err == nil { | ||
return err | ||
} | ||
se := &yamux.StreamError{} | ||
if errors.As(err, &se) { | ||
return &network.StreamError{Remote: se.Remote, ErrorCode: network.StreamErrorCode(se.ErrorCode)} | ||
} | ||
ce := &yamux.GoAwayError{} | ||
if errors.As(err, &ce) { | ||
return &network.ConnError{Remote: ce.Remote, ErrorCode: network.ConnErrorCode(ce.ErrorCode)} | ||
} | ||
// TODO: How should we handle resets for reason other than a remote error | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand this TODO, can you elaborate? |
||
if errors.Is(err, yamux.ErrStreamReset) { | ||
return fmt.Errorf("%w: %w", network.ErrReset, err) | ||
} | ||
return err | ||
} | ||
|
||
return n, err | ||
func (s *stream) Read(b []byte) (n int, err error) { | ||
n, err = s.yamux().Read(b) | ||
return n, parseResetError(err) | ||
} | ||
|
||
func (s *stream) Write(b []byte) (n int, err error) { | ||
n, err = s.yamux().Write(b) | ||
if err == yamux.ErrStreamReset { | ||
err = network.ErrReset | ||
} | ||
|
||
return n, err | ||
return n, parseResetError(err) | ||
} | ||
|
||
func (s *stream) Close() error { | ||
|
@@ -39,6 +52,10 @@ func (s *stream) Reset() error { | |
return s.yamux().Reset() | ||
} | ||
|
||
func (s *stream) ResetWithError(errCode network.StreamErrorCode) error { | ||
return s.yamux().ResetWithError(uint32(errCode)) | ||
} | ||
|
||
func (s *stream) CloseRead() error { | ||
return s.yamux().CloseRead() | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,6 +33,14 @@ func (c *tconn) Close() error { | |
return nil | ||
} | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we add a test here that the remote gets the expected error back? |
||
func (c *tconn) CloseWithError(code network.ConnErrorCode) error { | ||
atomic.StoreUint32(&c.closed, 1) | ||
if c.disconnectNotify != nil { | ||
c.disconnectNotify(nil, c) | ||
} | ||
return nil | ||
} | ||
|
||
func (c *tconn) isClosed() bool { | ||
return atomic.LoadUint32(&c.closed) == 1 | ||
} | ||
|
@@ -794,6 +802,7 @@ type mockConn struct { | |
} | ||
|
||
func (m mockConn) Close() error { panic("implement me") } | ||
func (m mockConn) CloseWithError(errCode network.ConnErrorCode) error { panic("implement me") } | ||
func (m mockConn) LocalPeer() peer.ID { panic("implement me") } | ||
func (m mockConn) RemotePeer() peer.ID { panic("implement me") } | ||
func (m mockConn) RemotePublicKey() crypto.PubKey { panic("implement me") } | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not add this to the MuxedStream Interface? I'm worried that this approach will have the following issues: