Skip to content

Commit 2c6a657

Browse files
collectors: detect DeadlineExceeded from gRPC status code
IsDeadlineExceeded missed a deadline error whose gRPC code is DeadlineExceeded but whose description is "stream terminated by RST_STREAM with error code: CANCEL" rather than "context deadline exceeded". Neither existing check caught it: status.FromContextError only matches errors that are/wrap context.DeadlineExceeded, and the string fallback looked for the literal "context deadline exceeded" description. As a result the error slipped past the tolerance guard in the collectors, got forwarded to errChan, and killed lndmon on a transient scrape-cycle timeout. Check the actual gRPC status code via status.FromError (which also unwraps wrapped errors through errors.As), so any DeadlineExceeded status is tolerated regardless of its description text. This is shared by every collector that calls IsDeadlineExceeded. Add errors_test.go covering the RST_STREAM variant (plain and wrapped), the original context-deadline shapes, and non-deadline errors. Add a "test" Makefile target that runs "go test -v ./..." so the unit tests can be run with "make test", and it shows up in "make list".
1 parent 4b630c9 commit 2c6a657

3 files changed

Lines changed: 104 additions & 0 deletions

File tree

Makefile

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ GO_BIN := ${GOPATH}/bin
99
LINT_BIN := $(GO_BIN)/golangci-lint
1010

1111
GOBUILD := go build -v
12+
GOTEST := go test -v
1213

1314
GOFILES_NOVENDOR = $(shell find . -type f -name '*.go' -not -path "./vendor/*")
1415
GOLIST := go list -deps $(PKG)/... | grep '$(PKG)'| grep -v '/vendor/'
@@ -47,6 +48,14 @@ build:
4748
@$(call print, "Building lndmon.")
4849
$(GOBUILD) $(PKG)/cmd/lndmon
4950

51+
# =======
52+
# TESTING
53+
# =======
54+
55+
test:
56+
@$(call print, "Running unit tests.")
57+
$(GOTEST) ./...
58+
5059
# =========
5160
# UTILITIES
5261
# =========

collectors/errors.go

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,19 @@ func IsDeadlineExceeded(err error) bool {
2626
return false
2727
}
2828

29+
// If the error is (or wraps) a gRPC status error, check its code
30+
// directly. This catches server-side deadlines that arrive with a
31+
// non-standard description, e.g. "stream terminated by RST_STREAM with
32+
// error code: CANCEL", where the code is DeadlineExceeded but the
33+
// description doesn't match context.DeadlineExceeded.
34+
if st, ok := status.FromError(err); ok &&
35+
st.Code() == codes.DeadlineExceeded {
36+
37+
return true
38+
}
39+
40+
// The error may also be a bare context error (client-side deadline)
41+
// that isn't wrapped in a gRPC status.
2942
st := status.FromContextError(err)
3043
if st.Code() == codes.DeadlineExceeded {
3144
return true

collectors/errors_test.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
package collectors
2+
3+
import (
4+
"context"
5+
"errors"
6+
"fmt"
7+
"testing"
8+
9+
"google.golang.org/grpc/codes"
10+
"google.golang.org/grpc/status"
11+
)
12+
13+
// TestIsDeadlineExceeded verifies that IsDeadlineExceeded recognizes the
14+
// various shapes a deadline error can take, most importantly a server-side
15+
// gRPC status error whose code is DeadlineExceeded but whose description does
16+
// not mention "context deadline exceeded".
17+
func TestIsDeadlineExceeded(t *testing.T) {
18+
tests := []struct {
19+
name string
20+
err error
21+
want bool
22+
}{
23+
{
24+
name: "nil error",
25+
err: nil,
26+
want: false,
27+
},
28+
{
29+
name: "bare context deadline",
30+
err: context.DeadlineExceeded,
31+
want: true,
32+
},
33+
{
34+
name: "grpc status with context deadline desc",
35+
err: status.Error(
36+
codes.DeadlineExceeded,
37+
context.DeadlineExceeded.Error(),
38+
),
39+
want: true,
40+
},
41+
{
42+
name: "grpc RST_STREAM deadline",
43+
err: status.Error(
44+
codes.DeadlineExceeded,
45+
"stream terminated by RST_STREAM with error "+
46+
"code: CANCEL",
47+
),
48+
want: true,
49+
},
50+
{
51+
name: "wrapped grpc RST_STREAM deadline",
52+
err: fmt.Errorf("WalletCollector WalletBalance failed "+
53+
"with: %w", status.Error(
54+
codes.DeadlineExceeded,
55+
"stream terminated by RST_STREAM with error "+
56+
"code: CANCEL",
57+
)),
58+
want: true,
59+
},
60+
{
61+
name: "unrelated grpc error",
62+
err: status.Error(
63+
codes.Unavailable, "connection refused",
64+
),
65+
want: false,
66+
},
67+
{
68+
name: "unrelated plain error",
69+
err: errors.New("something else failed"),
70+
want: false,
71+
},
72+
}
73+
74+
for _, tc := range tests {
75+
t.Run(tc.name, func(t *testing.T) {
76+
if got := IsDeadlineExceeded(tc.err); got != tc.want {
77+
t.Fatalf("IsDeadlineExceeded(%v) = %v, want %v",
78+
tc.err, got, tc.want)
79+
}
80+
})
81+
}
82+
}

0 commit comments

Comments
 (0)