diff --git a/changes/unreleased/4822.DrW3tJ3KoB8kTmHtNnNEpQ.toml b/changes/unreleased/4822.DrW3tJ3KoB8kTmHtNnNEpQ.toml new file mode 100644 index 000000000..060021dc6 --- /dev/null +++ b/changes/unreleased/4822.DrW3tJ3KoB8kTmHtNnNEpQ.toml @@ -0,0 +1,5 @@ +other = "Improve Informativeness of Network Errors Raised by ``BaseRequest.post/retrieve``" + +[[pull_requests]] +uid = "4822" +author_uid = "Bibo-Joshi" diff --git a/src/telegram/request/_baserequest.py b/src/telegram/request/_baserequest.py index 666f2d042..879d79d1c 100644 --- a/src/telegram/request/_baserequest.py +++ b/src/telegram/request/_baserequest.py @@ -317,45 +317,61 @@ class BaseRequest( if HTTPStatus.OK <= code <= 299: # 200-299 range are HTTP success statuses + # starting with Py 3.12 we can use `HTTPStatus.is_success` return payload - response_data = self.parse_json_payload(payload) + try: + message = f"{HTTPStatus(code).phrase} ({code})" + except ValueError: + message = f"Unknown HTTPError ({code})" - description = response_data.get("description") - message = description if description else "Unknown HTTPError" + parsing_exception: Optional[TelegramError] = None - # In some special cases, we can raise more informative exceptions: - # see https://core.telegram.org/bots/api#responseparameters and - # https://core.telegram.org/bots/api#making-requests - # TGs response also has the fields 'ok' and 'error_code'. - # However, we rather rely on the HTTP status code for now. - parameters = response_data.get("parameters") - if parameters: - migrate_to_chat_id = parameters.get("migrate_to_chat_id") - if migrate_to_chat_id: - raise ChatMigrated(migrate_to_chat_id) - retry_after = parameters.get("retry_after") - if retry_after: - raise RetryAfter(retry_after) + try: + response_data = self.parse_json_payload(payload) + except TelegramError as exc: + message += f". Parsing the server response {payload!r} failed" + parsing_exception = exc + else: + message = response_data.get("description") or message - message += f"\nThe server response contained unknown parameters: {parameters}" + # In some special cases, we can raise more informative exceptions: + # see https://core.telegram.org/bots/api#responseparameters and + # https://core.telegram.org/bots/api#making-requests + # TGs response also has the fields 'ok' and 'error_code'. + # However, we rather rely on the HTTP status code for now. + parameters = response_data.get("parameters") + if parameters: + migrate_to_chat_id = parameters.get("migrate_to_chat_id") + if migrate_to_chat_id: + raise ChatMigrated(migrate_to_chat_id) + retry_after = parameters.get("retry_after") + if retry_after: + raise RetryAfter(retry_after) + + message += f". The server response contained unknown parameters: {parameters}" if code == HTTPStatus.FORBIDDEN: # 403 - raise Forbidden(message) - if code in (HTTPStatus.NOT_FOUND, HTTPStatus.UNAUTHORIZED): # 404 and 401 + exception: TelegramError = Forbidden(message) + elif code in (HTTPStatus.NOT_FOUND, HTTPStatus.UNAUTHORIZED): # 404 and 401 # TG returns 404 Not found for # 1) malformed tokens # 2) correct tokens but non-existing method, e.g. api.tg.org/botTOKEN/unkonwnMethod # 2) is relevant only for Bot.do_api_request, where we have special handing for it. # TG returns 401 Unauthorized for correctly formatted tokens that are not valid - raise InvalidToken(message) - if code == HTTPStatus.BAD_REQUEST: # 400 - raise BadRequest(message) - if code == HTTPStatus.CONFLICT: # 409 - raise Conflict(message) - if code == HTTPStatus.BAD_GATEWAY: # 502 - raise NetworkError(description or "Bad Gateway") - raise NetworkError(f"{message} ({code})") + exception = InvalidToken(message) + elif code == HTTPStatus.BAD_REQUEST: # 400 + exception = BadRequest(message) + elif code == HTTPStatus.CONFLICT: # 409 + exception = Conflict(message) + elif code == HTTPStatus.BAD_GATEWAY: # 502 + exception = NetworkError(message) + else: + exception = NetworkError(message) + + if parsing_exception: + raise exception from parsing_exception + raise exception @staticmethod def parse_json_payload(payload: bytes) -> JSONDict: diff --git a/tests/request/test_request.py b/tests/request/test_request.py index 1672b8fb6..e6c6d1759 100644 --- a/tests/request/test_request.py +++ b/tests/request/test_request.py @@ -316,10 +316,14 @@ class TestRequestWithoutRequest: (-1, NetworkError), ], ) + @pytest.mark.parametrize("description", ["Test Message", None]) async def test_special_errors( - self, monkeypatch, httpx_request: HTTPXRequest, code, exception_class + self, monkeypatch, httpx_request: HTTPXRequest, code, exception_class, description ): - server_response = b'{"ok": "False", "description": "Test Message"}' + server_response_json = {"ok": False} + if description: + server_response_json["description"] = description + server_response = json.dumps(server_response_json).encode(TextEncoding.UTF_8) monkeypatch.setattr( httpx_request, @@ -327,7 +331,25 @@ class TestRequestWithoutRequest: mocker_factory(response=server_response, return_code=code), ) - with pytest.raises(exception_class, match="Test Message"): + if not description and code not in list(HTTPStatus): + match = f"Unknown HTTPError.*{code}" + else: + match = description or str(code.value) + + with pytest.raises(exception_class, match=match): + await httpx_request.post("", None, None) + + async def test_error_parsing_payload(self, monkeypatch, httpx_request: HTTPXRequest): + """Test that we raise an error if the payload is not a valid JSON.""" + server_response = b"invalid_json" + + monkeypatch.setattr( + httpx_request, + "do_request", + mocker_factory(response=server_response, return_code=HTTPStatus.BAD_GATEWAY), + ) + + with pytest.raises(TelegramError, match=r"502.*\. Parsing.*b'invalid_json' failed"): await httpx_request.post("", None, None) @pytest.mark.parametrize(