diff --git a/lib/node/index.js b/lib/node/index.js index 2f948a911..921382793 100644 --- a/lib/node/index.js +++ b/lib/node/index.js @@ -264,7 +264,13 @@ Request.prototype._getFormData = function() { if (!this._formData) { this._formData = new FormData(); this._formData.on('error', err => { - this.emit('error', err); + debug('FormData error', err); + if (this.called) { + // The request has already finished and the callback was called. + // Silently ignore the error. + return; + } + this.callback(err); this.abort(); }); } diff --git a/lib/request-base.js b/lib/request-base.js index 81e06b0b1..e4346bb94 100644 --- a/lib/request-base.js +++ b/lib/request-base.js @@ -242,7 +242,6 @@ RequestBase.prototype.then = function then(resolve, reject) { console.warn("Warning: superagent request was sent twice, because both .end() and .then() were called. Never call .end() if you use promises"); } this._fullfilledPromise = new Promise((innerResolve, innerReject) => { - self.on('error', innerReject); self.on('abort', () => { const err = new Error('Aborted'); err.code = "ABORTED"; diff --git a/test/node/multipart.js b/test/node/multipart.js index 6c0640a3d..e671dd4b1 100644 --- a/test/node/multipart.js +++ b/test/node/multipart.js @@ -73,31 +73,49 @@ describe("Multipart", () => { }); describe("when a file does not exist", () => { - it("should emit an error", done => { + it("should fail the request with an error", done => { const req = request.post(`${base}/echo`); req.attach("name", "foo"); req.attach("name2", "bar"); req.attach("name3", "baz"); - req.on("error", err => { + req.end((err, res) => { + assert.ok(!!err, "Request should have failed."); + err.code.should.equal("ENOENT"); err.message.should.containEql("ENOENT"); err.path.should.equal("foo"); done(); }); - - req.end((err, res) => { - if (err) return done(err); - assert(0, "end() was called"); - }); }); it("promise should fail", () => { - request.post('nevermind') - .field({a:1,b:2}) - .attach('c', 'does-not-exist.txt') - .then(() => assert.fail("It should not allow this")) - .catch(() => true); + return request.post(`${base}/echo`) + .field({a:1,b:2}) + .attach('c', 'does-not-exist.txt') + .then( + res => assert.fail("It should not allow this"), + err => { + err.code.should.equal("ENOENT"); + err.path.should.equal("does-not-exist.txt"); + }); + }); + + it("should report ECONNREFUSED via the callback", done => { + request.post('http://127.0.0.1:1') // nobody is listening there + .attach("name", "file-does-not-exist") + .end((err, res) => { + assert.ok(!!err, "Request should have failed"); + err.code.should.equal("ECONNREFUSED"); + done(); + }); + }); + it("should report ECONNREFUSED via Promise", () => { + return request.post('http://127.0.0.1:1') // nobody is listening there + .attach("name", "file-does-not-exist") + .then( + res => assert.fail("Request should have failed"), + err => err.code.should.equal("ECONNREFUSED")); }); }); }); @@ -143,13 +161,11 @@ describe("Multipart", () => { request .post(`${base}/echo`) .attach("filedata", "test/node/fixtures/non-existent-file.ext") - .on("error", err => { + .end((err, res) => { + assert.ok(!!err, "Request should have failed."); err.code.should.equal("ENOENT"); err.path.should.equal("test/node/fixtures/non-existent-file.ext"); done(); - }) - .end((err, res) => { - done(new Error("Request should have been aborted earlier!")); }); }); });