Refactor Sync and fix initialSyncLimit

[t3chguy]

Cleans up Sync::sync to have a clearer control flow
Cleans up Sync::doSync to be a while loop rather than tail recursion due to poor stackframes
Cleans up Sync::doSync error handling to not grow the stack frame via indirect recursion
Fixes initialSyncLimit support which previously affected filter for all Syncs, rather than just the initial one

---

Here's what your changelog entry will look like:

🐛 Bug Fixes

• Refactor Sync and fix initialSyncLimit (#2587).

[weeman1337]

👍

[t3chguy]

Blocking until after RC due to risk

[kegsay]

This is a much needed improvement!

[kegsay]

Will this work as intended? If this filter has a timeline limit > 1 and you do an incremental sync where you previously only got 1 event, will that cause it to pull in more?

[t3chguy]

Tests fine:

t3chguy@Michael-t3chguy-MBP ~> curl 'https://matrix-client.matrix.org/_matrix/client/r0/sync?filter=%7B%22room%22%3A%7B%22state%22%3A%7B%22lazy_load_members%22%3Atrue%7D%2C%22timeline%22%3A%7B%22limit%22%3A1%7D%7D%7D&timeout=0&_cacheBuster=1661267868536' \
                                     -H 'authority: matrix-client.matrix.org' \
                                     -H 'accept: application/json' \
                                     -H 'accept-language: en-GB,en;q=0.9' \
                                     -H 'authorization: Bearer XXX' \
                                     -H 'origin: http://localhost:8080' \
                                     -H 'sec-ch-ua: "Chromium";v="104", " Not A;Brand";v="99", "Google Chrome";v="104"' \
                                     -H 'sec-ch-ua-mobile: ?0' \
                                     -H 'sec-ch-ua-platform: "macOS"' \
                                     -H 'sec-fetch-dest: empty' \
                                     -H 'sec-fetch-mode: cors' \
                                     -H 'sec-fetch-site: cross-site' \
                                     -H 'user-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/104.0.0.0 Safari/537.36' \
                                     --compressed | jq -r '.next_batch, .rooms.join."!phxFokrQGfZBcMPPln:riot.ovh".timeline'
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  7834    0  7834    0     0   112k      0 --:--:-- --:--:-- --:--:--  134k
m3208786589~3.3208786609~2.3208786609~1.3208786609_757284974_610383_1545249272_1559480105_3502853_564923765_5019202378_0
{
  "events": [
    {
      "content": {
        "avatar_url": "mxc://matrix.org/uVZUPCuxefrZSebLyjpvOvRi",
        "displayname": "webdevguru_test16",
        "membership": "join"
      },
      "origin_server_ts": 1661267896354,
      "sender": "@webdevguru_test16:matrix.org",
      "state_key": "@webdevguru_test16:matrix.org",
      "type": "m.room.member",
      "unsigned": {
        "replaces_state": "$gxY-CsSYo0iJLIrmbZbA0WVxd0RJzhiH8waXnWKA2dw",
        "prev_content": {
          "avatar_url": "mxc://matrix.org/uVZUPCuxefrZSebLyjpvOvRi",
          "displayname": "webdevguru_test16",
          "membership": "invite"
        },
        "prev_sender": "@x:riot.ovh",
        "age": 302576
      },
      "event_id": "$gJZnU0UU0c78HoUlaz6ps9DYh-oQc999rzhN6G8fbwg"
    }
  ],
  "prev_batch": "t91-3208776565_757284974_610383_1545249272_1559480105_3502853_564923765_5019202378_0",
  "limited": true
}
t3chguy@Michael-t3chguy-MBP ~> curl 'https://matrix-client.matrix.org/_matrix/client/r0/sync?filter=%7B%22room%22%3A%7B%22state%22%3A%7B%22lazy_load_members%22%3Atrue%7D%2C%22timeline%22%3A%7B%22limit%22%3A20%7D%7D%7D&timeout=0&_cacheBuster=1661267868536&since=m3208786589~3.3208786609~2.3208786609~1.3208786609_757284974_610383_1545249272_1559480105_3502853_564923765_5019202378_0' \
                                     -H 'authority: matrix-client.matrix.org' \
                                     -H 'accept: application/json' \
                                     -H 'accept-language: en-GB,en;q=0.9' \
                                     -H 'authorization: Bearer XXX' \
                                     -H 'origin: http://localhost:8080' \
                                     -H 'sec-ch-ua: "Chromium";v="104", " Not A;Brand";v="99", "Google Chrome";v="104"' \
                                     -H 'sec-ch-ua-mobile: ?0' \
                                     -H 'sec-ch-ua-platform: "macOS"' \
                                     -H 'sec-fetch-dest: empty' \
                                     -H 'sec-fetch-mode: cors' \
                                     -H 'sec-fetch-site: cross-site' \
                                     -H 'user-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/104.0.0.0 Safari/537.36' \
                                     --compressed | jq -r '.next_batch, .rooms.join."!phxFokrQGfZBcMPPln:riot.ovh".timeline'
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   829    0   829    0     0   4133      0 --:--:-- --:--:-- --:--:--  4631
s3208791343_757284974_617131_1545252597_1559484219_3502856_564925803_5019212484_0
{
  "events": [
    {
      "content": {
        "body": "a",
        "msgtype": "m.text",
        "org.matrix.msc1767.text": "a"
      },
      "origin_server_ts": 1661268210098,
      "sender": "@x:riot.ovh",
      "type": "m.room.message",
      "unsigned": {},
      "event_id": "$xw2hrC65b62rp3ABA-z03NHYLouIjrfBddCM0KMuBxs"
    },
    {
      "content": {
        "body": "b",
        "msgtype": "m.text",
        "org.matrix.msc1767.text": "b"
      },
      "origin_server_ts": 1661268210560,
      "sender": "@x:riot.ovh",
      "type": "m.room.message",
      "unsigned": {},
      "event_id": "$mSNLoSZYa9OST4m8yv1zMRgFL_bcQqDqfwNDUtY-JkQ"
    },
    {
      "content": {
        "body": "c",
        "msgtype": "m.text",
        "org.matrix.msc1767.text": "c"
      },
      "origin_server_ts": 1661268210897,
      "sender": "@x:riot.ovh",
      "type": "m.room.message",
      "unsigned": {},
      "event_id": "$5yeflSHZY-eCMU1F0fMPjpPutfaV6VEn1NiSlMeJLPk"
    },
    {
      "content": {
        "body": "d",
        "msgtype": "m.text",
        "org.matrix.msc1767.text": "d"
      },
      "origin_server_ts": 1661268211178,
      "sender": "@x:riot.ovh",
      "type": "m.room.message",
      "unsigned": {},
      "event_id": "$IbEEfy8AAVjdMlBqHnRBN1bsTZUk7gqVQcb7HQsvtXk"
    },
    {
      "content": {
        "body": "e",
        "msgtype": "m.text",
        "org.matrix.msc1767.text": "e"
      },
      "origin_server_ts": 1661268211503,
      "sender": "@x:riot.ovh",
      "type": "m.room.message",
      "unsigned": {},
      "event_id": "$EftWZvUsO0LoMpfaQu_yUccSYW3DPuhVAVKZDd_i2Q0"
    },
    {
      "content": {
        "body": "f",
        "msgtype": "m.text",
        "org.matrix.msc1767.text": "f"
      },
      "origin_server_ts": 1661268211779,
      "sender": "@x:riot.ovh",
      "type": "m.room.message",
      "unsigned": {},
      "event_id": "$TZkp_Gqoodg8A-oEj9YCVho21dtdYpWHellSBQdhC80"
    },
    {
      "content": {
        "body": "g",
        "msgtype": "m.text",
        "org.matrix.msc1767.text": "g"
      },
      "origin_server_ts": 1661268212443,
      "sender": "@x:riot.ovh",
      "type": "m.room.message",
      "unsigned": {},
      "event_id": "$yLzq2MSCq-8drNz-fAjlaxSIFWp6TX7Puk9AcLwPW-U"
    }
  ],
  "prev_batch": "s3208789296_757284974_617131_1545252597_1559484219_3502856_564925803_5019212484_0",
  "limited": false
}

Initial sync has limit=1 and returns a single event
Then I sent 7 events into the room
2nd sync has limit=20 and returns 7 events

[t3chguy]

Going to ignore the rest of the strict mode errors, they're not new, just in refactored code

[3nprob]

@t3chguy Do you think the failing CI tests in #2586 could be indicative of a regression introduced here? It only triggered as I rebased on develop.

[t3chguy]

@3nprob this PR didn't change a filter being created or not, just which filter was sent with the initial sync, so it is unlikely, also that failure seems isolated to your branch, which implies being caused by it. Given you force pushed it is not possible if tests ever passed on your branch.

[3nprob]

@3nprob this PR didn't change a filter being created or not, just which filter was sent with the initial sync, so it is unlikely, also that failure seems isolated to your branch, which implies being caused by it. Given you force pushed it is not possible if tests ever passed on your branch.

I'm not sure I understand exactly what you were saying. The change in #2586 is trivial and you can easily run the tests locally with and without the other changes in 48cce65 to verify that this bug was introduced in 1d2c705 and has been present in active codepaths but undetected ever since until now.

[3nprob]

...Ah, I see now that you probably figured it out already, as e808e0f was added on top of the #2586 before merging.