Merge pull request #134 from ZZiigguurraatt/fix/deadline-exceeded-rst-stream

collectors: detect DeadlineExceeded from gRPC status code

Author: djkazic

Makefile

@@ -9,6 +9,7 @@ GO_BIN := ${GOPATH}/bin
 LINT_BIN := $(GO_BIN)/golangci-lint
 
 GOBUILD := go build -v
+GOTEST := go test -v
 
 GOFILES_NOVENDOR = $(shell find . -type f -name '*.go' -not -path "./vendor/*")
 GOLIST := go list -deps $(PKG)/... | grep '$(PKG)'| grep -v '/vendor/'
@@ -47,6 +48,14 @@ build:
 	@$(call print, "Building lndmon.")
 	$(GOBUILD) $(PKG)/cmd/lndmon
 
+# =======
+# TESTING
+# =======
+
+test:
+	@$(call print, "Running unit tests.")
+	$(GOTEST) ./...
+
 # =========
 # UTILITIES
 # =========

collectors/errors.go

@@ -26,6 +26,19 @@ func IsDeadlineExceeded(err error) bool {
 		return false
 	}
 
+	// If the error is (or wraps) a gRPC status error, check its code
+	// directly. This catches server-side deadlines that arrive with a
+	// non-standard description, e.g. "stream terminated by RST_STREAM with
+	// error code: CANCEL", where the code is DeadlineExceeded but the
+	// description doesn't match context.DeadlineExceeded.
+	if st, ok := status.FromError(err); ok &&
+		st.Code() == codes.DeadlineExceeded {
+
+		return true
+	}
+
+	// The error may also be a bare context error (client-side deadline)
+	// that isn't wrapped in a gRPC status.
 	st := status.FromContextError(err)
 	if st.Code() == codes.DeadlineExceeded {
 		return true

collectors/errors_test.go

@@ -0,0 +1,82 @@
+package collectors
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"testing"
+
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+)
+
+// TestIsDeadlineExceeded verifies that IsDeadlineExceeded recognizes the
+// various shapes a deadline error can take, most importantly a server-side
+// gRPC status error whose code is DeadlineExceeded but whose description does
+// not mention "context deadline exceeded".
+func TestIsDeadlineExceeded(t *testing.T) {
+	tests := []struct {
+		name string
+		err  error
+		want bool
+	}{
+		{
+			name: "nil error",
+			err:  nil,
+			want: false,
+		},
+		{
+			name: "bare context deadline",
+			err:  context.DeadlineExceeded,
+			want: true,
+		},
+		{
+			name: "grpc status with context deadline desc",
+			err: status.Error(
+				codes.DeadlineExceeded,
+				context.DeadlineExceeded.Error(),
+			),
+			want: true,
+		},
+		{
+			name: "grpc RST_STREAM deadline",
+			err: status.Error(
+				codes.DeadlineExceeded,
+				"stream terminated by RST_STREAM with error "+
+					"code: CANCEL",
+			),
+			want: true,
+		},
+		{
+			name: "wrapped grpc RST_STREAM deadline",
+			err: fmt.Errorf("WalletCollector WalletBalance failed "+
+				"with: %w", status.Error(
+				codes.DeadlineExceeded,
+				"stream terminated by RST_STREAM with error "+
+					"code: CANCEL",
+			)),
+			want: true,
+		},
+		{
+			name: "unrelated grpc error",
+			err: status.Error(
+				codes.Unavailable, "connection refused",
+			),
+			want: false,
+		},
+		{
+			name: "unrelated plain error",
+			err:  errors.New("something else failed"),
+			want: false,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := IsDeadlineExceeded(tc.err); got != tc.want {
+				t.Fatalf("IsDeadlineExceeded(%v) = %v, want %v",
+					tc.err, got, tc.want)
+			}
+		})
+	}
+}