Skip to content

Commit

Permalink
Improve service config errors handling
Browse files Browse the repository at this point in the history
  • Loading branch information
amenzhinsky committed Jun 30, 2020
1 parent ef23d5c commit 158b808
Show file tree
Hide file tree
Showing 3 changed files with 28 additions and 10 deletions.
7 changes: 4 additions & 3 deletions clientconn.go
Expand Up @@ -860,9 +860,10 @@ func (ac *addrConn) tryUpdateAddrs(addrs []resolver.Address) bool {
// GetMethodConfig gets the method config of the input method.
// If there's an exact match for input method (i.e. /service/method), we return
// the corresponding MethodConfig.
// If there isn't an exact match for the input method, we look for the default config
// under the service (i.e /service/). If there is a default MethodConfig for
// the service, we return it.
// If there isn't an exact match for the input method, we look for the service's default
// config under the service (i.e /service/) and then for the default for all services (empty string).
//
// If there is a default MethodConfig for the service, we return it.
// Otherwise, we return an empty MethodConfig.
func (cc *ClientConn) GetMethodConfig(method string) MethodConfig {
// TODO: Avoid the locking here.
Expand Down
15 changes: 8 additions & 7 deletions service_config.go
Expand Up @@ -230,16 +230,20 @@ type jsonName struct {
Method *string
}

var errIgnoreName = errors.New("ignore name")
var (
errMissingService = errors.New("'service' is missing")
errDuplicatedName = errors.New("duplicated name")
errEmptyServiceNonEmptyMethod = errors.New("cannot combine empty 'service' and non-empty 'method'")
)

func (j jsonName) generatePath() (string, error) {
if j.Service == nil {
return "", errIgnoreName
return "", errMissingService
}
if *j.Service == "" {
// when 'service' is empty 'method' must be empty as well
if j.Method != nil && *j.Method != "" {
return "", errors.New("cannot combine empty 'service' and non-empty 'method'")
return "", errEmptyServiceNonEmptyMethod
}
return "", nil
}
Expand Down Expand Up @@ -336,15 +340,12 @@ func parseServiceConfig(js string) *serviceconfig.ParseResult {
for i, n := range *m.Name {
path, err := n.generatePath()
if err != nil {
if err == errIgnoreName {
continue
}
grpclog.Warningf("grpc: parseServiceConfig error unmarshaling %s due to methodConfig[%d]: %v", js, i, err)
return &serviceconfig.ParseResult{Err: err}
}

if _, ok := paths[path]; ok {
err = errors.New("duplicated name")
err = errDuplicatedName
grpclog.Warningf("grpc: parseServiceConfig error unmarshaling %s due to methodConfig[%d]: %v", js, i, err)
return &serviceconfig.ParseResult{Err: err}
}
Expand Down
16 changes: 16 additions & 0 deletions service_config_test.go
Expand Up @@ -373,6 +373,22 @@ func (s) TestParseMsgSize(t *testing.T) {
runParseTests(t, testcases)
}

func (s) TestParseMethodConfigNullService(t *testing.T) {
runParseTests(t, []parseTestCase{{
`{
"methodConfig": [{
"name": [
{
"service": null,
"method": "Bar"
}
],
"waitForReady": true
}]
}`, nil, true,
}})
}

func (s) TestParseMethodConfigEmptyServiceAndNonEmptyMethod(t *testing.T) {
runParseTests(t, []parseTestCase{{
`{
Expand Down

0 comments on commit 158b808

Please sign in to comment.