Skip to content
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

Inconsistent behavior on Python 2.7 vs. Python 3.7 on Flask #360

Closed
mostafa opened this issue Jan 15, 2019 · 12 comments
Closed

Inconsistent behavior on Python 2.7 vs. Python 3.7 on Flask #360

mostafa opened this issue Jan 15, 2019 · 12 comments
Assignees

Comments

@mostafa
Copy link

mostafa commented Jan 15, 2019

Hey guys,

I am having an issue with webargs==5.x, in that, when I upgrade it from 4.x to 5.x, using Flask==1.0.2, it works and behaves normally on python 3.7, but it behaves differently on python 2.7 on the same code base. The issue is on this pull request mostafa/grest#84 and is evident on this build: https://travis-ci.org/mostafa/grest/builds/479199700. I've tried to trace my own code to see if there is a path I am not covering, but that seems not to be the case, because it passes all tests on python 3.x.

Thanks,
Mostafa.

@sloria
Copy link
Member

sloria commented Jan 16, 2019

Thanks for reporting @mostafa . That definitely seems suspicious. I don't have time to look into this right now, but if you do some digging and end up finding a fix, I'd gladly review and merge a PR.

@mostafa
Copy link
Author

mostafa commented Jan 17, 2019

I'll try to look into it to see if I can fix it! As far as I can trace the issue, it is related to the required property on different fields. It seems that Python 2.7 is required=True by default, but Python 3.7 is required=False. I know it is confusing, but that's one thing I just guessed by looking into multiple files from webargs and marshmallow.

@sloria
Copy link
Member

sloria commented Jan 17, 2019

Thanks for looking into it.

@mostafa
Copy link
Author

mostafa commented Jan 18, 2019

As I investigated more, the issue rises due to some minor differences between Python 2.7 and Python 3.7 and those are:

  1. In Python 2.7, an empty string is an instance of bytes, hence isinstance("", bytes) returns True, but reverse proves to be the case for Python 3.7.
  2. In Python 2.7, when an empty string is passed to the json.loads method, it raises a ValueError: No JSON object could be decoded, but Python 3.7 raises a JSONDecodeError: Expecting value: line 1 column 1 (char 0), which in Python 3.7 version, the _get_value method silently ignores the error.

This all mean that if one wants to parse JSON body from the request object and it is empty, it will return missing in case of Python 3.7, and raises an exception in Python 2.7.

@mostafa
Copy link
Author

mostafa commented Jan 18, 2019

One possible (very quick) solution is to pass {} as JSON request body instead of an empty body. This would eliminate the bug from happening altogether, obviously.

@mostafa
Copy link
Author

mostafa commented Jan 18, 2019

It seems that the issue that was fixed in 05d8e1f, has some relations to this bug. Because when I use the dev branch to test, it works fine.

@sloria
Copy link
Member

sloria commented Jan 19, 2019

Ah, interesting. I see that your mostafa/grest#84 was merged, so it looks like 5.1.0 had the fix.

Closing this for now.

@sloria sloria closed this as completed Jan 19, 2019
@mostafa
Copy link
Author

mostafa commented Jan 20, 2019

I merged it, but it didn't fix the issue. So I downgraded to 4.3.1 to be able to have a working version till the issue resolves. This BUG is playing a hide and seek game with me, hence why I named it inconsistent!

@sloria sloria reopened this Jan 20, 2019
@sloria
Copy link
Member

sloria commented Jan 28, 2019

I wonder if this is related to #363 . @mostafa Can you try explicitly installing simplejson in grest to see if that fixes it?

@mostafa
Copy link
Author

mostafa commented Jan 28, 2019

It seems that the ujson and the simplejson APIs are consistent in exception handling, but the internal python implementation raises different exceptions in the same conditions, e.g. empty string.
The simplejson library returns JSONDecodeError: Expecting value: line 1 column 1 (char 0) on Python 2 and 3, which is the standard Python 3 message and also the usjon one raises ValueError: Expected object or value, in which, the type of the exception is the same as Python 2's, but the message differs, and the exception and its messages are consistent in both Python versions.
What I tried -- as a quick fix -- was to explicitly pass an empty string as json data, and all the tests passed, except the YAML one, which is something different and it raises the same 422 error code which is not what I want and which is the same exact error, but on PyYAML library.

@mostafa
Copy link
Author

mostafa commented Jan 28, 2019

I monkey-patched the dumps and loads functions in flask with the ones from simplejson, and it worked perfectly. So, it seems safe to say that difference in the json APIs caused this!

mostafa added a commit to mostafa/grest that referenced this issue Jan 28, 2019
marshmallow-code/webargs#360 marshmallow-code/webargs#363

2. Add simplejson to requirements.txt
3. Remove quick-fix changes to tests
4. Bump webargs version to 5.1.0
mostafa added a commit to mostafa/grest that referenced this issue Jan 28, 2019
marshmallow-code/webargs#360 marshmallow-code/webargs#363

2. Add simplejson to requirements.txt
3. Remove quick-fix changes to tests
4. Bump webargs version to 5.1.0
@mostafa
Copy link
Author

mostafa commented Jan 28, 2019

Hopefully the builds are passing now with the 5.1.0 version.

@mostafa mostafa closed this as completed Jan 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants