Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle errors with no message but error code #1639

Merged
merged 2 commits into from
May 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -640,6 +640,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 @@ -688,6 +691,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