-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Remove return value from generateResponse() prepare method #4299
Comments
While this is definitely a breaking change (the docs will have to change), I feel that it would be fair to remove such a broken feature on the current release. |
I fully agree that this is broken, but I do have some concerns about removing it in v20. If someone is using the functionality, updating this and changing their responses could have worse effects for them than the bugs mentioned above. And if nobody is using it, then this issue doesn't affect anyone. I would be all for updating the docs now to discourage the behavior, then updating the code in v21. I also would be open to handling the case of no return value in v20 by assuming the response hasn't been changed. |
I guess you are right. My main concern is that someone might add new code that tries to depend on this. But that is probably quite unlikely. |
Resolved with v21 #4386 |
Support plan
Context
What are you trying to achieve or the steps to reproduce?
The request.generateResponse() takes a
prepare(response)
option that must return a response (existing or new).hapi/lib/toolkit.js
Lines 104 to 106 in 404d253
hapi/lib/response.js
Line 510 in 404d253
Returning anything other than the existing response is however fundamentally broken since:
close()
option to never be called.request.generateResponse()
is available, and if used can break since theprepare()
option is never called for it.Given the above, I expect the feature has never been used and I suggest it is simply removed. Especially since there is no test code that covers this, or any pertinent use cases. Any return value should just be ignored. The feature is likely a vestige from when returned errors would be handled.
FYI the
generateResponse()
prepare
option is currently only used in inert and vision.The text was updated successfully, but these errors were encountered: