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

Future-proof hapi for node v16, rely on res close rather than req #4225

Merged
merged 1 commit into from
Mar 20, 2021

Conversation

devinivy
Copy link
Member

@devinivy devinivy commented Jan 23, 2021

There was a regression in node v15.5.0 which broke hapi. We learned that this was considered a bug in node v15, but that a comparable change would land in node v16. As such, we now rely on res rather than req to handle premature 'close' events. This small alteration should be compatible with node v12+, and is not considered a breaking change. This relies on a change within shot referenced below.

Refs: #4208 hapijs/shot#139 nodejs/node#33035

@devinivy devinivy added the feature New functionality or improvement label Jan 23, 2021
@Nargonath
Copy link
Member

Should we merge this @devinivy or is there something blocking it?

@devinivy
Copy link
Member Author

Now seems like a good time to land this. See also: nodejs/citgm#852

@devinivy devinivy added this to the 20.1.2 milestone Mar 20, 2021
@devinivy devinivy self-assigned this Mar 20, 2021
@devinivy devinivy merged commit 4639686 into master Mar 20, 2021
@devinivy devinivy deleted the node-v16-futureproofing branch March 20, 2021 15:21
@glasser
Copy link

glasser commented Apr 28, 2021

Does this mean that no versions of hapi older than v20.1.2 are compatible with Node 16?

@devinivy
Copy link
Member Author

devinivy commented Apr 28, 2021

@glasser yep! hapi v20.1.2 has now also been tested against node v16 (which was formally released last week) and is passing, so I believe we're good to go. See also #4248.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New functionality or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants