diff --git a/plugin/helpers_bots.go b/plugin/helpers_bots.go index 664a954d5d..28ed4df3f0 100644 --- a/plugin/helpers_bots.go +++ b/plugin/helpers_bots.go @@ -4,9 +4,8 @@ package plugin import ( - "time" - "github.com/mattermost/mattermost-server/model" + "github.com/mattermost/mattermost-server/utils" "github.com/pkg/errors" ) @@ -17,11 +16,20 @@ func (p *HelpersImpl) EnsureBot(bot *model.Bot) (retBotId string, retErr error) } // If we fail for any reason, this could be a race between creation of bot and - // retreval from anouther EnsureBot. Just try the basic retrieve existing again. + // retrieval from another EnsureBot. Just try the basic retrieve existing again. defer func() { if retBotId == "" || retErr != nil { - time.Sleep(time.Second) - botIdBytes, err := p.API.KVGet(BOT_USER_KEY) + var err error + var botIdBytes []byte + + err = utils.ProgressiveRetry(func() error { + botIdBytes, err = p.API.KVGet(BOT_USER_KEY) + if err != nil { + return err + } + return nil + }) + if err == nil && botIdBytes != nil { retBotId = string(botIdBytes) retErr = nil diff --git a/utils/backoff.go b/utils/backoff.go new file mode 100644 index 0000000000..be6dcc9a79 --- /dev/null +++ b/utils/backoff.go @@ -0,0 +1,26 @@ +// Copyright (c) 2016-present Mattermost, Inc. All Rights Reserved. +// See License.txt for license information. + +package utils + +import ( + "time" +) + +var backoffTimeouts = []time.Duration{50 * time.Millisecond, 100 * time.Millisecond, 200 * time.Millisecond, 200 * time.Millisecond, 400 * time.Millisecond, 400 * time.Millisecond} + +// ProgressiveRetry executes a BackoffOperation and waits an increasing time before retrying the operation. +func ProgressiveRetry(operation func() error) error { + var err error + + for attempts := 0; attempts < len(backoffTimeouts); attempts++ { + err = operation() + if err == nil { + return nil + } + + time.Sleep(backoffTimeouts[attempts]) + } + + return err +} diff --git a/utils/backoff_test.go b/utils/backoff_test.go new file mode 100644 index 0000000000..1ebff82e73 --- /dev/null +++ b/utils/backoff_test.go @@ -0,0 +1,62 @@ +package utils + +import ( + "testing" + + "github.com/pkg/errors" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestProgressiveRetry(t *testing.T) { + var retries int + + type args struct { + operation func() error + } + tests := []struct { + name string + args args + wantErr bool + expectedRetries int + }{ + { + name: "Should fail and return error", + args: args{ + operation: func() error { + retries++ + return errors.New("Operation Failed") + }, + }, + wantErr: true, + expectedRetries: 6, + }, + { + name: "Should succeed after two retries", + args: args{ + operation: func() error { + retries++ + if retries == 2 { + return nil + } + + return errors.New("Operation Failed") + }, + }, + wantErr: false, + expectedRetries: 2, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + retries = 0 + + err := ProgressiveRetry(tt.args.operation) + if !tt.wantErr { + require.Nil(t, err) + } + + assert.Equal(t, tt.expectedRetries, retries) + }) + } +}