Fix several bugs in minecraft input (#2970)

This commit is contained in:
Adam Perlin 2017-06-27 13:14:07 -07:00 committed by Daniel Nelson
parent 6cd958b215
commit c30124e192
6 changed files with 100 additions and 14 deletions

View File

@ -171,9 +171,17 @@ func (c *Client) Send(typ int32, command string) (response *Packet, err error) {
} }
body := make([]byte, header.Size-int32(PacketHeaderSize)) body := make([]byte, header.Size-int32(PacketHeaderSize))
n, err = c.Connection.Read(body) n, err = c.Connection.Read(body)
for n < len(body) {
var nBytes int
nBytes, err = c.Connection.Read(body[n:])
if err != nil {
return
}
n += nBytes
}
if nil != err { if nil != err {
return return
} else if n != len(body) { } else if n != len(body) {

View File

@ -25,7 +25,7 @@ var (
// Client is an interface for a client which gathers data from a minecraft server // Client is an interface for a client which gathers data from a minecraft server
type Client interface { type Client interface {
Gather() ([]string, error) Gather(producer RCONClientProducer) ([]string, error)
} }
// Minecraft represents a connection to a minecraft server // Minecraft represents a connection to a minecraft server
@ -34,6 +34,7 @@ type Minecraft struct {
Port string Port string
Password string Password string
client Client client Client
clientSet bool
} }
// Description gives a brief description. // Description gives a brief description.
@ -48,16 +49,26 @@ func (s *Minecraft) SampleConfig() string {
// Gather uses the RCON protocal to collect player and // Gather uses the RCON protocal to collect player and
// scoreboard stats from a minecraft server. // scoreboard stats from a minecraft server.
//var hasClient bool = false
func (s *Minecraft) Gather(acc telegraf.Accumulator) error { func (s *Minecraft) Gather(acc telegraf.Accumulator) error {
if s.client == nil { // can't simply compare s.client to nil, because comparing an interface
// to nil often does not produce the desired result
if !s.clientSet {
var err error var err error
s.client, err = NewRCON(s.Server, s.Port, s.Password) s.client, err = NewRCON(s.Server, s.Port, s.Password)
if err != nil { if err != nil {
return err return err
} }
s.clientSet = true
} }
scores, err := s.client.Gather() // (*RCON).Gather() takes an RCONClientProducer for testing purposes
d := defaultClientProducer{
Server: s.Server,
Port: s.Port,
}
scores, err := s.client.Gather(d)
if err != nil { if err != nil {
return err return err
} }

View File

@ -161,7 +161,7 @@ type MockClient struct {
Err error Err error
} }
func (m *MockClient) Gather() ([]string, error) { func (m *MockClient) Gather(d RCONClientProducer) ([]string, error) {
return m.Result, m.Err return m.Result, m.Err
} }
@ -178,13 +178,19 @@ func TestGather(t *testing.T) {
}, },
Err: nil, Err: nil,
}, },
clientSet: true,
} }
err := testConfig.Gather(&acc) err := testConfig.Gather(&acc)
if err != nil { if err != nil {
t.Fatalf("gather returned error. Error: %s\n", err) t.Fatalf("gather returned error. Error: %s\n", err)
} }
if !testConfig.clientSet {
t.Fatalf("clientSet should be true, client should be set")
}
tags := map[string]string{ tags := map[string]string{
"player": "divislight", "player": "divislight",
"server": "biffsgang.net:25575", "server": "biffsgang.net:25575",

View File

@ -29,6 +29,22 @@ type RCON struct {
client RCONClient client RCONClient
} }
// RCONClientProducer is an interface which defines how a new client will be
// produced in the event of a network disconnect. It exists mainly for
// testing purposes
type RCONClientProducer interface {
newClient() (RCONClient, error)
}
type defaultClientProducer struct {
Server string
Port string
}
func (d defaultClientProducer) newClient() (RCONClient, error) {
return newClient(d.Server, d.Port)
}
// NewRCON creates a new RCON // NewRCON creates a new RCON
func NewRCON(server, port, password string) (*RCON, error) { func NewRCON(server, port, password string) (*RCON, error) {
client, err := newClient(server, port) client, err := newClient(server, port)
@ -50,18 +66,26 @@ func newClient(server, port string) (*rcon.Client, error) {
return nil, err return nil, err
} }
return rcon.NewClient(server, p) client, err := rcon.NewClient(server, p)
// If we've encountered any error, the contents of `client` could be corrupted,
// so we must return nil, err
if err != nil {
return nil, err
}
return client, nil
} }
// Gather recieves all player scoreboard information and returns it per player. // Gather recieves all player scoreboard information and returns it per player.
func (r *RCON) Gather() ([]string, error) { func (r *RCON) Gather(producer RCONClientProducer) ([]string, error) {
if r.client == nil { if r.client == nil {
var err error var err error
r.client, err = newClient(r.Server, r.Port) r.client, err = producer.newClient()
if err != nil { if err != nil {
return nil, err return nil, err
} }
} }
if _, err := r.client.Authorize(r.Password); err != nil { if _, err := r.client.Authorize(r.Password); err != nil {
// Potentially a network problem where the client will need to be // Potentially a network problem where the client will need to be
// re-initialized // re-initialized

View File

@ -0,0 +1,36 @@
package minecraft
import (
"errors"
"testing"
)
type MockRCONProducer struct {
Err error
}
func (m *MockRCONProducer) newClient() (RCONClient, error) {
return nil, m.Err
}
func TestRCONErrorHandling(t *testing.T) {
m := &MockRCONProducer{
Err: errors.New("Error: failed connection"),
}
c := &RCON{
Server: "craftstuff.com",
Port: "2222",
Password: "pass",
//Force fetching of new client
client: nil,
}
_, err := c.Gather(m)
if err == nil {
t.Errorf("Error nil, unexpected result")
}
if c.client != nil {
t.Fatal("c.client should be nil, unexpected result")
}
}

View File

@ -40,7 +40,8 @@ func TestRCONGather(t *testing.T) {
client: mock, client: mock,
} }
got, err := client.Gather() d := defaultClientProducer{}
got, err := client.Gather(d)
if err != nil { if err != nil {
t.Fatalf("Gather returned an error. Error %s\n", err) t.Fatalf("Gather returned an error. Error %s\n", err)
} }
@ -57,7 +58,7 @@ func TestRCONGather(t *testing.T) {
Err: nil, Err: nil,
} }
got, err = client.Gather() got, err = client.Gather(defaultClientProducer{})
if err != nil { if err != nil {
t.Fatalf("Gather returned an error. Error %s\n", err) t.Fatalf("Gather returned an error. Error %s\n", err)
} }