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

BUG: wsgi.error should be TextIO, not BytesIO in WSGI transport #1828

Merged
merged 6 commits into from Sep 7, 2021

Conversation

adriangb
Copy link
Member

@adriangb adriangb commented Aug 31, 2021

I found this from here: https://github.com/falconry/falcon/blob/e255bff9ae5a90d0cb3fe9af7c16917f18a92dc3/falcon/request.py#L1879

Based on the documentation at https://modwsgi.readthedocs.io/en/master/user-guides/debugging-techniques.html#apache-error-log-files, I believe Falcon is right and HTTPX is wrong.

This PR:

  1. Adds a parameter so that we can test it and users can easily collect the logs in their tests if they want.
  2. Changes the default from an inaccessible BytesIO (wrong type of stream, can't be accessed by user) to sys.stderr so that users (or pytest automatically) will capture the logs.

@adriangb adriangb marked this pull request as draft August 31, 2021 16:20
@adriangb adriangb changed the title wsgi.error should be StringIO, not BytesIO in WSGI transport BUG: wsgi.error should be TextIO, not BytesIO in WSGI transport Aug 31, 2021
@adriangb adriangb marked this pull request as ready for review August 31, 2021 16:41
@tomchristie tomchristie added the enhancement New feature or request label Sep 7, 2021
@tomchristie
Copy link
Member

Neat little, addition, yup! Thanks!

@tomchristie tomchristie merged commit 0b4a832 into encode:master Sep 7, 2021
@adriangb adriangb deleted the patch-1 branch September 7, 2021 14:45
@tomchristie tomchristie mentioned this pull request Oct 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants