Suppress Matrix timeout errors in console (fixes #835) #887
Conversation
The Matrix connector will often receive a timeout error when doing a sync request. The catch-all except would just dump the entire HTML of the timeout response to the console. This output can be quite voluminous and not that useful. By checking for know timeout error codes we can suppress the useless HTML output and just log the occurrence at the info level for those that are interested to know when timeouts occur. In addition to the standard `504 Gateway Timeout` error we also check for `522 Connection timed out` and `524 A Timeout Error` which are timeout errors specific to Cloudflare. See: https://support.cloudflare.com/hc/en-us/articles/115003011431/ We check for these Cloudflare errors because the largest Matrix host (matrix.org) is fronted by Cloudflare.
|
Now that we are handling |
Codecov Report
@@ Coverage Diff @@
## master #887 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 34 34
Lines 2201 2201
=====================================
Hits 2201 2201
Continue to review full report at Codecov.
|
|
The broad exception has been used since the beginning of opsdroid, I believe it was implemented in order to catch any exception that doesn't fit the main one. This is the reason why it was added, I'm not entirely sure if it's something that we need. Although this is a great catch and it does make sense to add a different catch for the timeout errors, also, like you said it's good to avoid filling up the terminal with the whole html file 👍 |
|
There needs to be a broad exception catch somewhere to stop broken skills from killing the whole opsdroid process. However I'm happy to discuss whether it would be more appropriate to have it somewhere else. |
Description
The Matrix connector will often receive a timeout error when doing a sync request. The catch-all except would just dump the entire HTML of the timeout response to the console. This output can be quite voluminous and not that useful. By checking for know timeout error codes we can suppress the useless HTML output and just log the occurrence at the info level for those that are interested to know when timeouts occur.
In addition to the standard
504 Gateway Timeouterror we also check for522 Connection timed outand524 A Timeout Errorwhich are timeout errors specific to Cloudflare. See:https://support.cloudflare.com/hc/en-us/articles/115003011431/
We check for these Cloudflare errors because the largest Matrix host (matrix.org) is fronted by Cloudflare.
Fixes #835
Status
READY
Type of change
How Has This Been Tested?
As far as I can tell there are no existing tests covering this part of the codebase, but the changes have been tested against the matrix.org homeserver using Python 3.7.2+ (Debian testing).
Checklist: