catch file stream error

codecov-io commented 9 years ago

Current coverage is `100.00%`

Merging #29 into master will not affect coverage as of c784e19

@@            master     #29   diff @@
======================================
  Files            1       1       
  Stmts           77      78     +1
  Branches         0       0       
  Methods          0       0       
======================================
+ Hit             77      78     +1
  Partial          0       0       
  Missed           0       0

Review entire Coverage Diff as of c784e19

Powered by Codecov. Updated on successful CI builds.

fengmk2 commented 9 years ago

@sabakugaara I see your issue #28 now.

Can you add a test for this change?

sabakugaara commented 9 years ago

I can't use fake pass-through stream reappear it, only can be reappear by real request. but i can use part.on('error', function(err) {}) catch it now. I think it's not a bug

sabakugaara commented 9 years ago

thanks a lot!

kevinprotoss commented 8 years ago

has this fix been published in the new version? I think it should be a bug here. If stream error occurs before user handling file event, it will always throw an exception.

kevinprotoss commented 8 years ago

However, I think the event handler should be removed if user handle it by himself.

Currently I have a case. I start to parse multipart request and save returned parts in the koa context. I will not pipe each part immediately. Before I try to use these stream in my middleware, the exception has already be thrown. I have no other chance to catch it.

haoxins commented 8 years ago

@kevinprotoss can you add some test cases for this?

kevinprotoss commented 8 years ago

I saw this error:

events.js:142
      throw er; // Unhandled 'error' event
      ^

Error: Part terminated early due to unexpected end of multipart data
    at /Invend/int_modules/body-parser/node_modules/dicer/lib/Dicer.js:65:36
    at nextTickCallbackWith0Args (node.js:433:9)
    at process._tickCallback (node.js:362:13)

I'll try to add a test case.

kevinprotoss commented 8 years ago

I found it's hard to reproduce this error in the test case, as @sabakugaara said.

I do the following thing:

In the first middleware

let parse = require('co-busboy');
// Initialize co-busboy
let body = {
  parts: parse(this)
};
// Save in the context of Koa.js
this.reqBody = body;

In a second middleware

console.log('real parsing');
let body = this.reqBody;
let parts = body.parts;
while (!parts.done()) {
  let part = yield parts;
}

The error occurs before the second middleware is called. I guess the stream has been started to pipe before next middleware is called, no matter if I use this stream immediately. If some errors are emitted during this time period, the error event will not be handled.

kevinprotoss commented 8 years ago

@coderhaoxin I created a simple gist for reproduce this case. Please have a check.

https://gist.github.com/kevinprotoss/418985fb7ecd7903569b

haoxins commented 8 years ago

@kevinprotoss I'm outing for 4 days. Will take a look ASAP. :ok_hand:

kevinprotoss commented 8 years ago

Thanks

kevinprotoss commented 8 years ago

How about this issue now?

haoxins commented 8 years ago

Oh, sorry, my fault. Will take a look these days.

haoxins commented 8 years ago

@kevinprotoss Is https://github.com/cojs/busboy/pull/31 helpful?

kevinprotoss commented 8 years ago

Thx, it looks helpful. I'll have a further test on my side.

kevinprotoss commented 8 years ago

@coderhaoxin I tested on my side. It works now. Thanks in deed again.

cojs / busboy