-
Notifications
You must be signed in to change notification settings - Fork 27
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
Add tests for interim response handling #147
base: main
Are you sure you want to change the base?
Conversation
This looks really good, thanks! I'll review more deeply as soon as I have a moment, but for now can you document the new directives in CONTRIBUTING.md? |
Undici is only available in Node.js, so don't require it by default in browsers
I have updated the documentation in CONTRIBUTING.md as well as the schema in testsuite-schema.json. I also fixed running the tests in browsers, where the In addition, I added two more test cases to cover the ideas from #78. Let me know if you had different intentions with them. |
Avoid interoperability issues with undici from userland and Node.js as in nodejs/undici#3973 (comment).
During one of the HTTP WG session at the IETF 121 meeting in Dublin, resumable uploads was discussed and the topic of support for interim responses in the current HTTP ecosystem came up. @mnot suggested that the cache-tests could be used to test how proxies handle interim responses.
This PR takes a first stab at this. It implements two tests for the 102 and 103 status codes. I would have liked to test an informational status code that is not yet registered to ensure that proxies don't limit their behavior to specific status codes, but currently Node.js' http module does not support sending arbitrary interim responses (see nodejs/node#27921 (comment)). The tests check that proxies pass interim responses through, but are still able to cache the final response.
So far, support for this is varying:
I can imagine that some of these failures are due to missing configuration settings. For example, for Nginx it was necessary to enable HTTP/1.1 to the origin. Otherwise it fell back to HTTP/1.0 which does not support interim responses as far as I know.
To accommodate the new tests, the
interim_response
andexpected_interim_responses
configurations have been added. Since the Fetch API itself does not expose interim responses to the caller, they have to be collected using the ability to intercept dispatcher in Node.js'undici
module, which is the implementation behind Node's Fetch API. See https://undici.nodejs.org/#/docs/api/Dispatcher?id=dispatchercomposeinterceptors-interceptor for more details.I would be happy for any feedback regarding the tests themselves, as well as the code changes.