Skip to content

Commit d3d5351

Browse files
committed
ping: return a stream of results
Otherwise, we can't return errors. This is a breaking change but unlikely to have a large impact on anyone but go-ipfs. Part of ipfs/kubo#6298
1 parent 643b9ac commit d3d5351

File tree

2 files changed

+42
-17
lines changed

2 files changed

+42
-17
lines changed

p2p/protocol/ping/ping.go

+36-11
Original file line numberDiff line numberDiff line change
@@ -71,42 +71,67 @@ func (p *PingService) PingHandler(s inet.Stream) {
7171
}
7272
}
7373

74-
func (ps *PingService) Ping(ctx context.Context, p peer.ID) (<-chan time.Duration, error) {
74+
// Result is a result of a ping attempt, either an RTT or an error.
75+
type Result struct {
76+
RTT time.Duration
77+
Error error
78+
}
79+
80+
func (ps *PingService) Ping(ctx context.Context, p peer.ID) <-chan Result {
7581
return Ping(ctx, ps.Host, p)
7682
}
7783

78-
func Ping(ctx context.Context, h host.Host, p peer.ID) (<-chan time.Duration, error) {
84+
// Ping pings the remote peer until the context is canceled, returning a stream
85+
// of RTTs or errors.
86+
func Ping(ctx context.Context, h host.Host, p peer.ID) <-chan Result {
7987
s, err := h.NewStream(ctx, p, ID)
8088
if err != nil {
81-
return nil, err
89+
ch := make(chan Result, 1)
90+
ch <- Result{Error: err}
91+
close(ch)
92+
return ch
8293
}
8394

84-
out := make(chan time.Duration)
95+
ctx, cancel := context.WithCancel(ctx)
96+
97+
out := make(chan Result)
8598
go func() {
8699
defer close(out)
87-
defer s.Reset()
100+
defer cancel()
101+
88102
for {
89103
select {
90104
case <-ctx.Done():
91105
return
92106
default:
93-
t, err := ping(s)
94-
if err != nil {
95-
log.Debugf("ping error: %s", err)
107+
var res Result
108+
res.RTT, res.Error = ping(s)
109+
110+
// canceled, ignore everything.
111+
if ctx.Err() != nil {
96112
return
97113
}
98114

99-
h.Peerstore().RecordLatency(p, t)
115+
// No error, record the RTT.
116+
if res.Error == nil {
117+
h.Peerstore().RecordLatency(p, res.RTT)
118+
}
119+
100120
select {
101-
case out <- t:
121+
case out <- res:
102122
case <-ctx.Done():
103123
return
104124
}
105125
}
106126
}
107127
}()
128+
go func() {
129+
// forces the ping to abort.
130+
<-ctx.Done()
131+
s.Reset()
132+
}()
108133

109-
return out, nil
134+
return out
110135
}
111136

112137
func ping(s inet.Stream) (time.Duration, error) {

p2p/protocol/ping/ping_test.go

+6-6
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,15 @@ func TestPing(t *testing.T) {
3737
func testPing(t *testing.T, ps *ping.PingService, p peer.ID) {
3838
pctx, cancel := context.WithCancel(context.Background())
3939
defer cancel()
40-
ts, err := ps.Ping(pctx, p)
41-
if err != nil {
42-
t.Fatal(err)
43-
}
40+
ts := ps.Ping(pctx, p)
4441

4542
for i := 0; i < 5; i++ {
4643
select {
47-
case took := <-ts:
48-
t.Log("ping took: ", took)
44+
case res := <-ts:
45+
if res.Error != nil {
46+
t.Fatal(res.Error)
47+
}
48+
t.Log("ping took: ", res.RTT)
4949
case <-time.After(time.Second * 4):
5050
t.Fatal("failed to receive ping")
5151
}

0 commit comments

Comments
 (0)