review comments

clientconn.go

@@ -31,6 +31,7 @@ import (
 	"time"
 
 	"google.golang.org/grpc/balancer"
+	"google.golang.org/grpc/balancer/base"
 	_ "google.golang.org/grpc/balancer/roundrobin" // To register roundrobin.
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/connectivity"
@@ -588,10 +589,13 @@ func (cc *ClientConn) updateResolverState(s resolver.State, err error) error {
 		if sc, ok := s.ServiceConfig.Config.(*ServiceConfig); s.ServiceConfig.Err == nil && ok {
 			cc.applyServiceConfigAndBalancer(sc, s.Addresses)
 		} else {
+			ret = balancer.ErrBadResolverState
 			if cc.balancerWrapper == nil {
-				cc.applyServiceConfigAndBalancer(emptyServiceConfig, s.Addresses)
+				cc.blockingpicker.updatePicker(base.NewErrPicker(status.Errorf(codes.Unavailable, "error parsing service config: %v", s.ServiceConfig.Err)))
+				cc.csMgr.updateState(connectivity.TransientFailure)
+				cc.mu.Unlock()
+				return ret
 			}
-			ret = balancer.ErrBadResolverState
 		}
 	}
 

service_config.go

@@ -265,13 +265,13 @@ func init() {
 }
 func parseServiceConfig(js string) *serviceconfig.ParseResult {
 	if len(js) == 0 {
-		return serviceconfig.NewParseResult(fmt.Errorf("no JSON service config provided"))
+		return &serviceconfig.ParseResult{Err: fmt.Errorf("no JSON service config provided")}
 	}
 	var rsc jsonSC
 	err := json.Unmarshal([]byte(js), &rsc)
 	if err != nil {
 		grpclog.Warningf("grpc: parseServiceConfig error unmarshaling %s due to %v", js, err)
-		return serviceconfig.NewParseResult(err)
+		return &serviceconfig.ParseResult{Err: err}
 	}
 	sc := ServiceConfig{
 		LB:                rsc.LoadBalancingPolicy,
@@ -285,7 +285,7 @@ func parseServiceConfig(js string) *serviceconfig.ParseResult {
 			if len(lbcfg) != 1 {
 				err := fmt.Errorf("invalid loadBalancingConfig: entry %v does not contain exactly 1 policy/config pair: %q", i, lbcfg)
 				grpclog.Warningf(err.Error())
-				return serviceconfig.NewParseResult(err)
+				return &serviceconfig.ParseResult{Err: err}
 			}
 			var name string
 			var jsonCfg json.RawMessage
@@ -300,7 +300,7 @@ func parseServiceConfig(js string) *serviceconfig.ParseResult {
 				var err error
 				sc.lbConfig.cfg, err = parser.ParseConfig(jsonCfg)
 				if err != nil {
-					return serviceconfig.NewParseResult(fmt.Errorf("error parsing loadBalancingConfig for policy %q: %v", name, err))
+					return &serviceconfig.ParseResult{Err: fmt.Errorf("error parsing loadBalancingConfig for policy %q: %v", name, err)}
 				}
 			} else if string(jsonCfg) != "{}" {
 				grpclog.Warningf("non-empty balancer configuration %q, but balancer does not implement ParseConfig", string(jsonCfg))
@@ -313,12 +313,12 @@ func parseServiceConfig(js string) *serviceconfig.ParseResult {
 			// case.
 			err := fmt.Errorf("invalid loadBalancingConfig: no supported policies found")
 			grpclog.Warningf(err.Error())
-			return serviceconfig.NewParseResult(err)
+			return &serviceconfig.ParseResult{Err: err}
 		}
 	}
 
 	if rsc.MethodConfig == nil {
-		return serviceconfig.NewParseResult(&sc)
+		return &serviceconfig.ParseResult{Config: &sc}
 	}
 	for _, m := range *rsc.MethodConfig {
 		if m.Name == nil {
@@ -327,7 +327,7 @@ func parseServiceConfig(js string) *serviceconfig.ParseResult {
 		d, err := parseDuration(m.Timeout)
 		if err != nil {
 			grpclog.Warningf("grpc: parseServiceConfig error unmarshaling %s due to %v", js, err)
-			return serviceconfig.NewParseResult(err)
+			return &serviceconfig.ParseResult{Err: err}
 		}
 
 		mc := MethodConfig{
@@ -336,7 +336,7 @@ func parseServiceConfig(js string) *serviceconfig.ParseResult {
 		}
 		if mc.retryPolicy, err = convertRetryPolicy(m.RetryPolicy); err != nil {
 			grpclog.Warningf("grpc: parseServiceConfig error unmarshaling %s due to %v", js, err)
-			return serviceconfig.NewParseResult(err)
+			return &serviceconfig.ParseResult{Err: err}
 		}
 		if m.MaxRequestMessageBytes != nil {
 			if *m.MaxRequestMessageBytes > int64(maxInt) {
@@ -361,13 +361,13 @@ func parseServiceConfig(js string) *serviceconfig.ParseResult {
 
 	if sc.retryThrottling != nil {
 		if mt := sc.retryThrottling.MaxTokens; mt <= 0 || mt > 1000 {
-			return serviceconfig.NewParseResult(fmt.Errorf("invalid retry throttling config: maxTokens (%v) out of range (0, 1000]", mt))
+			return &serviceconfig.ParseResult{Err: fmt.Errorf("invalid retry throttling config: maxTokens (%v) out of range (0, 1000]", mt)}
 		}
 		if tr := sc.retryThrottling.TokenRatio; tr <= 0 {
-			return serviceconfig.NewParseResult(fmt.Errorf("invalid retry throttling config: tokenRatio (%v) may not be negative", tr))
+			return &serviceconfig.ParseResult{Err: fmt.Errorf("invalid retry throttling config: tokenRatio (%v) may not be negative", tr)}
 		}
 	}
-	return serviceconfig.NewParseResult(&sc)
+	return &serviceconfig.ParseResult{Config: &sc}
 }
 
 func convertRetryPolicy(jrp *jsonRetryPolicy) (p *retryPolicy, err error) {

serviceconfig/serviceconfig.go

@@ -22,12 +22,6 @@
 // This package is EXPERIMENTAL.
 package serviceconfig
 
-import (
-	"google.golang.org/grpc/codes"
-	"google.golang.org/grpc/grpclog"
-	"google.golang.org/grpc/status"
-)
-
 // Config represents an opaque data structure holding a service config.
 type Config interface {
 	isServiceConfig()
@@ -39,21 +33,9 @@ type LoadBalancingConfig interface {
 	isLoadBalancingConfig()
 }
 
-// ParseResult contains a service config or an error.
+// ParseResult contains a service config or an error.  Exactly one must be
+// non-nil.
 type ParseResult struct {
 	Config Config
 	Err    error
 }
-
-// NewParseResult returns a ParseResult returning the provided parameter as
-// either the Config or Err field, depending upon its type.
-func NewParseResult(configOrError interface{}) *ParseResult {
-	if e, ok := configOrError.(error); ok {
-		return &ParseResult{Err: e}
-	}
-	if c, ok := configOrError.(Config); ok {
-		return &ParseResult{Config: c}
-	}
-	grpclog.Errorf("Unexpected configOrError type: %T", configOrError)
-	return &ParseResult{Err: status.Errorf(codes.Internal, "unexpected configOrError type: %T", configOrError)}
-}