onReqAborted should only throw an error if close event has not been fired

[ryhinchey]

This PR attempts to fix the bug described in this issue #138

[dougwilson]

We probably don't want to emit the aborted event either, as the form is closed, so doesn't make sense for it to close and then abort. Would just removing the aborted event listener when closing the form work?

[ryhinchey]

We probably don't want to emit the aborted event either, as the form is closed, so doesn't make sense for it to close and then abort. Would just removing the aborted event listener when closing the form work?

That sounds like an ideal implementation but I don't have the req object here. I must be missing something

[dougwilson]

What about add a self on close remove listener somewhere in https://github.com/pillarjs/multiparty/blob/master/index.js#L149 ?

[ryhinchey]

that works great when I test locally in a browser but my test fails with that approach. thanks so much for your input here!

here's what I added:

    self.on('close', function() {
      req.removeListener('aborted', onReqAborted);
      end(function() {
        cb(null, fields, files);
      });
    });

here's my test: (which passes by not throwing an error. With the above code, an error is still thrown in the test)

  {
    name: "connection aborted after close event does not throw an error",
    fn: function(cb) {
      var clientReq;

      var server = http.createServer(function (req, res) {
        var form = new multiparty.Form();

        form.on('close', function () {
          clientReq.abort();
          cb();
        });

        form.on('part', function(part) {
          part.resume();
        });

        form.parse(req);
      }).listen(0, 'localhost', function () {
        var bigFile = path.join(FIXTURE_PATH, "file", "pf1y5.png");
        var url = 'http://localhost:' + server.address().port + '/upload';
        clientReq = superagent.post(url);
        clientReq.field('a', 'a-value');
        clientReq.field('b', 'b-value');
        clientReq.attach('myimage.png', bigFile);
        clientReq.end();
      });
    }
  },

[dougwilson]

If you log the Date.now(), is the aborted coming before the close?

[ryhinchey]

If you log the Date.now(), is the aborted coming before the close?

no

[dougwilson]

Strange. So was the event listener successfully removed? If so, how could the error still be getting created?

[ryhinchey]

The event listener is successfully removed when testing this feature in the browser. In the unit tests, it is not.

uploading a file from the browser, aborting the request, and logging in node

    form.on('close', function() {
      console.log(req.listeners('aborted')) // []
    })

in unit tests:

        form.on('close', function () {
          console.log(req.listeners('aborted')); // [ [Function: onReqAborted] ]
          clientReq.abort();
          cb();
        });