resolver: manual resolver crashes if grpc.Dial isn't called before so…

…me methods (#6754)
grpc · Nov 7, 2023 · 3fe1123 · 3fe1123
1 parent cf9ae52
commit 3fe1123
Show file tree

Hide file tree

Showing 2 changed files with 84 additions and 4 deletions.
diff --git a/resolver/manual/manual.go b/resolver/manual/manual.go
@@ -78,12 +78,12 @@ func (r *Resolver) InitialState(s resolver.State) {
 func (r *Resolver) Build(target resolver.Target, cc resolver.ClientConn, opts resolver.BuildOptions) (resolver.Resolver, error) {
 	r.BuildCallback(target, cc, opts)
 	r.mu.Lock()
+	defer r.mu.Unlock()
 	r.CC = cc
 	if r.lastSeenState != nil {
 		err := r.CC.UpdateState(*r.lastSeenState)
 		go r.UpdateStateCallback(err)
 	}
-	r.mu.Unlock()
 	return r, nil
 }
 
@@ -105,15 +105,22 @@ func (r *Resolver) Close() {
 // UpdateState calls CC.UpdateState.
 func (r *Resolver) UpdateState(s resolver.State) {
 	r.mu.Lock()
-	err := r.CC.UpdateState(s)
+	defer r.mu.Unlock()
+	var err error
+	if r.CC == nil {
+		panic("cannot update state as grpc.Dial with resolver has not been called")
+	}
+	err = r.CC.UpdateState(s)
 	r.lastSeenState = &s
-	r.mu.Unlock()
 	r.UpdateStateCallback(err)
 }
 
 // ReportError calls CC.ReportError.
 func (r *Resolver) ReportError(err error) {
 	r.mu.Lock()
+	defer r.mu.Unlock()
+	if r.CC == nil {
+		panic("cannot report error as grpc.Dial with resolver has not been called")
+	}
 	r.CC.ReportError(err)
-	r.mu.Unlock()
 }
diff --git a/resolver/manual/manual_test.go b/resolver/manual/manual_test.go
@@ -0,0 +1,73 @@
+/*
+ *
+ * Copyright 2023 gRPC authors.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ *
+ */
+
+package manual_test
+
+import (
+	"errors"
+	"testing"
+
+	"google.golang.org/grpc"
+	"google.golang.org/grpc/credentials/insecure"
+	"google.golang.org/grpc/resolver"
+	"google.golang.org/grpc/resolver/manual"
+)
+
+func TestResolver(t *testing.T) {
+	r := manual.NewBuilderWithScheme("whatever")
+	r.InitialState(resolver.State{
+		Addresses: []resolver.Address{
+			{Addr: "address"},
+		},
+	})
+
+	t.Run("update_state_panics", func(t *testing.T) {
+		defer func() {
+			want := "cannot update state as grpc.Dial with resolver has not been called"
+			if r := recover(); r != want {
+				t.Errorf("expected panic %q, got %q", want, r)
+			}
+		}()
+		r.UpdateState(resolver.State{Addresses: []resolver.Address{
+			{Addr: "address"},
+			{Addr: "anotheraddress"},
+		}})
+	})
+	t.Run("report_error_panics", func(t *testing.T) {
+		defer func() {
+			want := "cannot report error as grpc.Dial with resolver has not been called"
+			if r := recover(); r != want {
+				t.Errorf("expected panic %q, got %q", want, r)
+			}
+		}()
+		r.ReportError(errors.New("example"))
+	})
+
+	t.Run("happy_path", func(t *testing.T) {
+		_, err := grpc.Dial("whatever://localhost",
+			grpc.WithTransportCredentials(insecure.NewCredentials()),
+			grpc.WithResolvers(r))
+		if err != nil {
+			t.Errorf("dial setup error: %v", err)
+		}
+		r.UpdateState(resolver.State{Addresses: []resolver.Address{
+			{Addr: "ok"},
+		}})
+		r.ReportError(errors.New("example"))
+	})
+}