429 responses w/o retry_after_ms cause exception

[ulope]

Sometimes (no idea how this is triggered) synapse sends responses with a 429 status code without the retry_after_ms field. In those cases, the python client crashes with a KeyError since

matrix-python-sdk/matrix_client/api.py

Line 666 in 401439f

sleep(response.json()['retry_after_ms'] / 1000)

unconditionally expects retry_after_ms to be available on 429 responses.

A relatively simple solution would be to have our own internal exponential backoff counter that gets reset every time a request succeeds or an explicit retry_after_ms value is received.

What do you think?

[ulope]

Looking a bit more into this it seems that in the case I'm seeing the retry_after_ms field is actually nested inside the error value:

{
  "errcode": "M_UNKNOWN", 
  "error": "{\"errcode\":\"M_LIMIT_EXCEEDED\",\"error\":\"Too Many Requests\",\"retry_after_ms\":650}"
}

[non-Jedi]

Without doing any digging, this sounds like a synapse bug. @ulope, any
particular endpoint causing this problem? Or is it completely random?

If anyone starts a PR to try to address this, please don't add any blocking code
(such as a exponential backoff counter) directly to MatrixHttpApi. We want
those methods to return as soon as possible, and I'm not really happy with even
the 429-handling logic that currently lives there; I just haven't come up with a
better place to put it yet.

[ulope]

@non-Jedi I can't be sure (I'll have to re-run the test that triggered this when I get a chance) but IIRC it was either during login / registration or joining rooms.

[non-Jedi]

To be clear, I've merged a PR (in release v0.2.0) that works around this issue, but I'm leaving it open to track the upstream problem.