From d96af4a62d5fc7fe49599710d3791e3cefd24c00 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20Unneb=C3=A4ck?= Date: Sat, 15 Aug 2015 01:20:37 +0200 Subject: [PATCH] lib: work around node.js bug --- lib/make-middleware.js | 10 ++++- package.json | 1 + test/_util.js | 8 +++- test/express-integration.js | 90 +++++++++++++++++++++++++++---------- 4 files changed, 83 insertions(+), 26 deletions(-) diff --git a/lib/make-middleware.js b/lib/make-middleware.js index 8ac9b55e..7c2a9d8f 100644 --- a/lib/make-middleware.js +++ b/lib/make-middleware.js @@ -1,6 +1,7 @@ var is = require('type-is') var Busboy = require('busboy') var extend = require('xtend') +var onFinished = require('on-finished') var appendField = require('append-field') var Counter = require('./counter') @@ -8,6 +9,10 @@ var makeError = require('./make-error') var FileAppender = require('./file-appender') var removeUploadedFiles = require('./remove-uploaded-files') +function drainStream (stream) { + stream.on('readable', stream.read.bind(stream)) +} + function makeMiddleware (setup) { return function multerMiddleware (req, res, next) { if (!is(req, ['multipart'])) return next() @@ -38,9 +43,12 @@ function makeMiddleware (setup) { function done (err) { if (isDone) return isDone = true + req.unpipe(busboy) + drainStream(req) busboy.removeAllListeners() - next(err) + + onFinished(req, function () { next(err) }) } function indicateDone () { diff --git a/package.json b/package.json index 06c6dbe0..77a36ae5 100644 --- a/package.json +++ b/package.json @@ -24,6 +24,7 @@ "concat-stream": "^1.5.0", "mkdirp": "^0.5.1", "object-assign": "^3.0.0", + "on-finished": "^2.3.0", "type-is": "^1.6.4", "xtend": "^4.0.0" }, diff --git a/test/_util.js b/test/_util.js index a6782ad8..e62bdafc 100644 --- a/test/_util.js +++ b/test/_util.js @@ -1,6 +1,7 @@ var fs = require('fs') var path = require('path') var stream = require('stream') +var onFinished = require('on-finished') exports.file = function file (name) { return fs.createReadStream(path.join(__dirname, 'files', name)) @@ -16,6 +17,11 @@ exports.submitForm = function submitForm (multer, form, cb) { var req = new stream.PassThrough() + req.complete = false + form.once('end', function () { + req.complete = true + }) + form.pipe(req) req.headers = { 'content-type': 'multipart/form-data; boundary=' + form.getBoundary(), @@ -23,7 +29,7 @@ exports.submitForm = function submitForm (multer, form, cb) { } multer(req, null, function (err) { - cb(err, req) + onFinished(req, function () { cb(err, req) }) }) }) } diff --git a/test/express-integration.js b/test/express-integration.js index 10ffb53c..b86429df 100644 --- a/test/express-integration.js +++ b/test/express-integration.js @@ -8,14 +8,34 @@ var util = require('./_util') var express = require('express') var FormData = require('form-data') var concat = require('concat-stream') +var onFinished = require('on-finished') var port = 34279 describe('Express Integration', function () { + var app + + before(function (done) { + app = express() + app.listen(port, done) + }) + + function submitForm (form, path, cb) { + var req = form.submit('http://localhost:' + port + path) + + req.on('error', cb) + req.on('response', function (res) { + res.on('error', cb) + res.pipe(concat({ encoding: 'buffer' }, function (body) { + onFinished(req, function () { cb(null, res, body) }) + })) + }) + } + it('should work with express error handling', function (done) { - var app = express() var limits = { fileSize: 200 } var upload = multer({ limits: limits }) + var router = new express.Router() var form = new FormData() var routeCalled = 0 @@ -23,45 +43,67 @@ describe('Express Integration', function () { form.append('avatar', util.file('large.jpg')) - app.post('/profile', upload.single('avatar'), function (req, res, next) { + router.post('/profile', upload.single('avatar'), function (req, res, next) { routeCalled++ res.status(200).end('SUCCESS') }) - app.use(function (err, req, res, next) { + router.use(function (err, req, res, next) { assert.equal(err.code, 'LIMIT_FILE_SIZE') errorCalled++ res.status(500).end('ERROR') }) - app.listen(port, function () { - var res, body - var req = form.submit('http://localhost:' + port + '/profile') - var pending = 2 + app.use('/t1', router) + submitForm(form, '/t1/profile', function (err, res, body) { + assert.ifError(err) + + assert.equal(routeCalled, 0) + assert.equal(errorCalled, 1) + assert.equal(body.toString(), 'ERROR') + assert.equal(res.statusCode, 500) + + done() + }) + }) + + it('should work when receiving error from fileFilter', function (done) { + function fileFilter (req, file, cb) { + cb(new Error('TEST')) + } + + var upload = multer({ fileFilter: fileFilter }) + var router = new express.Router() + var form = new FormData() + + var routeCalled = 0 + var errorCalled = 0 + + form.append('avatar', util.file('large.jpg')) - function validate () { - assert.equal(routeCalled, 0) - assert.equal(errorCalled, 1) - assert.equal(body.toString(), 'ERROR') - assert.equal(res.statusCode, 500) + router.post('/profile', upload.single('avatar'), function (req, res, next) { + routeCalled++ + res.status(200).end('SUCCESS') + }) - done() - } + router.use(function (err, req, res, next) { + assert.equal(err.message, 'TEST') - req.on('response', function (_res) { - res = _res + errorCalled++ + res.status(500).end('ERROR') + }) - res.pipe(concat({ encoding: 'buffer' }, function (_body) { - body = _body + app.use('/t2', router) + submitForm(form, '/t2/profile', function (err, res, body) { + assert.ifError(err) - if (--pending === 0) validate() - })) - }) + assert.equal(routeCalled, 0) + assert.equal(errorCalled, 1) + assert.equal(body.toString(), 'ERROR') + assert.equal(res.statusCode, 500) - req.on('finish', function () { - if (--pending === 0) validate() - }) + done() }) }) })