'Pagination' section of the spec is basically a lie

[richvdh]

There's a whole section in the spec about 'pagination', with the implication that lots of endpoints in the CS API follow the same pattern. In fact, the only one that I think follows this pattern is /messages, and others (/publicRooms, /notifications, /search) use different systems.

There's also an M_BAD_PAGINATION error code which I would be surprised to discover is actually used.

[richvdh]

I think the action here is to make sure that /messages (and its interaction with the tokens returned by /sync, /context, /search, etc) is adequately documented, and then kill the section.

[ara4n]

i think this might be a slight overstatement - i think there's some value in the various APIs which do pagination having the same shape (i.e. a chunks array, and a prev_batch and next_batch results which give opaque tokens which can be fed back to the API to go backwards & forwards, and whose tokens point to the gaps between objects in the chunks sequence). And I think other APIs /do/ follow this a bit?

[turt2live]

It looks like M_BAD_PAGINATION is not emitted from synapse at all:

• Declared: https://github.com/matrix-org/synapse/blob/74854a97191191b08101821753c2672efc2a65fd/synapse/api/errors.py#L37
• The only reference is an "EventStreamError" class: https://github.com/matrix-org/synapse/blob/74854a97191191b08101821753c2672efc2a65fd/synapse/api/errors.py#L267-L272
• ... which isn't used anywhere: https://github.com/matrix-org/synapse/search?q=EventStreamError&unscoped_q=EventStreamError

Clients do appear to make use of it where they can, however:

• android: https://github.com/matrix-org/matrix-android-sdk/blob/ccf12449b8f09b06a7a8f501b9d7a382270b2305/matrix-sdk/src/main/java/org/matrix/androidsdk/rest/model/MatrixError.java#L44
• ios: https://github.com/matrix-org/matrix-ios-sdk/blob/f61b94965de7e0d395214b30b17aeae761dc0c2f/MatrixSDK/MXError.m#L32

although they also don't make use of the code outside of declaring it. I think if we were to use it, it would be best to declare it as M_INVALID_PARAM which appears to be used elsewhere in the spec, but may not be used for pagination currently.

Thoughts?