Skip to content

Improve Informativeness of Network Errors #4822

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 29, 2025

Conversation

Bibo-Joshi
Copy link
Member

Based on observations in #4821

  • Uses HTTPStatus.phrase as more descriptive error message text
  • wraps the call to parse_json_payload in a try-except to add information about failing to parse the server response while still raising the appropriate exception

@Bibo-Joshi Bibo-Joshi requested review from aelkheir and harshil21 June 9, 2025 13:01
@Bibo-Joshi Bibo-Joshi added the 🛠 refactor change type: refactor label Jun 9, 2025
@Bibo-Joshi Bibo-Joshi requested a review from Copilot June 9, 2025 13:01
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances network error handling by using HTTPStatus.phrase for more descriptive messages and chaining JSON parsing errors to provide context.

  • Improved test coverage to validate new error messages and parsing failure behavior
  • Updated _request_wrapper to build messages from HTTPStatus.phrase, wrap parse_json_payload in try/except, and chain exceptions
  • Added changelog entry for the feature

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
tests/request/test_request.py Adjusted tests to check for descriptive HTTP status phrases and JSON parse errors
src/telegram/request/_baserequest.py Enhanced error message construction, used HTTPStatus phrases, and chained parsing exceptions
changes/unreleased/4822.DrW3tJ3KoB8kTmHtNnNEpQ.toml Added changelog entry for this PR
Comments suppressed due to low confidence (2)

src/telegram/request/_baserequest.py:355

  • [nitpick] The variable name exception shadows the built-in exception concept. Consider renaming it to something like err or telegram_error for clarity.
exception: TelegramError = Forbidden(message)

src/telegram/request/_baserequest.py:318

  • [nitpick] Python 3.12 introduces HTTPStatus.is_success, which can replace the manual range check for success codes to make the intent clearer and the code more maintainable.
if HTTPStatus.OK <= code <= 299:

Copy link
Member

@harshil21 harshil21 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just another try-except! :D

@Bibo-Joshi Bibo-Joshi requested a review from harshil21 June 28, 2025 12:22
@Bibo-Joshi Bibo-Joshi merged commit 22ae75c into master Jun 29, 2025
42 of 44 checks passed
@Bibo-Joshi Bibo-Joshi deleted the improve-http-error-handling branch June 29, 2025 16:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🛠 refactor change type: refactor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants