Skip to content

Commit

Permalink
GOCBC-1149: Do not try parse response on error in GetAllScopes
Browse files Browse the repository at this point in the history
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 <brett19@gmail.com>
Tested-by: Charles Dixon <chvckd@gmail.com>
  • Loading branch information
chvck committed Aug 10, 2021
1 parent ed16d5d commit ec0df67
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 5 deletions.
10 changes: 5 additions & 5 deletions bucket_collectionsmgr.go
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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)

Expand Down
16 changes: 16 additions & 0 deletions bucket_collectionsmgr_test.go
Expand Up @@ -3,6 +3,7 @@ package gocb
import (
"errors"
"fmt"
"github.com/stretchr/testify/mock"
"strconv"
"time"
)
Expand Down Expand Up @@ -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)
}

0 comments on commit ec0df67

Please sign in to comment.