Repository: matrix-org/sydent (language: python)

Issue:
Catch failures to contact remote homeserver for `/register`
Make it and the other "the remove homeserver did something silly" cases explicitly return 500. Log a warning message so that we can see the problem in logs, but not in sentry. This addresses the bulk of the errors spotted by matrix-org/matrix-spec#40.

[The spec](https://matrix.org/docs/spec/identity_service/r0.3.0#post-matrix-identity-v2-account-register) has nothing to say about status codes in the failure case. (matrix-org/matrix-doc#3470)


Discussion / hints:
Another sentry error comes from this request timing out. I'll extend this PR to handle that case too.
> I'm not convinced about the 500 error, but I think at need clarification from the spec side there.

FWIW I'm not very happy with it either, but at least this is consistent with the existing behaviour.

Fix patch:
diff --git a/changelog.d/463.misc b/changelog.d/463.misc
new file mode 100644
index 00000000..a13cf686
--- /dev/null
+++ b/changelog.d/463.misc
@@ -0,0 +1,1 @@
+Demote a failure to parse JSON from homeservers in `/register` from an error to a warning.
\ No newline at end of file
diff --git a/sydent/http/httpclient.py b/sydent/http/httpclient.py
index 7001f74f..03522ef5 100644
--- a/sydent/http/httpclient.py
+++ b/sydent/http/httpclient.py
@@ -64,7 +64,7 @@ async def get_json(self, uri: str, max_size: Optional[int] = None) -> JsonDict:
             # json.loads doesn't allow bytes in Python 3.5
             json_body = json_decoder.decode(body.decode("UTF-8"))
         except Exception:
-            logger.exception("Error parsing JSON from %s", uri)
+            logger.warning("Error parsing JSON from %s", uri)
             raise
         if not isinstance(json_body, dict):
             raise TypeError
diff --git a/sydent/http/servlets/registerservlet.py b/sydent/http/servlets/registerservlet.py
index 9a852aa4..378d2661 100644
--- a/sydent/http/servlets/registerservlet.py
+++ b/sydent/http/servlets/registerservlet.py
@@ -15,6 +15,7 @@
 import logging
 import urllib
 from http import HTTPStatus
+from json import JSONDecodeError
 from typing import TYPE_CHECKING, Dict
 
 from twisted.internet.error import ConnectError, DNSLookupError
@@ -80,6 +81,10 @@ def federation_request_problem(error: str) -> Dict[str, str]:
             return federation_request_problem(
                 f"Unable to contact the Matrix homeserver ({type(e).__name__})"
             )
+        except JSONDecodeError:
+            return federation_request_problem(
+                "The Matrix homeserver returned invalid JSON"
+            )
 
         if "sub" not in result:
             return federation_request_problem(


Test patch:
diff --git a/tests/test_register.py b/tests/test_register.py
index 972b638a..65c62dc7 100644
--- a/tests/test_register.py
+++ b/tests/test_register.py
@@ -12,6 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 from http import HTTPStatus
+from json import JSONDecodeError
 from unittest.mock import patch
 
 import twisted.internet.error
@@ -76,3 +77,24 @@ def test_connection_failure(self, exc: Exception) -> None:
         # Check that we haven't just returned the generic error message in asyncjsonwrap
         self.assertNotEqual(channel.json_body["error"], "Internal Server Error")
         self.assertIn("contact", channel.json_body["error"])
+
+    def test_federation_does_not_return_json(self) -> None:
+        self.sydent.run()
+        request, channel = make_request(
+            self.sydent.reactor,
+            "POST",
+            "/_matrix/identity/v2/account/register",
+            content={
+                "matrix_server_name": "matrix.alice.com",
+                "access_token": "back_in_wonderland",
+            },
+        )
+        servlet = self.sydent.servlets.registerServlet
+        exc = JSONDecodeError("ruh roh", "C'est n'est pas une objet JSON", 0)
+        with patch.object(servlet.client, "get_json", side_effect=exc):
+            request.render(servlet)
+        self.assertEqual(channel.code, HTTPStatus.INTERNAL_SERVER_ERROR)
+        self.assertEqual(channel.json_body["errcode"], "M_UNKNOWN")
+        # Check that we haven't just returned the generic error message in asyncjsonwrap
+        self.assertNotEqual(channel.json_body["error"], "Internal Server Error")
+        self.assertIn("JSON", channel.json_body["error"])