From ff914fa30cc2eaa070a34ffd33d523ac5836aee0 Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Mon, 4 May 2020 17:58:30 +0100 Subject: [PATCH] Review comments --- federationsender/internal/perform.go | 9 ++++++--- roomserver/internal/perform_leave.go | 28 ++++++++++++++-------------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/federationsender/internal/perform.go b/federationsender/internal/perform.go index cc23f5448..ab877c8a7 100644 --- a/federationsender/internal/perform.go +++ b/federationsender/internal/perform.go @@ -184,11 +184,13 @@ func (r *FederationSenderInternalAPI) PerformLeave( "membership": "leave", } if err = respMakeLeave.LeaveEvent.SetContent(content); err != nil { - return fmt.Errorf("respMakeLeave.LeaveEvent.SetContent: %w", err) + logrus.WithError(err).Warnf("respMakeLeave.LeaveEvent.SetContent failed") + continue } } if err = respMakeLeave.LeaveEvent.SetUnsigned(struct{}{}); err != nil { - return fmt.Errorf("respMakeLeave.LeaveEvent.SetUnsigned: %w", err) + logrus.WithError(err).Warnf("respMakeLeave.LeaveEvent.SetUnsigned failed") + continue } // Work out if we support the room version that has been supplied in @@ -206,7 +208,8 @@ func (r *FederationSenderInternalAPI) PerformLeave( respMakeLeave.RoomVersion, ) if err != nil { - return fmt.Errorf("respMakeLeave.LeaveEvent.Build: %w", err) + logrus.WithError(err).Warnf("respMakeLeave.LeaveEvent.Build failed") + continue } // Try to perform a send_leave using the newly built event. diff --git a/roomserver/internal/perform_leave.go b/roomserver/internal/perform_leave.go index eb41fe47e..422748e6a 100644 --- a/roomserver/internal/perform_leave.go +++ b/roomserver/internal/perform_leave.go @@ -38,13 +38,13 @@ func (r *RoomserverInternalAPI) performLeaveRoomByID( ) error { // If there's an invite outstanding for the room then respond to // that. - senderUser, err := r.isInvitePending(ctx, req, res) - if err == nil { + isInvitePending, senderUser, err := r.isInvitePending(ctx, req, res) + if err == nil && isInvitePending { return r.performRejectInvite(ctx, req, res, senderUser) } - // First of all we want to find out if the room exists and if the - // user is actually in it. + // There's no invite pending, so first of all we want to find out + // if the room exists and if the user is actually in it. latestReq := api.QueryLatestEventsAndStateRequest{ RoomID: req.RoomID, StateToFetch: []gomatrixserverlib.StateKeyTuple{ @@ -162,21 +162,21 @@ func (r *RoomserverInternalAPI) isInvitePending( ctx context.Context, req *api.PerformLeaveRequest, res *api.PerformLeaveResponse, // nolint:unparam -) (string, error) { +) (bool, string, error) { // Look up the room NID for the supplied room ID. roomNID, err := r.DB.RoomNID(ctx, req.RoomID) if err != nil { - return "", fmt.Errorf("r.DB.RoomNID: %w", err) + return false, "", fmt.Errorf("r.DB.RoomNID: %w", err) } // Look up the state key NID for the supplied user ID. targetUserNIDs, err := r.DB.EventStateKeyNIDs(ctx, []string{req.UserID}) if err != nil { - return "", fmt.Errorf("r.DB.EventStateKeyNIDs: %w", err) + return false, "", fmt.Errorf("r.DB.EventStateKeyNIDs: %w", err) } targetUserNID, targetUserFound := targetUserNIDs[req.UserID] if !targetUserFound { - return "", fmt.Errorf("missing NID for user %q (%+v)", req.UserID, targetUserNIDs) + return false, "", fmt.Errorf("missing NID for user %q (%+v)", req.UserID, targetUserNIDs) } // Let's see if we have an event active for the user in the room. If @@ -184,25 +184,25 @@ func (r *RoomserverInternalAPI) isInvitePending( // send_leave to. senderUserNIDs, err := r.DB.GetInvitesForUser(ctx, roomNID, targetUserNID) if err != nil { - return "", fmt.Errorf("r.DB.GetInvitesForUser: %w", err) + return false, "", fmt.Errorf("r.DB.GetInvitesForUser: %w", err) } if len(senderUserNIDs) == 0 { - return "", fmt.Errorf("no senderUserNIDs") + return false, "", nil } // Look up the user ID from the NID. senderUsers, err := r.DB.EventStateKeys(ctx, senderUserNIDs) if err != nil { - return "", fmt.Errorf("r.DB.EventStateKeys: %w", err) + return false, "", fmt.Errorf("r.DB.EventStateKeys: %w", err) } if len(senderUsers) == 0 { - return "", fmt.Errorf("no senderUsers") + return false, "", fmt.Errorf("no senderUsers") } senderUser, senderUserFound := senderUsers[senderUserNIDs[0]] if !senderUserFound { - return "", fmt.Errorf("missing user for NID %d (%+v)", senderUserNIDs[0], senderUsers) + return false, "", fmt.Errorf("missing user for NID %d (%+v)", senderUserNIDs[0], senderUsers) } - return senderUser, nil + return true, senderUser, nil }