Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

/sync can omit state changes when certain events are filtered out #16928

Closed
richvdh opened this issue Feb 16, 2024 · 3 comments · Fixed by #16930
Closed

/sync can omit state changes when certain events are filtered out #16928

richvdh opened this issue Feb 16, 2024 · 3 comments · Fixed by #16930
Labels
A-Sync T-Defect

Comments

@richvdh
Copy link
Member

richvdh commented Feb 16, 2024 •
edited
Loading

Consider the following DAG:

           E1
         ↗    ↖
        |      S2
        |      ↑
      --|------|----
        |      |
        E3     |
         ↖    /
           E4

The client has a filter that excludes E4, and a limit that means we return a timeline that is limited to after S2, represented by the horizontal dashed line above. (Either limit=1, or there are lots of events at E3.)

We still need to tell the client about S2, so it should appear in the state section of the response.

Currently, it is omitted; instead the timeline contains E3, and the state is empty.
@richvdh richvdh added the A-Sync label Feb 16, 2024
@richvdh
Copy link
Member Author

richvdh commented Feb 16, 2024

Possibly this also happens if the user has chosen to ignore messages from the sender of E4 ?

@richvdh
Copy link
Member Author

richvdh commented Feb 16, 2024

I think this was introduced by matrix-org/synapse@9e696bd.

richvdh added a commit that referenced this issue Feb 16, 2024
@richvdh
/sync: fix bug in calculating state response
bcfc3e1 
Fix an issue which could cause state to be omitted from the sync response if
the last event was filtered out.

Fixes: #16928
@richvdh richvdh mentioned this issue Feb 16, 2024
Merged
@richvdh
Copy link
Member Author

richvdh commented Feb 16, 2024

@kegsay has promised to write a complement test for this

richvdh added a commit that referenced this issue Feb 16, 2024
@richvdh
/sync: fix bug in calculating state response
5932a28 
Fix an issue which could cause state to be omitted from the sync response if
the last event was filtered out.

Fixes: #16928
richvdh added a commit that referenced this issue Feb 16, 2024
@richvdh
/sync: fix bug in calculating state response
50828c0 
Fix an issue which could cause state to be omitted from the sync response if
the last event was filtered out.

Fixes: #16928
kegsay added a commit to matrix-org/complement that referenced this issue Feb 16, 2024
@kegsay
Regression test for element-hq/synapse#16928
b0f2451 
And some drive-by logging improvements, along with a new
`ServerRoom.WaiterForEvent(eventID)` function which makes
waiting for an event to arrive on the Complement server
much much easier to write in code.
@richvdh richvdh mentioned this issue Feb 16, 2024
Merged
richvdh added a commit that referenced this issue Feb 17, 2024
@richvdh
/sync: fix bug in calculating state response
37e775d 
Fix an issue which could cause state to be omitted from the sync response if
the last event was filtered out.

Fixes: #16928
richvdh added a commit that referenced this issue Feb 19, 2024
@richvdh
/sync: fix bug in calculating state response
754f79e 
Fix an issue which could cause state to be omitted from the sync response if
the last event was filtered out.

Fixes: #16928
richvdh added a commit that referenced this issue Feb 19, 2024
@richvdh
/sync: fix bug in calculating state response
15ca2bd 
Fix an issue which could cause state to be omitted from the sync response if
the last event was filtered out.

Fixes: #16928
richvdh added a commit that referenced this issue Mar 14, 2024
@richvdh
/sync: fix bug in calculating state response
4abac50 
Fix an issue which could cause state to be omitted from the sync response if
the last event was filtered out.

Fixes: #16928
richvdh added a commit that referenced this issue Apr 4, 2024
@richvdh
/sync: fix bug in calculating state response
bfb5cc4 
Fix an issue which could cause state to be omitted from the sync response if
the last event was filtered out.

Fixes: #16928
richvdh added a commit that referenced this issue Apr 4, 2024
@richvdh
/sync: fix bug in calculating state response (#16930)
230b709 
Fix a long-standing issue which could cause state to be omitted from the
sync response if the last event was filtered out.

Fixes: #16928
erikjohnston pushed a commit to matrix-org/complement that referenced this issue May 2, 2024
@kegsay
Regression test for element-hq/synapse#16928 (#710)
370a014
@richvdh richvdh mentioned this issue May 31, 2024
Open
82 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-Sync T-Defect
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

/sync: fix bug in calculating state response element-hq/synapse
1 participant
@richvdh