Fix client IPs being broken on Python 3 #3908
Conversation
synapse/storage/_base.py
Outdated
| @@ -117,7 +117,7 @@ def _do_execute(self, func, sql, *args): | |||
| sql, *args | |||
| ) | |||
| except Exception as e: | |||
| logger.debug("[SQL FAIL] {%s} %s", self.name, e) | |||
| logger.error("[SQL FAIL] {%s} %s", self.name, e) | |||
There was a problem hiding this comment.
Please don't, as this can get really quite spammy iirc
There was a problem hiding this comment.
From the ticket from @richvdh :
if update_client_ips is failing, it needs to log at ERROR or above.
Do we not want to know when SQL fails?
There was a problem hiding this comment.
Well we raise and handle it later. There are a bunch of failure modes that are non-error case, but instead get retried: https://github.com/matrix-org/synapse/blob/master/synapse/storage/_base.py#L249-L280
I'd really expect the exception to bubble up all the way to the client IP code, so I wonder if we're just swallowing it there?
| "Couldn't drop old db: " + str(e), category=UserWarning | ||
| ) | ||
| time.sleep(0.5) | ||
|
|
There was a problem hiding this comment.
Can you add quick comment why this is necessary?
synapse/storage/client_ips.py
Outdated
| ) | ||
| except Exception as e: | ||
| # Failed to upsert, log and continue | ||
| logger.error("Failed to insert client IP: %r", entry) |
There was a problem hiding this comment.
could you log the reason for the failure (ie, e) as well as the fact it's failed?
Fixes #3893