Skip to content

Fix Server response could not be decoded using UTF-8 #1623

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.

Sign up for GitHub

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

Jump to bottom
Merged
tsnoam merged 5 commits into from
Feb 2, 2020
Merged

Fix Server response could not be decoded using UTF-8 #1623

tsnoam merged 5 commits into from
Feb 2, 2020

Conversation

@rikonaka
Copy link
Contributor

@rikonaka rikonaka commented Nov 19, 2019 •
edited
Loading

  • Remove the handling of UnicodeDecodeError errors beacause of using the decoded_s = json_data.decode('utf-8', 'replace').

  • Remove test for invalid JSON data cause there is no more error caused by decoding utf-8..

issues #1609

@rikonaka rikonaka changed the title Fix bugs in issus #1609 Fix bugs in issus [#1609](https://github.com/python-telegram-bot/python-telegram-bot/issues/1609) Nov 19, 2019
@rikonaka rikonaka changed the title Fix bugs in issus [#1609](https://github.com/python-telegram-bot/python-telegram-bot/issues/1609) Fix bugs in issus #1609 Nov 19, 2019
@rikonaka rikonaka changed the title Fix bugs in issus #1609 Fix bugs in issues #1609 Nov 19, 2019
@rikonaka
Copy link
Contributor Author

rikonaka commented Nov 19, 2019

I have some doubts about this automated test result.

test_send_live_location failed (2 runs remaining out of 3).
	<class 'telegram.error.BadRequest'>
	Message is not modified: specified new message content and reply markup are exactly the same as a current content and reply markup of the message
	[<TracebackEntry /home/runner/work/python-telegram-bot/python-telegram-bot/tests/test_location.py:60>, <TracebackEntry /home/runner/work/python-telegram-bot/python-telegram-bot/telegram/bot.py:66>, <TracebackEntry /home/runner/work/python-telegram-bot/python-telegram-bot/telegram/bot.py:1160>, <TracebackEntry /home/runner/work/python-telegram-bot/python-telegram-bot/telegram/bot.py:123>, <TracebackEntry /home/runner/work/python-telegram-bot/python-telegram-bot/telegram/utils/request.py:323>, <TracebackEntry /home/runner/work/python-telegram-bot/python-telegram-bot/telegram/utils/request.py:234>]
test_send_live_location failed (1 runs remaining out of 3).
	<class 'telegram.error.BadRequest'>
	Message is not modified: specified new message content and reply markup are exactly the same as a current content and reply markup of the message
	[<TracebackEntry /home/runner/work/python-telegram-bot/python-telegram-bot/tests/test_location.py:60>, <TracebackEntry /home/runner/work/python-telegram-bot/python-telegram-bot/telegram/bot.py:66>, <TracebackEntry /home/runner/work/python-telegram-bot/python-telegram-bot/telegram/bot.py:1160>, <TracebackEntry /home/runner/work/python-telegram-bot/python-telegram-bot/telegram/bot.py:123>, <TracebackEntry /home/runner/work/python-telegram-bot/python-telegram-bot/telegram/utils/request.py:323>, <TracebackEntry /home/runner/work/python-telegram-bot/python-telegram-bot/telegram/utils/request.py:234>]

I have never modified the code for test_location.py.

This should be solved by the guy responsible for this module.

@Bibo-Joshi
Copy link
Member

Bibo-Joshi commented Nov 19, 2019

The main problem still is test_parse_illegal_callback_data, since now no error is raised. Please adjust it accordingly.

@rikonaka
Copy link
Contributor Author

rikonaka commented Nov 19, 2019 •
edited
Loading

The main problem still is test_parse_illegal_callback_data, since now no error is raised. Please adjust it accordingly.

I think may be we should drop the invalid data which uses ignore instead of replace.

@Bibo-Joshi
Copy link
Member

Bibo-Joshi commented Nov 19, 2019

test_parse_illegal_callback_data is still expecting an error, though ;)

@rikonaka
Copy link
Contributor Author

rikonaka commented Nov 19, 2019 •
edited
Loading

test_parse_illegal_callback_data is still expecting an error, though ;)

Yes, I know where is the problem now, I use the replace or ignore in decode function, so there is no TelegramError anymore cause decoding invalid utf-8 data, I will remove this test for invalid data.

@rikonaka
Copy link
Contributor Author

rikonaka commented Nov 19, 2019

I think it should be okay now.

@Bibo-Joshi Bibo-Joshi added the 📋 pending-review work status: pending-review label Nov 19, 2019
@Bibo-Joshi Bibo-Joshi requested a review from tsnoam November 19, 2019 15:29
@Poolitzer Poolitzer changed the title Fix bugs in issues #1609 Fix Server response could not be decoded using UTF-8 Nov 19, 2019
@Poolitzer Poolitzer added this to the 12.4 milestone Nov 19, 2019
@isjerryxiao
Copy link

isjerryxiao commented Nov 28, 2019

Maybe my solution #1651 is better.

@rikonaka
Copy link
Contributor Author

rikonaka commented Dec 4, 2019 •
edited
Loading

Maybe my solution #1651 is better.

I have read your code and comment on your pull request, and it didn't solve the problem...😂

@isjerryxiao
Copy link

isjerryxiao commented Dec 5, 2019

It definitely solves the problem.

tsnoam
tsnoam approved these changes Feb 2, 2020
View reviewed changes
Copy link
Member

@tsnoam tsnoam left a comment

Choose a reason for hiding this comment

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

if CI pass we can merge

@tsnoam
Copy link
Member

tsnoam commented Feb 2, 2020

failure in unitests is unrelated

@tsnoam tsnoam merged commit 818475b into python-telegram-bot:master Feb 2, 2020
@tsnoam
Copy link
Member

tsnoam commented Feb 2, 2020

@rikonaka thank you for your contribution

@Bibo-Joshi Bibo-Joshi mentioned this pull request Feb 3, 2020
Closed
@github-actions github-actions bot locked and limited conversation to collaborators Aug 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@tsnoam tsnoam tsnoam approved these changes

Assignees

No one assigned

Labels

📋 pending-review work status: pending-review

Projects

None yet

Milestone

12.4

Development

Successfully merging this pull request may close these issues.

5 participants

@rikonaka @Bibo-Joshi @isjerryxiao @tsnoam @Poolitzer