fix(GODT-2693): Duplicate messages in sent folder

* Only call `RemoveOnFail` on error. It was also called on success.
* Add some logging to track send times.
* Fix non-deterministic hash from `rfc8222.GetMessageHash`.
    https://github.com/ProtonMail/gluon/pull/395
This commit is contained in:
Leander Beernaert
2023-08-29 12:38:13 +02:00
parent 66c30716ea
commit f7252ed40e
4 changed files with 25 additions and 6 deletions

View File

@ -22,6 +22,7 @@ import (
"errors"
"fmt"
"io"
"time"
"github.com/ProtonMail/gluon/async"
"github.com/ProtonMail/gluon/logging"
@ -254,6 +255,13 @@ type sendMailReq struct {
func (s *Service) sendMail(ctx context.Context, req *sendMailReq) error {
defer async.HandlePanic(s.panicHandler)
start := time.Now()
s.log.Debug("Received send mail request")
defer func() {
end := time.Now()
s.log.Debugf("Send mail request finished in %v", end.Sub(start))
}()
if err := s.smtpSendMail(ctx, req.authID, req.from, req.to, req.r); err != nil {
if apiErr := new(proton.APIError); errors.As(err, &apiErr) {
s.log.WithError(apiErr).WithField("Details", apiErr.DetailsToString()).Error("failed to send message")

View File

@ -75,6 +75,7 @@ func (s *Service) smtpSendMail(ctx context.Context, authID string, from string,
}
// Check if we already tried to send this message recently.
s.log.Debug("Checking for duplicate message")
srID, ok, err := s.recorder.TryInsertWait(ctx, hash, to, time.Now().Add(90*time.Second))
if err != nil {
return fmt.Errorf("failed to check send hash: %w", err)
@ -83,12 +84,11 @@ func (s *Service) smtpSendMail(ctx context.Context, authID string, from string,
return nil
}
// If we fail to send this message, we should remove the hash from the send recorder.
defer s.recorder.RemoveOnFail(hash, srID)
// Create a new message parser from the reader.
parser, err := parser.New(bytes.NewReader(b))
if err != nil {
s.log.Debug("Message failed to send, removing from send recorder")
s.recorder.RemoveOnFail(hash, srID)
return fmt.Errorf("failed to create parser: %w", err)
}
@ -100,10 +100,12 @@ func (s *Service) smtpSendMail(ctx context.Context, authID string, from string,
// Load the user's mail settings.
settings, err := s.client.GetMailSettings(ctx)
if err != nil {
s.log.Debug("Message failed to send, removing from send recorder")
s.recorder.RemoveOnFail(hash, srID)
return fmt.Errorf("failed to get mail settings: %w", err)
}
return usertypes.WithAddrKR(s.identityState.User, fromAddr, s.keyPassProvider.KeyPass(), func(userKR, addrKR *crypto.KeyRing) error {
if err := usertypes.WithAddrKR(s.identityState.User, fromAddr, s.keyPassProvider.KeyPass(), func(userKR, addrKR *crypto.KeyRing) error {
// Use the first key for encrypting the message.
addrKR, err := addrKR.FirstKey()
if err != nil {
@ -150,10 +152,17 @@ func (s *Service) smtpSendMail(ctx context.Context, authID string, from string,
}
// If the message was successfully sent, we can update the message ID in the record.
s.log.Debug("Message sent successfully, signaling recorder")
s.recorder.SignalMessageSent(hash, srID, sent.ID)
return nil
})
}); err != nil {
s.log.Debug("Message failed to send, removing from send recorder")
s.recorder.RemoveOnFail(hash, srID)
return err
}
return nil
}
// sendWithKey sends the message with the given address key.