Implement error codes spec #2927

sukunrt · 2024-08-19T14:42:45Z

Specification: libp2p/specs#479

Update mplex to be interface compliant before merging

MarcoPolo

Looks good. Just a couple of small comments.

Do we have a PR that updates the WebRTC spec changes?

core/network/mux.go

MarcoPolo · 2025-01-07T18:03:45Z

core/network/stream.go

+
+	// ResetWithError closes both ends of the stream with errCode. The errCode is sent
+	// to the peer.
+	ResetWithError(errCode StreamErrorCode) error


What about CloseWithError as well?

Do we need that? Close implies non error reliable close, no? For instance, we also flush all writes, and retransmit if necessary, when calling Close.

p2p/muxer/yamux/stream.go

p2p/net/connmgr/connmgr_test.go

MarcoPolo · 2025-01-07T18:26:39Z

p2p/net/mock/mock_stream.go

@@ -144,6 +144,22 @@ func (s *stream) Reset() error {
 	return nil
 }

+func (s *stream) ResetWithError(errCode network.StreamErrorCode) error {
+	// Cancel any pending reads/writes with an error.
+	// TODO: Should these be the other way round(remote=true)?


Yes. The errors will be returned to the other side of the pipe. In this case the peer will see these errors, so we should set remote=true.

I'm removing support for error codes here. It's a much larger change. We need to propogate resets from read as well. I'll add it in a follow up PR.

p2p/test/transport/transport_test.go

MarcoPolo · 2025-01-07T21:52:16Z

p2p/transport/quic/stream.go

+	if errors.As(err, &se) {
+		code := se.ErrorCode
+		if code > math.MaxUint32 {
+			// TODO(sukunrt): do we need this?


This should probably set the code to a sentinel value that signifies there was an error converting the transport code to a libp2p code. We also need to update the specs to mention this case (in case we haven't).

Added a sentinel code. libp2p/specs@4eca305

p2p/transport/webrtc/stream_read.go

sukunrt · 2025-01-27T15:39:06Z

Should we add CloseReadWithError for streams? Only quic and webrtc support it.

I can introduce that in a follow up PR.

MarcoPolo · 2025-01-27T21:45:58Z

p2p/test/transport/transport_test.go

+				})
+			})
+
+			t.Run("StreamResetByConnCloseWithError", func(t *testing.T) {


This should likely be called "NewStreamsErrByConnCloseWithError"

Split this into two

Stream methods should get the connection close error code if the connection was closed.

Conn.NewStream should get the connection close error code.

p2p/test/transport/transport_test.go

MarcoPolo

A couple minor things to fix, but looks good!

core/network/mux.go

p2p/net/swarm/swarm.go

MarcoPolo · 2025-01-28T19:30:58Z

p2p/transport/webtransport/stream.go

@@ -56,6 +56,12 @@ func (s *stream) Reset() error {
 	return nil
 }

+func (s *stream) ResetWithError(_ network.StreamErrorCode) error {


Please leave a comment with some context for why we can't do this yet.

added some text.

The type ErrNotSupported introduced in multiformats/go-multistream#114 handles the lazy handshake fail correctly.

sukunrt force-pushed the sukun/error-codes branch 3 times, most recently from a6e85e0 to b7d79b0 Compare August 20, 2024 17:12

sukunrt mentioned this pull request Aug 27, 2024

add support for sending error codes on session close libp2p/go-yamux#121

Merged

sukunrt force-pushed the sukun/error-codes branch 2 times, most recently from 66c8cdb to 53d436f Compare September 13, 2024 13:13

MarcoPolo mentioned this pull request Oct 28, 2024

v0.38 #3005

Closed

26 tasks

sukunrt self-assigned this Oct 29, 2024

sukunrt force-pushed the sukun/error-codes branch 5 times, most recently from 3d1038b to aa8b049 Compare November 20, 2024 10:05

sukunrt mentioned this pull request Nov 20, 2024

Error Codes Tracking Issue #3053

Open

10 tasks

sukunrt force-pushed the sukun/error-codes branch from d6baf8e to 1ad628f Compare November 21, 2024 08:33

sukunrt marked this pull request as ready for review November 21, 2024 13:16

sukunrt requested a review from MarcoPolo November 21, 2024 13:16

MarcoPolo mentioned this pull request Dec 5, 2024

v0.39 #3082

Open

20 tasks

MarcoPolo reviewed Jan 7, 2025

View reviewed changes

sukunrt force-pushed the sukun/error-codes branch 3 times, most recently from 354fbdc to 798258c Compare January 27, 2025 15:22

sukunrt requested a review from MarcoPolo January 27, 2025 15:37

MarcoPolo reviewed Jan 27, 2025

View reviewed changes

p2p/test/transport/transport_test.go Show resolved Hide resolved

p2p/test/transport/transport_test.go Outdated Show resolved Hide resolved

MarcoPolo mentioned this pull request Jan 28, 2025

Return wrapped network.ErrReset error type with more info #3156

Open

MarcoPolo self-requested a review January 28, 2025 18:57

MarcoPolo approved these changes Jan 28, 2025

View reviewed changes

sukunrt force-pushed the sukun/error-codes branch from 6a39dda to be45dcb Compare January 29, 2025 16:48

sukunrt force-pushed the sukun/error-codes branch from be45dcb to 146812c Compare January 29, 2025 16:54

sukunrt requested a review from MarcoPolo January 29, 2025 16:54

sukunrt force-pushed the sukun/error-codes branch from 146812c to dfa7e28 Compare January 29, 2025 17:09

MarcoPolo approved these changes Jan 29, 2025

View reviewed changes

MarcoPolo mentioned this pull request Jan 29, 2025

v0.40 #3164

Closed

20 tasks

sukunrt force-pushed the sukun/error-codes branch from 4df0ee3 to 1c3909d Compare January 31, 2025 14:21

sukunrt added 12 commits February 9, 2025 21:07

error-codes: add implementation for quic, yamux, websockets, webrtc

093396f

add support for multistream

4975a0f

add support for conn manager

f6a2290

send error codes on transport conn and stream failures

b04f87e

close rcmgr scope for transportConn

687d2fd

address review comments

0dbb2ae

remove multistream test

766c1d3

The type ErrNotSupported introduced in multiformats/go-multistream#114 handles the lazy handshake fail correctly.

address review comments 2

fa004ea

make CloseWithError required for transport conns

b903adf

propagate transport error for yamux

c935688

use ranges based on binary rep

598092f

better range for transport errors

c5e1ac9

sukunrt force-pushed the sukun/error-codes branch from f721398 to cb1cfc1 Compare February 9, 2025 15:38

update yamux to v5

a4d9781

sukunrt force-pushed the sukun/error-codes branch from cb1cfc1 to a4d9781 Compare February 9, 2025 15:40

sukunrt merged commit 02ab795 into master Feb 9, 2025
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement error codes spec #2927

Implement error codes spec #2927

sukunrt commented Aug 19, 2024 •

edited

Loading

MarcoPolo left a comment

MarcoPolo Jan 7, 2025

sukunrt Jan 25, 2025

MarcoPolo Jan 7, 2025

sukunrt Jan 27, 2025

MarcoPolo Jan 7, 2025

sukunrt Jan 29, 2025

sukunrt commented Jan 27, 2025

MarcoPolo Jan 27, 2025

sukunrt Jan 29, 2025

MarcoPolo left a comment

MarcoPolo Jan 28, 2025

sukunrt Jan 29, 2025

Implement error codes spec #2927

Implement error codes spec #2927

Conversation

sukunrt commented Aug 19, 2024 • edited Loading

MarcoPolo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sukunrt commented Jan 27, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MarcoPolo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sukunrt commented Aug 19, 2024 •

edited

Loading