-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Unhandled UnicodeDecodeError
exception if response with status 400 and request contains binary payload (for FastHttpUser)
#1447
Comments
I think this is a bug in geventhttpclient. It seems to assume anything that is of binary type can be decoded as utf-8 here: If you want, file a bug there (or better yet, a PR - I have admin there as well so I can merge it) |
Or, one could argue, geventhttpclient shouldnt even try to decode the response if it was a failure (line 366). After all, decoding doesnt normally happen until the user asks for the response text, so it is a bit weird to start doing it in error cases. |
Agree, especially because request and response are passed to the exception and can be handled directly if necessary |
@cyberw could you please take a look, I made a PR https://github.com/locustio/geventhttpclient/pull/2 |
Nice! Maybe the second change makes the first change unnecessary (I know I said I thought it was a bad idea to try and decode a broken response, but maybe there are cases where this is needed for debugging, so I kind of changed my mind :) Maybe instead of just silently ignoring the error ( |
Thank you. Done |
Thanks! I dont have time to build a release right now, but will do it in a few days... |
Oh wait, you should submit your PR to the main gwik/geventhttpclient repo, not the fork under locustio/geventhttpclient |
Bug
I get an unhandled
UnicodeDecodeError
exception if the response has an invalid status code and the request contains binary payload (for FastHttpUser)Expected behavior
UnicodeDecodeError
shouldn't be raised in case of non-utf-8 characters in the payloadActual behavior
That is a stack trace:
Steps to reproduce
Basically that's a known issue: an attempt to decode the fully binary string fails with
UnicodeDecodeError
. It might be reproduced even without locust, just try to decode any binary string:Environment
locust
The text was updated successfully, but these errors were encountered: