Skip to content

Conversation

@Singletoned
Copy link
Contributor

The WSGI adapter was trying to take the status code from start_response straightaway, rather than waiting until the first non-empty chunk, as per the spec: https://www.python.org/dev/peps/pep-3333/#the-start-response-callable

I've made a small change that iterates through the response until it gets a non-empty chunk, and then passes everything on as normal. I've also added a test for that scenario.

@florimondmanca florimondmanca requested a review from a team March 28, 2020 21:02
Copy link
Contributor

@florimondmanca florimondmanca left a comment

Choose a reason for hiding this comment

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

Good catch, thanks! I checked out the code locally and it looks good (tests are failing without the fix). Left some suggestions here to reuse existing tests.

@Singletoned
Copy link
Contributor Author

I removed the "Content-length" header from the tests because it isn't actually used anywhere in the code, and if people look at the tests as an example, it might be misleading.

Copy link
Contributor

@yeraydiazdiaz yeraydiazdiaz left a comment

Choose a reason for hiding this comment

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

Thanks @Singletoned! Just left a question regarding an edge case

Relevant section of PEP 3333 for reference:

However, the start_response callable must not actually transmit the response headers. Instead, it must store them for the server or gateway to transmit only after the first iteration of the application return value that yields a non-empty bytestring, or upon the application's first invocation of the write() callable. In other words, response headers must not be sent until there is actual body data available, or until the application's returned iterable is exhausted. (The only possible exception to this rule is if the response headers explicitly include a Content-Length of zero.)

This delaying of response header transmission is to ensure that buffered and asynchronous applications can replace their originally intended output with error output, up until the last possible moment. For example, the application may need to change the response status from "200 OK" to "500 Internal Error", if an error occurs while the body is being generated within an application buffer.

Singletoned and others added 3 commits March 29, 2020 11:47
Co-Authored-By: Florimond Manca <[email protected]>
Co-Authored-By: Florimond Manca <[email protected]>
Co-Authored-By: Florimond Manca <[email protected]>
Copy link
Contributor

@florimondmanca florimondmanca left a comment

Choose a reason for hiding this comment

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

All set on my side once this last fix is in! 👍

Thank you so much!

Co-Authored-By: Florimond Manca <[email protected]>
@florimondmanca florimondmanca changed the title Handle generator WSGI app Fix support for generator-based WSGI apps Mar 29, 2020
@florimondmanca florimondmanca merged commit 94323f9 into encode:master Mar 29, 2020
@lovelydinosaur lovelydinosaur mentioned this pull request May 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants