Skip to content

Commit

Permalink
change Err field in PeerStatus to string
Browse files Browse the repository at this point in the history
The main use for this status is to send update messages through the WS,
in the form of JSON. Marshalling a JSON field into an error object
doesn't work in Go, so it's better to send the error message as string.
See golang/go#5161
  • Loading branch information
marcovidonis committed Jan 15, 2024
1 parent e9a8f81 commit 5c138a6
Show file tree
Hide file tree
Showing 4 changed files with 20 additions and 10 deletions.
11 changes: 5 additions & 6 deletions client-peerconn_test.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package torrent

import (
"errors"
"io"
"os"
"testing"
Expand Down Expand Up @@ -37,7 +36,7 @@ func TestPeerConnObserverReadStatusOk(t *testing.T) {

status := readChannelTimeout(t, cfg.Observers.Peers.PeerStatus, 500*time.Millisecond).(PeerStatus)
require.True(t, status.Ok)
require.Nil(t, status.Err)
require.Equal(t, "", status.Err)
}

func TestPeerConnObserverReadStatusErr(t *testing.T) {
Expand All @@ -55,13 +54,13 @@ func TestPeerConnObserverReadStatusErr(t *testing.T) {

go func() {
cfg.Observers.Peers.PeerStatus <- PeerStatus{
Err: errors.New("test error"),
Err: "test error",
}
}()

status := readChannelTimeout(t, cfg.Observers.Peers.PeerStatus, 500*time.Millisecond).(PeerStatus)
require.False(t, status.Ok)
require.EqualError(t, status.Err, "test error")
require.Equal(t, status.Err, "test error")
}

func TestPeerConnEstablished(t *testing.T) {
Expand Down Expand Up @@ -92,13 +91,13 @@ func TestPeerConnEstablished(t *testing.T) {
// FIXME converting [20]byte to string is not enough to pass the test
// require.Equal(t, "12345123451234512345", fmt.Sprintf("%+q", status.Id))
require.True(t, status.Ok)
require.Nil(t, status.Err)
require.Equal(t, "", status.Err)

// Peer conn is dropped after transfer is finished. This is the next update we receive.
status = readChannelTimeout(t, obs.Peers.PeerStatus, 500*time.Millisecond).(PeerStatus)
// TODO a check on PeerID
require.False(t, status.Ok)
require.Nil(t, status.Err)
require.Equal(t, "", status.Err)
}

type ConfigureClient struct {
Expand Down
12 changes: 10 additions & 2 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -1095,11 +1095,19 @@ func (cl *Client) runHandshookConn(c *PeerConn, t *Torrent) error {

// TODO here we could send an update to say the PeerConn state connected is true
// perhaps could also use c.pex.IsEnabled() or something similar
c.UpdatePeerConnStatus(PeerStatus{c.PeerID, true, nil})
c.UpdatePeerConnStatus(PeerStatus{
Id: c.PeerID,
Ok: true,
})

err := c.mainReadLoop()
if err != nil {
c.UpdatePeerConnStatus(PeerStatus{c.PeerID, false, err})
// c.UpdatePeerConnStatus(PeerStatus{c.PeerID, false, err})
c.UpdatePeerConnStatus(PeerStatus{
Id: c.PeerID,
Ok: false,
Err: fmt.Sprintf("%s", err),
})
return fmt.Errorf("main read loop: %w", err)
}
return nil
Expand Down
2 changes: 1 addition & 1 deletion peerconn.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import (
type PeerStatus struct {
Id PeerID
Ok bool
Err error
Err string // see https://github.com/golang/go/issues/5161
}

type PeerObserver struct {
Expand Down
5 changes: 4 additions & 1 deletion torrent.go
Original file line number Diff line number Diff line change
Expand Up @@ -1596,7 +1596,10 @@ func (t *Torrent) dropConnection(c *PeerConn) {
t.cl.event.Broadcast()
c.close()

c.UpdatePeerConnStatus(PeerStatus{c.PeerID, false, nil})
c.UpdatePeerConnStatus(PeerStatus{
Id: c.PeerID,
Ok: false,
})
t.logger.WithDefaultLevel(log.Debug).Printf("dropping connection to %+q, sent peerconn update", c.PeerID)

if t.deletePeerConn(c) {
Expand Down

0 comments on commit 5c138a6

Please sign in to comment.