Fix exception reporting due to HTTP request errors. #7556
Conversation
These are business as usual errors, rather than stuff we want to log at error.
| raise e.to_synapse() from e | ||
| except RequestSendFailed as e: | ||
| raise SynapseError(502, "Failed to talk to master") from e |
There was a problem hiding this comment.
"from e" is implicit when inside an except block... maybe it's worth including for clarity?
There was a problem hiding this comment.
I don't believe that is true: https://www.python.org/dev/peps/pep-3134/#explicit-exception-chaining
There was a problem hiding this comment.
There was a problem hiding this comment.
hrm ISWYM, they seem to do slightly different things. How does the result differ?
There was a problem hiding this comment.
I mean, that is slightly more interesting question. TBH I was mostly going by the fact that we bother using six.raise_from elsewhere when wrapping like this.
There was a problem hiding this comment.
I wonder if it gets rid of some of the return deferred exceptions as we explicitly set the __cause__ and so ignore the implicit __context__?
There was a problem hiding this comment.
Asi in, I think the intention is that the implicit style is for "argh we failed to process this exception and raised another one" while the explicit style is for wrapping exceptions.
There was a problem hiding this comment.
You might be right. In which case, there are probably lots of other places we should be doing the same.
These are business as usual errors, rather than stuff we want to log at error.
These are business as usual errors, rather than stuff we want to log at error.