Skip to content
This repository has been archived by the owner on Aug 30, 2023. It is now read-only.

Commit

Permalink
Capture and CaptureError not to panic when packet or err is nil
Browse files Browse the repository at this point in the history
Capture methods can handle situations when client is nil, but when
packet or err args are nil it will cause calling thread to panic.
This is highly undesirable in PROD systems that accidentally try to log
a nil packet or error as it might cause a system crash on something
not produciton critical as error logging.
  • Loading branch information
mikhailkolesnik authored and mattrobenolt committed Dec 6, 2017
1 parent c9de0b9 commit 32a1379
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 0 deletions.
9 changes: 9 additions & 0 deletions client.go
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,11 @@ func (client *Client) Capture(packet *Packet, captureTags map[string]string) (ev
return
}

if packet == nil {
close(ch)
return
}

if client.shouldExcludeErr(packet.Message) {
return
}
Expand Down Expand Up @@ -661,6 +666,10 @@ func (client *Client) CaptureError(err error, tags map[string]string, interfaces
return ""
}

if err == nil {
return ""
}

if client.shouldExcludeErr(err.Error()) {
return ""
}
Expand Down
21 changes: 21 additions & 0 deletions client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -255,3 +255,24 @@ func TestNilClient(t *testing.T) {
t.Error("expected nil err:", err)
}
}

func TestCaptureNil(t *testing.T) {
var client *Client = DefaultClient
eventID, ch := client.Capture(nil, nil)
if eventID != "" {
t.Error("expected empty eventID:", eventID)
}
// wait on ch: no send should succeed immediately
err := <-ch
if err != nil {
t.Error("expected nil err:", err)
}
}

func TestCaptureNilError(t *testing.T) {
var client *Client = DefaultClient
eventID := client.CaptureError(nil, nil)
if eventID != "" {
t.Error("expected empty eventID:", eventID)
}
}

0 comments on commit 32a1379

Please sign in to comment.