GODT-1645: Changing timeouts to not send too many login attempts.

This commit is contained in:
Jakub
2022-08-08 13:55:00 +02:00
committed by Romain Le Jeune
parent bc6ec2579a
commit 73f8811a4b
4 changed files with 32 additions and 12 deletions

View File

@ -85,7 +85,7 @@ func newManager(cfg Config) *manager {
// The resty is increasing the delay between retries up to 1 minute // The resty is increasing the delay between retries up to 1 minute
// (SetRetryMaxWaitTime) so for 10 retries the cumulative delay can be // (SetRetryMaxWaitTime) so for 10 retries the cumulative delay can be
// up to 5min. // up to 5min.
m.rc.SetRetryCount(5) m.rc.SetRetryCount(3)
m.rc.SetRetryMaxWaitTime(time.Minute) m.rc.SetRetryMaxWaitTime(time.Minute)
m.rc.SetRetryAfter(catchRetryAfter) m.rc.SetRetryAfter(catchRetryAfter)
m.rc.AddRetryCondition(m.shouldRetry) m.rc.AddRetryCondition(m.shouldRetry)

View File

@ -62,6 +62,10 @@ func (m *manager) NewClientWithLogin(ctx context.Context, username string, passw
return nil, nil, err return nil, nil, err
} }
// Do not retry requests after this point. The ephemeral from auth info
// won't be valid any more
ctx = ContextWithoutRetry(ctx)
auth, err := m.auth(ctx, AuthReq{ auth, err := m.auth(ctx, AuthReq{
Username: username, Username: username,
ClientProof: base64.StdEncoding.EncodeToString(proofs.ClientProof), ClientProof: base64.StdEncoding.EncodeToString(proofs.ClientProof),

View File

@ -105,17 +105,27 @@ func logConnReuse(_ *resty.Client, res *resty.Response) error {
func catchRetryAfter(_ *resty.Client, res *resty.Response) (time.Duration, error) { func catchRetryAfter(_ *resty.Client, res *resty.Response) (time.Duration, error) {
if res.StatusCode() == http.StatusTooManyRequests { if res.StatusCode() == http.StatusTooManyRequests {
if after := res.Header().Get("Retry-After"); after != "" { if after := res.Header().Get("Retry-After"); after != "" {
l := log.
WithField("statusCode", res.StatusCode()).
WithField("url", res.Request.URL).
WithField("verb", res.Request.Method)
seconds, err := strconv.Atoi(after) seconds, err := strconv.Atoi(after)
if err != nil { if err != nil {
log.WithError(err).Warning("Cannot convert Retry-After to number") l.WithError(err).Warning("Cannot convert Retry-After to number")
seconds = 10 seconds = 10
} }
// To avoid spikes when all clients retry at the same time, we add some random wait. // To avoid spikes when all clients retry at the same time, we add some random wait.
seconds += rand.Intn(10) //nolint:gosec // It is OK to use weak random number generator here. seconds += rand.Intn(10) //nolint:gosec // It is OK to use weak random number generator here.
l = l.WithField("seconds", seconds).WithField("start", time.Now().Unix())
log.Warningf("Retrying %s after %ds induced by http code %d", res.Request.URL, seconds, res.StatusCode()) // Maximum retry time in client is is one minute. But
return time.Duration(seconds) * time.Second, nil // here wait times can be longer e.g. high API load
l.Warn("Retrying after induced by http code. Waiting now...")
time.Sleep(time.Duration(seconds) * time.Second)
l.Warn("Wait done")
return 0, nil
} }
} }

View File

@ -21,6 +21,7 @@ import (
"context" "context"
"fmt" "fmt"
"math/rand" "math/rand"
"os"
"path/filepath" "path/filepath"
"time" "time"
@ -29,6 +30,7 @@ import (
"github.com/ProtonMail/proton-bridge/v2/internal/users" "github.com/ProtonMail/proton-bridge/v2/internal/users"
"github.com/ProtonMail/proton-bridge/v2/pkg/pmapi" "github.com/ProtonMail/proton-bridge/v2/pkg/pmapi"
"github.com/pkg/errors" "github.com/pkg/errors"
"github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert" "github.com/stretchr/testify/assert"
) )
@ -52,14 +54,20 @@ func (ctx *TestContext) LoginUser(username string, password, mailboxPassword []b
} }
} }
userID, err := ctx.users.FinishLogin(client, auth, mailboxPassword) user, err := ctx.users.FinishLogin(client, auth, mailboxPassword)
if err != nil { if err != nil {
return errors.Wrap(err, "failed to finish login") return errors.Wrap(err, "failed to finish login")
} }
ctx.addCleanupChecked(func() error { ctx.addCleanupChecked(
return ctx.bridge.LogoutUser(userID) func() error {
}, "Logging out user") if os.Getenv(EnvName) == EnvLive {
logrus.Warn("Pausing user.Logout by two minutes to not hit issues with too many login attempts.")
time.Sleep(2 * time.Minute)
}
return user.Logout()
}, "Logging out user",
)
return nil return nil
} }
@ -75,14 +83,12 @@ func (ctx *TestContext) FinishLogin(client pmapi.Client, mailboxPassword []byte)
return errors.New("cannot get current auth tokens from client") return errors.New("cannot get current auth tokens from client")
} }
userID, err := ctx.users.FinishLogin(client, c.GetCurrentAuth(), mailboxPassword) user, err := ctx.users.FinishLogin(client, c.GetCurrentAuth(), mailboxPassword)
if err != nil { if err != nil {
return errors.Wrap(err, "failed to finish login") return errors.Wrap(err, "failed to finish login")
} }
ctx.addCleanupChecked(func() error { ctx.addCleanupChecked(user.Logout, "Logging out user")
return ctx.bridge.LogoutUser(userID)
}, "Logging out user")
return nil return nil
} }