Skip to content

Commit

Permalink
Merge pull request #1639 from agriffaut/patch/handle-errors-without-m…
Browse files Browse the repository at this point in the history
…essage

Handle errors with no message but error code
  • Loading branch information
dnwe authored May 5, 2020
2 parents 1e4e077 + c55e6a9 commit 9c1c364
Show file tree
Hide file tree
Showing 3 changed files with 118 additions and 0 deletions.
6 changes: 6 additions & 0 deletions admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -643,6 +643,9 @@ func (ca *clusterAdmin) DescribeConfig(resource ConfigResource) ([]ConfigEntry,
if rspResource.ErrorMsg != "" {
return nil, errors.New(rspResource.ErrorMsg)
}
if rspResource.ErrorCode != 0 {
return nil, KError(rspResource.ErrorCode)
}
for _, cfgEntry := range rspResource.Configs {
entries = append(entries, *cfgEntry)
}
Expand Down Expand Up @@ -691,6 +694,9 @@ func (ca *clusterAdmin) AlterConfig(resourceType ConfigResourceType, name string
if rspResource.ErrorMsg != "" {
return errors.New(rspResource.ErrorMsg)
}
if rspResource.ErrorCode != 0 {
return KError(rspResource.ErrorCode)
}
}
}
return nil
Expand Down
64 changes: 64 additions & 0 deletions admin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -704,6 +704,39 @@ func TestClusterAdminDescribeConfig(t *testing.T) {
}
}

func TestClusterAdminDescribeConfigWithErrorCode(t *testing.T) {
seedBroker := NewMockBroker(t, 1)
defer seedBroker.Close()

seedBroker.SetHandlerByMap(map[string]MockResponse{
"MetadataRequest": NewMockMetadataResponse(t).
SetController(seedBroker.BrokerID()).
SetBroker(seedBroker.Addr(), seedBroker.BrokerID()),
"DescribeConfigsRequest": NewMockDescribeConfigsResponseWithErrorCode(t),
})

config := NewConfig()
config.Version = V1_1_0_0
admin, err := NewClusterAdmin([]string{seedBroker.Addr()}, config)
if err != nil {
t.Fatal(err)
}
defer func() {
_ = admin.Close()
}()

resource := ConfigResource{
Name: "r1",
Type: TopicResource,
ConfigNames: []string{"my_topic"},
}

_, err = admin.DescribeConfig(resource)
if err == nil {
t.Fatal(errors.New("ErrorCode present but no Error returned"))
}
}

// TestClusterAdminDescribeBrokerConfig ensures that a describe broker config
// is sent to the broker in the resource struct, _not_ the controller
func TestClusterAdminDescribeBrokerConfig(t *testing.T) {
Expand Down Expand Up @@ -789,6 +822,37 @@ func TestClusterAdminAlterConfig(t *testing.T) {
}
}

func TestClusterAdminAlterConfigWithErrorCode(t *testing.T) {
seedBroker := NewMockBroker(t, 1)
defer seedBroker.Close()

seedBroker.SetHandlerByMap(map[string]MockResponse{
"MetadataRequest": NewMockMetadataResponse(t).
SetController(seedBroker.BrokerID()).
SetBroker(seedBroker.Addr(), seedBroker.BrokerID()),
"AlterConfigsRequest": NewMockAlterConfigsResponseWithErrorCode(t),
})

config := NewConfig()
config.Version = V1_0_0_0
admin, err := NewClusterAdmin([]string{seedBroker.Addr()}, config)
if err != nil {
t.Fatal(err)
}
defer func() {
_ = admin.Close()
}()

var value string
entries := make(map[string]*string)
value = "60000"
entries["retention.ms"] = &value
err = admin.AlterConfig(TopicResource, "my_topic", entries, false)
if err == nil {
t.Fatal(errors.New("ErrorCode present but no Error returned"))
}
}

func TestClusterAdminAlterBrokerConfig(t *testing.T) {
controllerBroker := NewMockBroker(t, 1)
defer controllerBroker.Close()
Expand Down
48 changes: 48 additions & 0 deletions mockresponses.go
Original file line number Diff line number Diff line change
Expand Up @@ -853,6 +853,31 @@ func (mr *MockDescribeConfigsResponse) For(reqBody versionedDecoder) encoderWith
return res
}

type MockDescribeConfigsResponseWithErrorCode struct {
t TestReporter
}

func NewMockDescribeConfigsResponseWithErrorCode(t TestReporter) *MockDescribeConfigsResponseWithErrorCode {
return &MockDescribeConfigsResponseWithErrorCode{t: t}
}

func (mr *MockDescribeConfigsResponseWithErrorCode) For(reqBody versionedDecoder) encoderWithHeader {
req := reqBody.(*DescribeConfigsRequest)
res := &DescribeConfigsResponse{
Version: req.Version,
}

for _, r := range req.Resources {
res.Resources = append(res.Resources, &ResourceResponse{
Name: r.Name,
Type: r.Type,
ErrorCode: 83,
ErrorMsg: "",
})
}
return res
}

type MockAlterConfigsResponse struct {
t TestReporter
}
Expand All @@ -874,6 +899,29 @@ func (mr *MockAlterConfigsResponse) For(reqBody versionedDecoder) encoderWithHea
return res
}

type MockAlterConfigsResponseWithErrorCode struct {
t TestReporter
}

func NewMockAlterConfigsResponseWithErrorCode(t TestReporter) *MockAlterConfigsResponseWithErrorCode {
return &MockAlterConfigsResponseWithErrorCode{t: t}
}

func (mr *MockAlterConfigsResponseWithErrorCode) For(reqBody versionedDecoder) encoderWithHeader {
req := reqBody.(*AlterConfigsRequest)
res := &AlterConfigsResponse{}

for _, r := range req.Resources {
res.Resources = append(res.Resources, &AlterConfigsResourceResponse{
Name: r.Name,
Type: r.Type,
ErrorCode: 83,
ErrorMsg: "",
})
}
return res
}

type MockCreateAclsResponse struct {
t TestReporter
}
Expand Down

0 comments on commit 9c1c364

Please sign in to comment.