-
Notifications
You must be signed in to change notification settings - Fork 129
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
Versi 6.x : Cannot destructure property 'XXX' of 'ctx.request.body' as it is undefined. #218
Comments
@zgramming can you please share:
Thx |
|
Your not passing any content-type header and no data, that means it parses nothing at all. Version 5 used to give you back an empty object which v6 is not anymore. isJson, isText, isUrlencoded, isMultipart are all falsy @MarkHerhold should we add back this behavior since we do the same if the request method does not match parsedMethods? I created a pr that changes back the empty object behavior. Or should we remove the empty object also for non matching parsedMethods that the body always gets undefined if it matches neither parsedMethods and no content-type |
The behavior for version 6.x is different, but in my opinion you should anyways do some validations on the body before destructuring and give back a http 400 if it doesn't match. |
@TheDadi IMHO I think revert back old behaviour version 5 good option, because maybe we can have some request can be optional/undefined 😀. |
Hey everyone, I recently ran into this issue, and was about halfway into fixing it on my fork when I found this issue+PR. I have extended the previous proposed solution by adding a new config option, that would default to the "new" behavior (undefined body), while allowing support for the old behavior (empty object body). Restoring the old behavior would be simpler, ofc, but since this is already live, someone might be relying on it, so having a config option adds complexity but should also keep everyone happy. Kudos to @TheDadi for the original PR, whose code I copy-pasted into my commit, and extended. Let me know your thoughts. |
@MarkHerhold Since you seem very busy at the moment, would it be possible to talk about how to get a maintainer of this repo and help you out? |
@MarkHerhold ping ☝️ |
Describe the bug
Node.js version: v18.9.1
OS version: Windows 11 Home Version 22H2
Description:
Actual behavior
Destructure property undefined throw error
Expected behavior
It's use version 5
Destructure property undefined not throw error
Code to reproduce
Checklist
The text was updated successfully, but these errors were encountered: