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

Cannot set property 'files' #25

Closed
ghost opened this issue Jan 12, 2016 · 9 comments
Closed

Cannot set property 'files' #25

ghost opened this issue Jan 12, 2016 · 9 comments
Milestone

Comments

@ghost
Copy link

ghost commented Jan 12, 2016

TypeError: Cannot set property 'files' of undefined
at handleRequest .../node_modules/koa-better-body/index.js:125:23)

It can be changed to better

returns = returns || {};
returns[filesKey] = cache.files;
@tunnckoCore
Copy link
Member

Strange. It's not on L125, but okey. Can you give the full stack and use case example?

@tunnckoCore tunnckoCore added this to the v2 milestone Jan 30, 2016
@ghost
Copy link
Author

ghost commented Feb 3, 2016

Occurs when:

  • There is no data in the request body.
  • Specified filesKey = false
  • Request multipart

@tunnckoCore
Copy link
Member

@isdenya thanks for the info. I'm working on next release locally.

@sedenardi
Copy link
Contributor

This same bug happens if you post an empty body with fieldsKey = false.
At line L115 (returns = cache.fields;) returns is set to undefined, so when you get to L125 (returns[filesKey] = cache.files;) you're assigning a field of an undefined object, and it breaks.

@tunnckoCore
Copy link
Member

@sedenardi good point.

@sedenardi
Copy link
Contributor

@tunnckoCore I think the main issue is that L87 this.request.body !== undefined fails because you're posting with an empty body. I usually use an empty body when implementing /Logout in my APIs, so I think this is a valid use case.

@tunnckoCore
Copy link
Member

Agreed. 👍

I'm trying to come to this as soon as possible, or at least to push my local things to new branch and to be able others to continue.

PRs are always welcome, i'm just too busy and i'm not using this package anywhere atm.

@sedenardi
Copy link
Contributor

@tunnckoCore Cool. I'll run my possible fix through a couple of different scenarios and make sure your tests don't break and submit a PR in the next few days.

@tunnckoCore
Copy link
Member

Yea, thanks. If you can easy fix it for current version it would be great. I'm open.

tunnckoCore added a commit that referenced this issue Apr 4, 2016
don't break on empty bodies (empty posts)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants