feat(BRIDGE-340): additional logging for label operations & bad events

This commit is contained in:
Atanas Janeshliev
2025-03-21 16:37:12 +01:00
parent df409925ec
commit 4e764fe93d
9 changed files with 137 additions and 28 deletions

View File

@ -257,7 +257,7 @@ func (s *Connector) DeleteMailbox(ctx context.Context, _ connector.IMAPStateWrit
wLabels := s.labels.Write()
defer wLabels.Close()
wLabels.Delete(string(mboxID))
wLabels.Delete(string(mboxID), "connectorDeleteMailbox")
return nil
}
@ -555,7 +555,7 @@ func (s *Connector) createLabel(ctx context.Context, name []string) (imap.Mailbo
wLabels := s.labels.Write()
defer wLabels.Close()
wLabels.SetLabel(label.ID, label)
wLabels.SetLabel(label.ID, label, "connectorCreateLabel")
return toIMAPMailbox(label, s.flags, s.permFlags, s.attrs), nil
}
@ -593,7 +593,7 @@ func (s *Connector) createFolder(ctx context.Context, name []string) (imap.Mailb
}
// Add label to list so subsequent sub folder create requests work correct.
wLabels.SetLabel(label.ID, label)
wLabels.SetLabel(label.ID, label, "connectorCreateFolder")
return toIMAPMailbox(label, s.flags, s.permFlags, s.attrs), nil
}
@ -619,7 +619,7 @@ func (s *Connector) updateLabel(ctx context.Context, labelID imap.MailboxID, nam
wLabels := s.labels.Write()
defer wLabels.Close()
wLabels.SetLabel(label.ID, update)
wLabels.SetLabel(label.ID, update, "connectorUpdateLabel")
return nil
}
@ -660,7 +660,7 @@ func (s *Connector) updateFolder(ctx context.Context, labelID imap.MailboxID, na
return err
}
wLabels.SetLabel(label.ID, update)
wLabels.SetLabel(label.ID, update, "connectorUpdateFolder")
return nil
}

View File

@ -43,7 +43,7 @@ func TestFixGODT3003Labels(t *testing.T) {
Path: []string{"bar", "Foo"},
Color: "",
Type: proton.LabelTypeFolder,
})
}, "")
wr.SetLabel("0", proton.Label{
ID: "0",
@ -52,7 +52,7 @@ func TestFixGODT3003Labels(t *testing.T) {
Path: []string{"Inbox"},
Color: "",
Type: proton.LabelTypeSystem,
})
}, "")
wr.SetLabel("bar", proton.Label{
ID: "bar",
@ -61,7 +61,7 @@ func TestFixGODT3003Labels(t *testing.T) {
Path: []string{"bar"},
Color: "",
Type: proton.LabelTypeFolder,
})
}, "")
wr.SetLabel("my_label", proton.Label{
ID: "my_label",
@ -70,7 +70,7 @@ func TestFixGODT3003Labels(t *testing.T) {
Path: []string{"MyLabel"},
Color: "",
Type: proton.LabelTypeLabel,
})
}, "")
wr.SetLabel("my_label2", proton.Label{
ID: "my_label2",
@ -79,7 +79,7 @@ func TestFixGODT3003Labels(t *testing.T) {
Path: []string{labelPrefix, "MyLabel2"},
Color: "",
Type: proton.LabelTypeLabel,
})
}, "")
wr.Close()
mboxs := []imap.MailboxNoAttrib{
@ -133,7 +133,7 @@ func TestFixGODT3003Labels_Noop(t *testing.T) {
Path: []string{folderPrefix, "bar", "Foo"},
Color: "",
Type: proton.LabelTypeFolder,
})
}, "")
wr.SetLabel("0", proton.Label{
ID: "0",
@ -142,7 +142,7 @@ func TestFixGODT3003Labels_Noop(t *testing.T) {
Path: []string{"Inbox"},
Color: "",
Type: proton.LabelTypeSystem,
})
}, "")
wr.SetLabel("bar", proton.Label{
ID: "bar",
@ -151,7 +151,7 @@ func TestFixGODT3003Labels_Noop(t *testing.T) {
Path: []string{folderPrefix, "bar"},
Color: "",
Type: proton.LabelTypeFolder,
})
}, "")
wr.SetLabel("my_label", proton.Label{
ID: "my_label",
@ -160,7 +160,7 @@ func TestFixGODT3003Labels_Noop(t *testing.T) {
Path: []string{labelPrefix, "MyLabel"},
Color: "",
Type: proton.LabelTypeLabel,
})
}, "")
wr.SetLabel("my_label2", proton.Label{
ID: "my_label2",
@ -169,7 +169,7 @@ func TestFixGODT3003Labels_Noop(t *testing.T) {
Path: []string{labelPrefix, "MyLabel2"},
Color: "",
Type: proton.LabelTypeLabel,
})
}, "")
wr.Close()
mboxs := []imap.MailboxNoAttrib{

View File

@ -34,6 +34,8 @@ type IMAPServerManager interface {
) error
RemoveIMAPUser(ctx context.Context, deleteData bool, provider GluonIDProvider, addrID ...string) error
LogRemoteLabelIDs(ctx context.Context, provider GluonIDProvider, addrID ...string) error
}
type NullIMAPServerManager struct{}
@ -57,6 +59,14 @@ func (n NullIMAPServerManager) RemoveIMAPUser(
return nil
}
func (n NullIMAPServerManager) LogRemoteLabelIDs(
_ context.Context,
_ GluonIDProvider,
_ ...string,
) error {
return nil
}
func NewNullIMAPServerManager() *NullIMAPServerManager {
return &NullIMAPServerManager{}
}

View File

@ -355,6 +355,12 @@ func (s *Service) run(ctx context.Context) { //nolint gocyclo
case *onBadEventReq:
s.log.Debug("Bad Event Request")
// // Log remote label IDs stored in the local labelMap.
s.labels.LogLabels()
// Log the remote label IDs store in Gluon.
if err := s.logRemoteMailboxIDsFromServer(ctx, s.connectors); err != nil {
s.log.Warnf("Could not obtain remote mailbox IDs from server: %v", err)
}
err := s.removeConnectorsFromServer(ctx, s.connectors, false)
req.Reply(ctx, nil, err)
@ -572,6 +578,16 @@ func (s *Service) addConnectorsToServer(ctx context.Context, connectors map[stri
return nil
}
func (s *Service) logRemoteMailboxIDsFromServer(ctx context.Context, connectors map[string]*Connector) error {
addrIDs := make([]string, 0, len(connectors))
for _, c := range connectors {
addrIDs = append(addrIDs, c.addrID)
}
return s.serverManager.LogRemoteLabelIDs(ctx, s.gluonIDProvider, addrIDs...)
}
func (s *Service) removeConnectorsFromServer(ctx context.Context, connectors map[string]*Connector, deleteData bool) error {
addrIDs := make([]string, 0, len(connectors))

View File

@ -85,7 +85,7 @@ func onLabelCreated(ctx context.Context, s *Service, event proton.LabelEvent) []
wr := s.labels.Write()
defer wr.Close()
wr.SetLabel(event.Label.ID, event.Label)
wr.SetLabel(event.Label.ID, event.Label, "onLabelCreated")
for _, updateCh := range maps.Values(s.connectors) {
update := newMailboxCreatedUpdate(imap.MailboxID(event.ID), GetMailboxName(event.Label))
@ -121,7 +121,7 @@ func onLabelUpdated(ctx context.Context, s *Service, event proton.LabelEvent) ([
// Only update the label if it exists; we don't want to create it as a client may have just deleted it.
if _, ok := wr.GetLabel(label.ID); ok {
wr.SetLabel(label.ID, event.Label)
wr.SetLabel(label.ID, event.Label, "onLabelUpdatedLabelEventID")
}
// API doesn't notify us that the path has changed. We need to fetch it again.
@ -134,7 +134,7 @@ func onLabelUpdated(ctx context.Context, s *Service, event proton.LabelEvent) ([
}
// Update the label in the map.
wr.SetLabel(apiLabel.ID, apiLabel)
wr.SetLabel(apiLabel.ID, apiLabel, "onLabelUpdatedApiID")
// Notify the IMAP clients.
for _, updateCh := range maps.Values(s.connectors) {
@ -176,7 +176,7 @@ func onLabelDeleted(ctx context.Context, s *Service, event proton.LabelEvent) []
wr := s.labels.Write()
wr.Close()
wr.Delete(event.ID)
wr.Delete(event.ID, "onLabelDeleted")
s.eventPublisher.PublishEvent(ctx, events.UserLabelDeleted{
UserID: s.identityState.UserID(),

View File

@ -22,6 +22,8 @@ import (
"github.com/ProtonMail/go-proton-api"
"github.com/ProtonMail/proton-bridge/v3/internal/usertypes"
"github.com/bradenaw/juniper/xslices"
"github.com/sirupsen/logrus"
"golang.org/x/exp/maps"
)
@ -42,8 +44,8 @@ type labelsRead interface {
type labelsWrite interface {
labelsRead
SetLabel(id string, label proton.Label)
Delete(id string)
SetLabel(id string, label proton.Label, actionSource string)
Delete(id string, actionSource string)
}
type rwLabels struct {
@ -51,6 +53,22 @@ type rwLabels struct {
labels labelMap
}
func (r *rwLabels) LogLabels() {
r.lock.RLock()
defer r.lock.RUnlock()
remoteLabelIDs := make([]string, len(r.labels))
i := 0
for labelID := range r.labels {
remoteLabelIDs[i] = labelID
i++
}
logrus.WithFields(logrus.Fields{
"remoteLabelIDs": remoteLabelIDs,
}).Debug("Logging remote label IDs stored in labelMap")
}
func (r *rwLabels) Read() labelsRead {
r.lock.RLock()
return &rwLabelsRead{rw: r}
@ -75,6 +93,15 @@ func (r *rwLabels) SetLabels(labels []proton.Label) {
r.lock.Lock()
defer r.lock.Unlock()
labelIDs := xslices.Map(labels, func(label proton.Label) string {
return label.ID
})
logrus.WithFields(logrus.Fields{
"pkg": "rwLabels",
"labelIDs": labelIDs,
}).Info("Setting labels")
r.labels = usertypes.GroupBy(labels, func(label proton.Label) string { return label.ID })
}
@ -123,10 +150,20 @@ func (r rwLabelsWrite) GetLabels() []proton.Label {
return r.rw.getLabelsUnsafe()
}
func (r rwLabelsWrite) SetLabel(id string, label proton.Label) {
func (r rwLabelsWrite) SetLabel(id string, label proton.Label, actionSource string) {
logAction("SetLabel", actionSource, label.ID)
r.rw.labels[id] = label
}
func (r rwLabelsWrite) Delete(id string) {
func (r rwLabelsWrite) Delete(id string, actionSource string) {
logAction("Delete", actionSource, id)
delete(r.rw.labels, id)
}
func logAction(actionType, actionSource, labelID string) {
logrus.WithFields(logrus.Fields{
"pkg": "rwLabelsWrite",
"actionSource": actionSource,
"labelID": labelID,
}).Debug(actionType)
}

View File

@ -170,6 +170,14 @@ func (sm *Service) SetGluonDir(ctx context.Context, gluonDir string) error {
return err
}
func (sm *Service) LogRemoteLabelIDs(ctx context.Context, provider imapservice.GluonIDProvider, addrID ...string) error {
_, err := sm.requests.Send(ctx, &smRequestLogRemoteMailboxIDs{
addrID: addrID,
idProvider: provider,
})
return err
}
func (sm *Service) RemoveIMAPUser(ctx context.Context, deleteData bool, provider imapservice.GluonIDProvider, addrID ...string) error {
_, err := sm.requests.Send(ctx, &smRequestRemoveIMAPUser{
withData: deleteData,
@ -244,6 +252,10 @@ func (sm *Service) run(ctx context.Context, subscription events.Subscription) {
sm.handleLoadedUserCountChange(ctx)
}
case *smRequestLogRemoteMailboxIDs:
err := sm.logRemoteLabelIDsFromServer(ctx, r.addrID, r.idProvider)
request.Reply(ctx, nil, err)
case *smRequestRemoveIMAPUser:
err := sm.handleRemoveIMAPUser(ctx, r.withData, r.idProvider, r.addrID...)
request.Reply(ctx, nil, err)
@ -311,6 +323,35 @@ func (sm *Service) handleAddIMAPUser(ctx context.Context,
return sm.handleAddIMAPUserImpl(ctx, connector, addrID, idProvider, syncStateProvider)
}
func (sm *Service) logRemoteLabelIDsFromServer(ctx context.Context, addrIDs []string, idProvider imapservice.GluonIDProvider) error {
if sm.imapServer == nil {
return fmt.Errorf("no imap server instance running")
}
for _, addrID := range addrIDs {
gluonID, ok := idProvider.GetGluonID(addrID)
if !ok {
sm.log.Warnf("Could not find Gluon ID for addrID %v", addrID)
continue
}
log := sm.log.WithFields(logrus.Fields{
"addrID": addrID,
"gluonID": gluonID,
})
remoteLabelIDs, err := sm.imapServer.GetAllMailboxRemoteIDsForUser(ctx, gluonID)
if err != nil {
log.WithError(err).Error("Could not obtain remote label IDs for user")
continue
}
log.WithField("remoteLabelIDs", remoteLabelIDs).Debug("Logging Gluon remote Label IDs")
}
return nil
}
func (sm *Service) handleAddIMAPUserImpl(ctx context.Context,
connector connector.Connector,
addrID string,
@ -723,3 +764,8 @@ type smRequestAddSMTPAccount struct {
type smRequestRemoveSMTPAccount struct {
account *bridgesmtp.Service
}
type smRequestLogRemoteMailboxIDs struct {
addrID []string
idProvider imapservice.GluonIDProvider
}