From ec0df67a7e87f7fadaded15a4a58b35fcdead1c4 Mon Sep 17 00:00:00 2001 From: Charles Dixon Date: Tue, 10 Aug 2021 13:48:29 +0100 Subject: [PATCH] GOCBC-1149: Do not try parse response on error in GetAllScopes Motivation ---------- If the management request fails to send for GetAllScopes then we attempt to parse the response body which causes a panic because the response object is nil. On error we should not be doing this. Changes ------- Move the call to tryParseErrorMessage out of the error handling block and into the resp.Status != 200 handling block. Change-Id: I856c026689967fef34207d3c2f99d68d5921000b Reviewed-on: http://review.couchbase.org/c/gocb/+/159007 Reviewed-by: Brett Lawson Tested-by: Charles Dixon --- bucket_collectionsmgr.go | 10 +++++----- bucket_collectionsmgr_test.go | 16 ++++++++++++++++ 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/bucket_collectionsmgr.go b/bucket_collectionsmgr.go index 90b1026..9938fdb 100644 --- a/bucket_collectionsmgr.go +++ b/bucket_collectionsmgr.go @@ -116,15 +116,15 @@ func (cm *CollectionManager) GetAllScopes(opts *GetAllScopesOptions) ([]ScopeSpe resp, err := cm.mgmtProvider.executeMgmtRequest(opts.Context, req) if err != nil { - colErr := cm.tryParseErrorMessage(&req, resp) - if colErr != nil { - return nil, colErr - } return nil, makeMgmtBadStatusError("failed to get all scopes", &req, resp) } defer ensureBodyClosed(resp.Body) if resp.StatusCode != 200 { + colErr := cm.tryParseErrorMessage(&req, resp) + if colErr != nil { + return nil, colErr + } return nil, makeMgmtBadStatusError("failed to get all scopes", &req, resp) } @@ -378,7 +378,7 @@ func (cm *CollectionManager) CreateScope(scopeName string, opts *CreateScopeOpti resp, err := cm.mgmtProvider.executeMgmtRequest(opts.Context, req) if err != nil { - return err + return makeGenericMgmtError(err, &req, resp) } defer ensureBodyClosed(resp.Body) diff --git a/bucket_collectionsmgr_test.go b/bucket_collectionsmgr_test.go index 45b3a72..f7157ae 100644 --- a/bucket_collectionsmgr_test.go +++ b/bucket_collectionsmgr_test.go @@ -3,6 +3,7 @@ package gocb import ( "errors" "fmt" + "github.com/stretchr/testify/mock" "strconv" "time" ) @@ -576,3 +577,18 @@ func (suite *IntegrationTestSuite) TestMaxNumberOfCollectionInScope() { suite.Require().True(success) } + +func (suite *UnitTestSuite) TestGetAllScopesMgmtRequestFails() { + provider := new(mockMgmtProvider) + provider.On("executeMgmtRequest", nil, mock.AnythingOfType("gocb.mgmtRequest")).Return(nil, errors.New("http send failure")) + + mgr := CollectionManager{ + mgmtProvider: provider, + tracer: &NoopTracer{}, + meter: &meterWrapper{meter: &NoopMeter{}, isNoopMeter: true}, + } + + scopes, err := mgr.GetAllScopes(nil) + suite.Require().NotNil(err) + suite.Require().Nil(scopes) +}