Multipart request (formData) can raise double callback

[djaibi]

Hi,

I have encountered a problem with supertest, in the case we do request with attach (multipart) it will use a formData instance and this instance will be pipped to req : https://github.com/visionmedia/superagent/blob/master/lib/node/index.js#L1017.

But if for some reason the file is not completely uploaded and we send back a response (because of an error for example). This part of the code will be reached:

https://github.com/visionmedia/superagent/blob/master/lib/node/index.js#L862

And self.callback will be called . The connection will be closed, so req will raise an error event

[Error: write ECONNRESET] code: 'ECONNRESET', errno: 'ECONNRESET', syscall: 'write' (https://github.com/visionmedia/superagent/blob/master/lib/node/index.js#L731).

In the end the callback is called a second time.

Maybe I'm doing something wrong, Is this behaviour intended ?

[mleanos]

I'm experiencing the same behavior. I've described it here, ladjs/supertest#230, and it may be related to this as well, ladjs/supertest#258

In my case, I am using .attach with a request, and this particular test checks to make sure a User is logged in, before attempting to upload the file; this properly send back a 400 response in the form of res.status(400).send(... This is what I want my API method to do. However, it does appear that since the response is sent back so quickly, the attached file isn't loaded into memory (shooting from the hip here, as I don't know much about how the attach feature works), thus causing the ECONNRESET error.

I have tested this with varying file sizes. A file size of somewhere between 44 KB & 50 KB will exhibit this behavior. A file size of 44 KB never has this issue. Thus, my conclusion is that superagent's .attach method hasn't completed what it is doing, due to the file size, before I send back the response to my test.

I hope this helps in determining the issue. I realize that I have no need to use the .attach method with this use case, and I have since removed it from the test in question. However, I think this is definitely a bug somewhere in superagent's attach method, or somewhere else down the line as it gets propagated through the assertion process.

[sherodtaylor]

This is still happening when I use attach. It calls back twice one with res defined and then another with undefined res.

    superagent
      .post(`${domain}/api/bitcoinaddresses`)
      .attach('bitcoin_addresses', files, filename)
      .end(function(err, res) {
        cb(err, res, res.body);
      });

[fractalawareness]

I get the same exact behavior as mleanos described even in the v.2.0.0

[kornelski]

What about v2.2?

[timendez]

I am still experiencing this problem in v3.0.0, notably, I am using two .attach functions

[kornelski]

Can you provide more information? The attach functions don't set up any callbacks, so there may be other options/server response headers that cause it.

[timendez]

Oh, woops, I am using supertest, not superagent. Do you still want to see my code?

[kornelski]

Only if you can reproduce the bug without supertest involved. Otherwise report the problem to supertest.