Throws an unlistenable error

[formula1]

https://github.com/andrewrk/node-multiparty/blob/master/index.js#L731

This is associated to situations when a multi-part form is artificially created by means of form-data however no "transfer-encoding: chunked" is provided in the request headers. This will lead to the post data being improperly handled, however more importantly should throw an error. Multiparty does throw an error however its uncatchable. I'm quite confident I'm watching all error events possible. If not, let me know.

This is especially bad as it can turn into an exploit unless the there is a global listener for errors. Though there is little to no context associated if it is listened for globally.

form-data/form-data#108

[dougwilson]

Hi! There seems to be a lot of link following and reading to do. Would you be willing, really quick, just to post the err.stack of the uncatchable error?

[formula1]

Absolutely.

Error: stream ended unexpectedly
    at Form.<anonymous> (/home/sam/Programming/client/codesmith/BigBrother/issues/superagent-multiparty/node_modules/multiparty/index.js:731:24)
    at Form.emit (events.js:129:20)
    at finishMaybe (_stream_writable.js:484:14)
    at afterWrite (_stream_writable.js:362:3)
    at _stream_writable.js:349:9
    at process._tickCallback (node.js:355:11)

btw thanks for the quick response ^__^

[dougwilson]

Thank you! And hopefully the last question I wasn't able to figure out yet from the links: what version of Node.js are you getting this on?

[formula1]

v0.12.2 also no good on v0.10.38

[dougwilson]

So your gist https://gist.github.com/formula1/582b2acb8c1e6513fe1a seems to be the easiest way for me to reproduce (thanks!), but what are the instructions to run it?

[dougwilson]

Actually, I think I figured it out; seems like it was not compatible with Windows out of the box so I had to make some changes.

[formula1]

I apologize, I was running it on ubuntu and was trying to write things kinda quickly : /

[dougwilson]

There is nothing un-catchable here; you're catching it in your own code. If there were an unhandled error, the server.js app would have actually closed down.

The something in the request module is closing the TCP socket, not multiparty, which is why you get the ECONNRESET on the client and why the server thinks the request ended early. This is also why you are seeing ECONNRESET across the board between modules with different code bases (you may also want to test the pez parser).

[dougwilson]

Well, let me re-open until I can tell you where the issue is.

[dougwilson]

Sorry, seems I'm going in some circles as well, lol. Please hang on (:

[formula1]

server.js does close down for me for both superagent-bad and request-bad : /

Oh wow, didn't realize walmart labs has their own parser. Will definitely check that out after I'm done with work today

[dougwilson]

Yea, I saw that as well. For me it does it sporadically and didn't experience that before my original post and after making lots of them, it closed down eventually for me.

[dougwilson]

I have figured it out: there is an issue in multiparty 💃 lol. It's that we can emit the error event on the part object prior to passing it to your code to attach an error handler to, lol.

[formula1]

Ah! sounds like a job for process.nextTick

For what its worth, Throwing an error is definitely better than busboys situation. When things go silently, I get really worried...

[dougwilson]

Haha. Don't worry, I'm working on a fix for this right now :)

[dougwilson]

Ok, can you tell me if the new master branch of this repo crashes the server any longer for you?

[formula1]

I'm definitely recieving an emitted error! But I'm still recieving a global error. Is it possible its being thrown twice?

I believe I'm pulling from the right repo.

Also updated the logging so we know who's error is whos

[dougwilson]

But I'm still recieving a global error.

What does this mean? Your program is crashing? If it's not, what does "global error" mean? Remember, according to our docs, errors are always emitted on the form, even if they were also emitted on a part.

[formula1]

global error comes from https://gist.github.com/formula1/582b2acb8c1e6513fe1a#file-server-js-L98

It could be because the socket hangs up the req emits an error after, are you unpiping after your done? Perhaps I should unpipe

This might be a two for one!

[dougwilson]

I'm not sure what you mean. I'm not getting that anymore, though. It's possible that you're not actually using the new code. Can you confirm the file contents of our index.js file compared to what you have installed? If they are not the same, do rm -rf node_modules && npm cache clean && npm install to try again?

[dougwilson]

With the updated multiparty, I've made 2,000 requests so far and nothing hitting your new "GLOBAL" logging. Prior to the fix, I typically only had to make 4-8 requests before it hit.

[formula1]

removed node_modules, cleaned cache and installed clean still an error

only listening to form, still an error

hm... should I throw it?

I'm basing all of my issue of the gist, If you think anything should be changed there let me know. I think you can edit it also

[dougwilson]

Ok. And, did you confirm the index.js contents in your node_modules/multiparty folder are identical to what is in this repo? I'm going to re-open this issue, but I cannot reproduce any longer, so I'm at a loss.

[formula1]

What operating system are you on?
Once I install my new harddrive I might as well run it in a virtual machine

[dougwilson]

I only have Windows 7 x64.

[dougwilson]

Wait a minute, why did you remove the part.on('error' ...) from https://gist.github.com/formula1/582b2acb8c1e6513fe1a#file-parter-multiparty-js-L1 ? I didn't fully update since I have Windows modifications. You have to listen on that error.

[dougwilson]

From the readme:

part emits 'error' events! Make sure you handle them.

If you don't have the error listener on the part, of course you would be getting GLOBAL errors still...

[dougwilson]

I'm going to close this issue since I can reproduce the issue with your latest code, and the reason is because you remove the part error handling, which is required. If you add it back, it doesn't crash any longer, since you're handling the error now.

It seems that when I first pulled, you didn't have pushed up the commit that removed the error handling. I had pulled again and then there was a new commit after the GLOBAL commit that removed error handling, letting me error out again, but you cannot actually remove that error handling, of course.

[formula1]

I was about to close it!
I'm in the car but yes, no global rrrors

[dougwilson]

yay! So @formula1 I'm going to release this as a patch release tonight unless you realize there is some issue :) I really appreciate the bug report! This was definitely a "nasty" one to figure out, test, and confirm :D

[formula1]

No issue, i am glad everything was sorted out so quickly!

[dougwilson]

@formula1 this fix is published in 4.1.2

[dtytomi]

@formula1 am still having Error: stream ended unexpectedly and the multiparty is 4.1.2, but I would like you to please look through my code to checkout what am doing if it's right

var uuid = require('node-uuid'),
multiparty = require('multiparty');

exports.create = function(req, res){

console.log(req.body, req.files);

//Parse Form
var form = new multiparty.Form();

form.on('error', function(err) {
console.log('Error parsing form: ' + err.stack);
});

form.parse(req, function(err, fields, files){
if (err) {
res.status(500).send({
message: err
});
}

if (files.file) {

    //if there is a file do upload
    var file = files.file[0];
    var contentType = file.headers['content-type'];
    var tmpPath = file.path;
    var extIndex = tmpPath.lastIndexOf('.');
    var extension = (extIndex < 0) ? '' : tmpPath.substr(extIndex);

    // uuid is for generating unique filenames. 
    var fileName = uuid.v4() + extension;
        // destPath = 'public/modules/core/img/server/Temp/' + fileName;
    uploadImage(tmpPath, contentType, fileName);
}

});
};