From b05e028f7d8befc045dcb62e87d09ad462bb277c Mon Sep 17 00:00:00 2001 From: Neil Alexander Date: Tue, 13 Sep 2022 12:52:09 +0100 Subject: [PATCH] Tweak `LoadMembershipAtEvent` behaviour when state not known (#2716) Previously `LoadMembershipAtEvent` would fail if the state before one of the events was not known, i.e. because it was an outlier. This modifies it so that it gracefully handles not knowing the state and returns no memberships instead, so that history visibility doesn't freak out and kill `/sync` requests dead. --- roomserver/api/query.go | 3 ++- roomserver/internal/query/query.go | 9 ++++++++- roomserver/state/state.go | 8 ++++++-- syncapi/streams/stream_pdu.go | 5 ++--- 4 files changed, 18 insertions(+), 7 deletions(-) diff --git a/roomserver/api/query.go b/roomserver/api/query.go index 32d63bb5..aa7dc473 100644 --- a/roomserver/api/query.go +++ b/roomserver/api/query.go @@ -439,6 +439,7 @@ type QueryMembershipAtEventRequest struct { // QueryMembershipAtEventResponse is the response to QueryMembershipAtEventRequest. type QueryMembershipAtEventResponse struct { - // Memberships is a map from eventID to a list of events (if any). + // Memberships is a map from eventID to a list of events (if any). Events that + // do not have known state will return an empty array here. Memberships map[string][]*gomatrixserverlib.HeaderedEvent `json:"memberships"` } diff --git a/roomserver/internal/query/query.go b/roomserver/internal/query/query.go index d08c5c49..b41a92e9 100644 --- a/roomserver/internal/query/query.go +++ b/roomserver/internal/query/query.go @@ -208,6 +208,9 @@ func (r *Queryer) QueryMembershipForUser( return err } +// QueryMembershipAtEvent returns the known memberships at a given event. +// If the state before an event is not known, an empty list will be returned +// for that event instead. func (r *Queryer) QueryMembershipAtEvent( ctx context.Context, request *api.QueryMembershipAtEventRequest, @@ -237,7 +240,11 @@ func (r *Queryer) QueryMembershipAtEvent( } for _, eventID := range request.EventIDs { - stateEntry := stateEntries[eventID] + stateEntry, ok := stateEntries[eventID] + if !ok { + response.Memberships[eventID] = []*gomatrixserverlib.HeaderedEvent{} + continue + } memberships, err := helpers.GetMembershipsAtState(ctx, r.DB, stateEntry, false) if err != nil { return fmt.Errorf("unable to get memberships at state: %w", err) diff --git a/roomserver/state/state.go b/roomserver/state/state.go index a40a2e9b..cb96d83e 100644 --- a/roomserver/state/state.go +++ b/roomserver/state/state.go @@ -18,6 +18,7 @@ package state import ( "context" + "database/sql" "fmt" "sort" "sync" @@ -134,11 +135,14 @@ func (v *StateResolution) LoadMembershipAtEvent( for i := range eventIDs { eventID := eventIDs[i] snapshotNID, err := v.db.SnapshotNIDFromEventID(ctx, eventID) - if err != nil { + if err != nil && err != sql.ErrNoRows { return nil, fmt.Errorf("LoadStateAtEvent.SnapshotNIDFromEventID failed for event %s : %w", eventID, err) } if snapshotNID == 0 { - return nil, fmt.Errorf("LoadStateAtEvent.SnapshotNIDFromEventID(%s) returned 0 NID, was this event stored?", eventID) + // If we don't know a state snapshot for this event then we can't calculate + // memberships at the time of the event, so skip over it. This means that + // it isn't guaranteed that the response map will contain every single event. + continue } snapshotNIDMap[snapshotNID] = append(snapshotNIDMap[snapshotNID], eventID) } diff --git a/syncapi/streams/stream_pdu.go b/syncapi/streams/stream_pdu.go index ffcf64df..0ab6de88 100644 --- a/syncapi/streams/stream_pdu.go +++ b/syncapi/streams/stream_pdu.go @@ -388,7 +388,7 @@ func applyHistoryVisibilityFilter( if err != nil { // Not a fatal error, we can continue without the stateEvents, // they are only needed if there are state events in the timeline. - logrus.WithError(err).Warnf("failed to get current room state") + logrus.WithError(err).Warnf("Failed to get current room state for history visibility") } alwaysIncludeIDs := make(map[string]struct{}, len(stateEvents)) for _, ev := range stateEvents { @@ -397,7 +397,6 @@ func applyHistoryVisibilityFilter( startTime := time.Now() events, err := internal.ApplyHistoryVisibilityFilter(ctx, db, rsAPI, recentEvents, alwaysIncludeIDs, userID, "sync") if err != nil { - return nil, err } logrus.WithFields(logrus.Fields{ @@ -405,7 +404,7 @@ func applyHistoryVisibilityFilter( "room_id": roomID, "before": len(recentEvents), "after": len(events), - }).Debug("applied history visibility (sync)") + }).Trace("Applied history visibility (sync)") return events, nil }