Skip to content

Commit

Permalink
Use an error constant for "nil client" errors (#90)
Browse files Browse the repository at this point in the history
* Define a constant error for nil clients & return that

This should allow tests and other code that relied on some
predictable behavior on nil client usage to still receive that
behavior: Instead of `nil`, we just return ErrNoClient now.

* Add a test case for ErrNoClient behavior

* Add four methods not covered by the test

* Incr, Decr
* Timing, TimeInMilliseconds

* Remove TestNilSafe in favor of TestNilError

By using assertNotPanics in the test case, we can assert that it
doesn't panic just as well, and it doesn't require maintaining the
same list of function calls in two tests.
  • Loading branch information
asf-stripe authored and arbll committed May 29, 2019
1 parent c31b787 commit f6e7675
Show file tree
Hide file tree
Showing 2 changed files with 43 additions and 22 deletions.
22 changes: 16 additions & 6 deletions statsd/statsd.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,16 @@ const (
entityIDTagName = "dd.internal.entity_id"
)

type noClientErr string

// ErrNoClient is returned if statsd reporting methods are invoked on
// a nil client.
const ErrNoClient = noClientErr("statsd client is nil")

func (e noClientErr) Error() string {
return string(e)
}

/*
Stat suffixes
*/
Expand Down Expand Up @@ -220,7 +230,7 @@ func (c *Client) format(name string, value interface{}, suffix []byte, tags []st
// SetWriteTimeout allows the user to set a custom UDS write timeout. Not supported for UDP.
func (c *Client) SetWriteTimeout(d time.Duration) error {
if c == nil {
return fmt.Errorf("Client is nil")
return ErrNoClient
}
return c.writer.SetWriteTimeout(d)
}
Expand Down Expand Up @@ -307,7 +317,7 @@ func copyAndResetBuffer(buf *bytes.Buffer) []byte {
// Flush forces a flush of the pending commands in the buffer
func (c *Client) Flush() error {
if c == nil {
return fmt.Errorf("Client is nil")
return ErrNoClient
}
c.Lock()
defer c.Unlock()
Expand Down Expand Up @@ -361,7 +371,7 @@ func (c *Client) sendMsg(msg []byte) error {
// send handles sampling and sends the message over UDP. It also adds global namespace prefixes and tags.
func (c *Client) send(name string, value interface{}, suffix []byte, tags []string, rate float64) error {
if c == nil {
return fmt.Errorf("Client is nil")
return ErrNoClient
}
if rate < 1 && rand.Float64() > rate {
return nil
Expand Down Expand Up @@ -419,7 +429,7 @@ func (c *Client) TimeInMilliseconds(name string, value float64, tags []string, r
// Event sends the provided Event.
func (c *Client) Event(e *Event) error {
if c == nil {
return fmt.Errorf("Client is nil")
return ErrNoClient
}
stat, err := e.Encode(c.Tags...)
if err != nil {
Expand All @@ -437,7 +447,7 @@ func (c *Client) SimpleEvent(title, text string) error {
// ServiceCheck sends the provided ServiceCheck.
func (c *Client) ServiceCheck(sc *ServiceCheck) error {
if c == nil {
return fmt.Errorf("Client is nil")
return ErrNoClient
}
stat, err := sc.Encode(c.Tags...)
if err != nil {
Expand All @@ -455,7 +465,7 @@ func (c *Client) SimpleServiceCheck(name string, status ServiceCheckStatus) erro
// Close the client connection.
func (c *Client) Close() error {
if c == nil {
return fmt.Errorf("Client is nil")
return ErrNoClient
}
select {
case c.stop <- struct{}{}:
Expand Down
43 changes: 27 additions & 16 deletions statsd/statsd_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -537,23 +537,34 @@ func TestSendMsgUDP(t *testing.T) {
}
}

func TestNilSafe(t *testing.T) {
func TestNilError(t *testing.T) {
var c *Client
assertNotPanics(t, func() { c.SetWriteTimeout(0) })
assertNotPanics(t, func() { c.Flush() })
assertNotPanics(t, func() { c.Close() })
assertNotPanics(t, func() { c.Count("", 0, nil, 1) })
assertNotPanics(t, func() { c.Histogram("", 0, nil, 1) })
assertNotPanics(t, func() { c.Distribution("", 0, nil, 1) })
assertNotPanics(t, func() { c.Gauge("", 0, nil, 1) })
assertNotPanics(t, func() { c.Set("", "", nil, 1) })
assertNotPanics(t, func() {
c.send("", "", []byte(""), nil, 1)
})
assertNotPanics(t, func() { c.Event(NewEvent("", "")) })
assertNotPanics(t, func() { c.SimpleEvent("", "") })
assertNotPanics(t, func() { c.ServiceCheck(NewServiceCheck("", Ok)) })
assertNotPanics(t, func() { c.SimpleServiceCheck("", Ok) })
tests := []func() error{
func() error { return c.SetWriteTimeout(0) },
func() error { return c.Flush() },
func() error { return c.Close() },
func() error { return c.Count("", 0, nil, 1) },
func() error { return c.Incr("", nil, 1) },
func() error { return c.Decr("", nil, 1) },
func() error { return c.Histogram("", 0, nil, 1) },
func() error { return c.Distribution("", 0, nil, 1) },
func() error { return c.Gauge("", 0, nil, 1) },
func() error { return c.Set("", "", nil, 1) },
func() error { return c.Timing("", time.Second, nil, 1) },
func() error { return c.TimeInMilliseconds("", 1, nil, 1) },
func() error { return c.send("", "", []byte(""), nil, 1) },
func() error { return c.Event(NewEvent("", "")) },
func() error { return c.SimpleEvent("", "") },
func() error { return c.ServiceCheck(NewServiceCheck("", Ok)) },
func() error { return c.SimpleServiceCheck("", Ok) },
}
for i, f := range tests {
var err error
assertNotPanics(t, func() { err = f() })
if err != ErrNoClient {
t.Errorf("Test case %d: expected ErrNoClient, got %#v", i, err)
}
}
}

func TestEvents(t *testing.T) {
Expand Down

0 comments on commit f6e7675

Please sign in to comment.