Skip to content

Commit 31ad8cb

Browse files
authored
*: fix QUIC false error (#4585)
* Do not error on close write, if stream was already canceled * Nolint for revive
1 parent 6599a4e commit 31ad8cb

2 files changed

Lines changed: 27 additions & 3 deletions

File tree

p2p/sender.go

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -344,7 +344,17 @@ func SendReceive(ctx context.Context, p2pNode host.Host, peerID peer.ID,
344344
}
345345

346346
if err := s.CloseWrite(); err != nil {
347-
return errors.Wrap(err, "close write", z.Any("protocol", s.Protocol()))
347+
// A canceled-stream error here is benign: the request was already written and
348+
// delivered above, and the peer resetting our send-direction (STOP_SENDING) does
349+
// not affect the receive-direction, so the response is still readable below.
350+
// This happens much more frequently when QUIC is enabled.
351+
//
352+
//nolint:revive // Prioritise readability.
353+
if isCanceledStreamErr(err) {
354+
log.Debug(ctx, "Closing write of canceled stream", z.Err(err), z.Any("protocol", s.Protocol()))
355+
} else {
356+
return errors.Wrap(err, "close write", z.Any("protocol", s.Protocol()))
357+
}
348358
}
349359

350360
if err = reader.ReadMsg(resp); err != nil {
@@ -355,7 +365,7 @@ func SendReceive(ctx context.Context, p2pNode host.Host, peerID peer.ID,
355365
// This close error doesn't require a retry or warning mechanism since the message was
356366
// correctly sent to the destination. However, it is still unknown what/who is causing the
357367
// stream to be canceled. This error appears much more frequently when QUIC is enabled.
358-
if strings.Contains(err.Error(), "close called for canceled stream") {
368+
if isCanceledStreamErr(err) {
359369
log.Debug(ctx, "Closing canceled stream", z.Err(err), z.Any("protocol", s.Protocol()))
360370
return nil
361371
}
@@ -412,7 +422,7 @@ func Send(ctx context.Context, p2pNode host.Host, protoID protocol.ID, peerID pe
412422
// This close error doesn't require a retry or warning mechanism since the message was
413423
// correctly sent to the destination. However, it is still unknown what/who is causing the
414424
// stream to be canceled. This error appears much more frequently when QUIC is enabled.
415-
if strings.Contains(err.Error(), "close called for canceled stream") {
425+
if isCanceledStreamErr(err) {
416426
log.Debug(ctx, "Closing canceled stream", z.Err(err), z.Any("protocol", s.Protocol()))
417427
return nil
418428
}
@@ -423,6 +433,13 @@ func Send(ctx context.Context, p2pNode host.Host, protoID protocol.ID, peerID pe
423433
return nil
424434
}
425435

436+
// isCanceledStreamErr returns true if the error is the benign QUIC stream error
437+
// returned when closing a stream whose send-direction the peer has already reset
438+
// (via STOP_SENDING). It occurs much more frequently when QUIC is enabled.
439+
func isCanceledStreamErr(err error) bool {
440+
return err != nil && strings.Contains(err.Error(), "close called for canceled stream")
441+
}
442+
426443
// protocolPrefix returns the common prefix of the provided protocol IDs.
427444
func protocolPrefix(pIDs ...protocol.ID) protocol.ID {
428445
if len(pIDs) == 0 {

p2p/sender_internal_test.go

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,3 +160,10 @@ func TestNonZeroResponse(t *testing.T) {
160160
err := SendReceive(ctx, nil, "", nil, &pbv1.Duty{Slot: 1}, "")
161161
require.ErrorContains(t, err, "bug: response proto must be zero value")
162162
}
163+
164+
func TestIsCanceledStreamErr(t *testing.T) {
165+
require.False(t, isCanceledStreamErr(nil))
166+
require.False(t, isCanceledStreamErr(errors.New("some other error")))
167+
require.True(t, isCanceledStreamErr(errors.New("close called for canceled stream 4")))
168+
require.True(t, isCanceledStreamErr(errors.Wrap(errors.New("close called for canceled stream 4"), "close write")))
169+
}

0 commit comments

Comments
 (0)