-
Notifications
You must be signed in to change notification settings - Fork 42
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
fix: do not auto destroy request stream #132
fix: do not auto destroy request stream #132
Conversation
This is another attempt to resolve the node 14 streaming bug - it's smaller scope than #129 and only addresses the issue I encountered. The hapi test suite from master passes with this change made to shot. |
The node docs say: > Implementors should not override this method, but instead implement readable._destroy() Fixes hapijs/hapi#4149
958ab92
to
c8124cf
Compare
This is not the right fix. The simple fix, that preserves as much as possible, is to just add |
I've added this to the constructor and removed the
|
Please don't remove the destroy overload for a minimal patch, as it could create extra 'close' emits (from |
I've restored it, though the docs say not to override it. |
I think we should go with something more like #133, but I'd like to keep the test here. |
I have no strong preference either way, I'd just like the bug to be squashed! |
#133 has a handful of reviews. Can you please revert the changes to |
Co-authored-by: Alex Potsides <alex@achingbrain.net>
@achingbrain thanks for the contribution and for bearing with all the back-and-forth on this one. This bug turned out to be a tricky one. |
We got there in the end, thanks for all the help. Will this be released soon? |
@achingbrain Yeah, I expect it will. Someone just has to go through the steps of releasing shot and a patch version of hapi. My guess is that it will be settled before Monday. |
Auto destroying the request stream stops
for await..of
-style iteration over request payloads working whenoutput: 'stream', parse: false
are used as request handler payload config options.Fixes hapijs/hapi#4149