Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Request pending if file limit exceed #168

Closed
vmartis opened this issue Jul 21, 2015 · 19 comments
Closed

Request pending if file limit exceed #168

vmartis opened this issue Jul 21, 2015 · 19 comments
Labels

Comments

@vmartis
Copy link

vmartis commented Jul 21, 2015

I use multer with single file variant.

var multer  = require('multer');

  var avatarMulter = multer({
    dest: './uploads/avatar',
    limits: {
      fieldNameSize: 100,
      fileSize: 1048576,
      files: 1,
      fields: 1
    }
  });

app.post('/api/photo', avatarMulter.single('file'), auth.requiresLogin, usersCustom.processAvatarImage);

Log:

Error: File too large
    at makeError (C:\Users\Viktor\Develop\OnlineKonzultace\sources\onlinekonzultace\node_modules\multer\lib\make-error.js:12:13)
    at abortWithCode (C:\Users\Viktor\Develop\OnlineKonzultace\sources\onlinekonzultace\node_modules\multer\lib\make-middleware.js:64:22)
    at FileStream.<anonymous> (C:\Users\Viktor\Develop\OnlineKonzultace\sources\onlinekonzultace\node_modules\multer\lib\make-middleware.js:116:11)
    at FileStream.emit (events.js:104:17)
    at PartStream.onData (C:\Users\Viktor\Develop\OnlineKonzultace\sources\onlinekonzultace\node_modules\multer\node_modules\busboy\lib\types\multipart.js:212:18)
    at PartStream.emit (events.js:107:17)
    at readableAddChunk (_stream_readable.js:163:16)
    at PartStream.Readable.push (_stream_readable.js:126:10)
    at Dicer._oninfo (C:\Users\Viktor\Develop\OnlineKonzultace\sources\onlinekonzultace\node_modules\multer\node_modules\busboy\node_modules\dicer\lib\Dicer.js:192:36)
    at SBMH.<anonymous> (C:\Users\Viktor\Develop\OnlineKonzultace\sources\onlinekonzultace\node_modules\multer\node_modules\busboy\node_modules\dicer\lib\Dicer.js:127:10)
    at SBMH.emit (events.js:118:17)
    at SBMH._sbmh_feed (C:\Users\Viktor\Develop\OnlineKonzultace\sources\onlinekonzultace\node_modules\multer\node_modules\busboy\node_modules\dicer\node_modules\streamsearch\lib\sbmh.js:188:10)
   at SBMH.push (C:\Users\Viktor\Develop\OnlineKonzultace\sources\onlinekonzultace\node_modules\multer\node_modules\busboy\node_modules\dicer\node_modules\streamsearch\lib\sbmh.js:56:14)
    at Dicer._write (C:\Users\Viktor\Develop\OnlineKonzultace\sources\onlinekonzultace\node_modules\multer\node_modules\busboy\node_modules\dicer\lib\Dicer.js:109:17)
    at doWrite (_stream_writable.js:301:12)
    at writeOrBuffer (_stream_writable.js:288:5)
    at Dicer.Writable.write (_stream_writable.js:217:11)
    at Multipart.write (C:\Users\Viktor\Develop\OnlineKonzultace\sources\onlinekonzultace\node_modules\multer\node_modules\busboy\lib\types\multipart.js:282:24)
    at Busboy._write (C:\Users\Viktor\Develop\OnlineKonzultace\sources\onlinekonzultace\node_modules\multer\node_modules\busboy\lib\main.js:72:16)
    at doWrite (_stream_writable.js:301:12)
    at writeOrBuffer (_stream_writable.js:288:5)
    at Busboy.Writable.write (_stream_writable.js:217:11)
POST /api/photo 500 - - 74.112 ms

But express still processing request (rrequest is still pending from browser). Also invalid file is not deleted from /upload/avatar directory.

I try use checking err directly (below), but problem is same. My processing avatar function is never run after mutler CB. I also try remove everything except mutler callback and problem is still in.

multerInstance(req, res, function (err) {
      if (err) {
        console.log(err);
        return res.status(500).json({error: 'error'});
      }
      console.log('OK');
      res.json({});
    })
@vmartis
Copy link
Author

vmartis commented Jul 22, 2015

I made a most simply example as possible

var express = require('express')
var multer  = require('multer')
var logger = require('morgan');
var upload = multer(
  { 
    dest: 'uploads/', 
    limits: {
       fields: 1,
       files: 1,
       fileSize: 512000
    }
  })

var app = express();

app.use(express.static('public'));
app.use(logger());

app.post('/avatar', upload.single('file'), function (req, res, next) {  
  res.end('finished');
})

var server = app.listen(3000, function () {
  var host = server.address().address;
  var port = server.address().port;

  console.log('Example app listening at http://%s:%s', host, port);
});

Pending problem start when i add error handler function to express:

app.use(function(err, req, res, next) {
  console.log('ERROR');
  res.status(500);
  res.end('');
  console.error(err.stack);
});

There is a HTML index.html file:

<html>
  <body>
    <form action="/avatar" method="POST" enctype="multipart/form-data">
      <input type="file" name="file" /> 
      <input type="submit" value="Send" />
    </form>
  </body>
</html>

With file which exceed defined limit, POST request pending and doesn't end. Without error handler function, request ends, with exception stack as response body.

@gabeio
Copy link
Member

gabeio commented Jul 22, 2015

where did you add the error handling function? (it matters)

@vmartis
Copy link
Author

vmartis commented Jul 22, 2015

Here is complete code:

var express = require('express')
var multer  = require('multer')
var logger = require('morgan');
var upload = multer(
  { 
    dest: 'uploads/', 
    limits: {
       fields: 1,
       files: 1,
       fileSize: 512000
    }
  })

var app = express();

app.use(express.static('public'));
app.use(logger());

app.post('/avatar', upload.single('file'), function (req, res, next) {  
  res.end('finished');
})

app.use(function(err, req, res, next) {
  console.log('ERROR');
  res.status(500);
  res.end('');
  console.error(err.stack);
});

var server = app.listen(3000, function () {
  var host = server.address().address;
  var port = server.address().port;

  console.log('Example app listening at http://%s:%s', host, port);
});

@LinusU
Copy link
Member

LinusU commented Jul 22, 2015

Hi @vmartis, sorry for taking my time to respond. I can't seem to reproduce your problem.

Could you please check out the code in #173 and run all the tests? I've left instructions on how to do that there. Thanks 👍

@LinusU LinusU added the bug label Jul 24, 2015
@kemar
Copy link

kemar commented Jul 29, 2015

Hi,

Exact same problem since upgrading to 1.0.1.

The problem occurs when uploading a file which has a size > limits.fileSize.

The error is delegated to express but the response seems to never end.

Here is a simple example:

var express = require('express');
var multer  = require('multer');
var router  = express.Router();

var multerConf = multer({
  dest: './uploads/',
  limits: {
    fileSize: 5 * 1000000
  }
});

router.post('/', multerConf.single('file'), function (req, res) {
  res.status(200).send({file: req.file});
});

router.use(function (err, req, res, next) {
  res.status(413).send('File too large');
})

module.exports = router;

I am doing something wrong?

Also, what's the best way to delete a partially uploaded file (previously it was done via onFileSizeLimit using fs.unlink('./' + file.path))

@LinusU
Copy link
Member

LinusU commented Jul 29, 2015

Partially uploaded files will now be removed automatically 🎆

Otherwise I don't think you are doing anything wrong, this seems to be a confirmed bug and I think that I have traced it to somewhere between multer and busboy. For some reason the incoming pipe won't start again after I unpipe it to busboy and try to resume it.

@Pavek
Copy link

Pavek commented Aug 6, 2015

I have similar issue with the following setup:

   var uploader = multer({
        storage: multer.diskStorage({
            destination: uploadDirFn,
            filename: uploadFileNameFn
        }),
        limits: {
            files: 1,
            fileSize: UPLOAD_LIMIT
        }
    });
    var upload = uploader.single('file');

app.post('/api/upload/file', uploadFile); // this is xhr request

function uploadFile(req, res, next) {
  // using multer as suggested here: https://github.com/expressjs/multer#error-handling
  // res is not used, so I don't pass it
  upload(req, null, function (err) {
    if (err) {
        return res.json({error: err.toString()}); // HTTP response is never sent!
    }

    // success
    res.json({result: true});
  });
}

I see this issue constantly: it doesn't matter if this is the very first upload that exceeds the limit or it comes after some successful uploads (when limit is met).

@LinusU
Copy link
Member

LinusU commented Aug 6, 2015

Should be fixed by dfa2095, please reopen if issue persists.

Finally 🎆 🍹

@LinusU LinusU closed this as completed Aug 6, 2015
@Pavek
Copy link

Pavek commented Aug 6, 2015

I confirm my issue above is fixed. Great work!

@LinusU
Copy link
Member

LinusU commented Aug 7, 2015

Thank you!

@LinusU
Copy link
Member

LinusU commented Aug 7, 2015

Turns out that this issue was probably a bug in Node.js, and I just managed to slightly work around it 😭

See nodejs/node-v0.x-archive#25823

@sumedhm-iprogrammer
Copy link

There is one work around which I have used in my current project

File: make-middleware.js
change "function done(err)" at end of this function

Replace
Line 52: onFinished(req, function () { next(err) })

With:
Line 52: onFinished(req, function () { if( typeof err != "undefined" && err.code == 'LIMIT_FILE_SIZE') { req.fileSizeError = 1; next() } else next(err) })

And in app file you can change the code to

app.post('/upload', upload.single('upload'), function (req, res, next) {
    if(typeof req.fileSizeError != "undefined") {
        res.send({"error":"File too large"});// to display filesize error
    } else {
        res.send({"file":req.file}); // when file uploaded successfully
    }
}); 

@mathiesha
Copy link

I am also getting the same error.
Using express : 4.13.4
Multer : current version 1.1.0

used @sumedhm-iprogrammer hack currently to work. But hoping this will get fixed soon

@LinusU
Copy link
Member

LinusU commented Mar 17, 2016

Which version of Node.js are you using?

@mathiesha
Copy link

Hey @LinusU
I am using 5.6.0, I hope the issue is to do with the node.js version.
Thanks

@amarkantk14
Copy link

amarkantk14 commented Sep 18, 2016

If you are getting error

Error: File too large
at makeError (/home/amar/xvb/node_modules/multer/lib/make-error.js:12:13)
at abortWithCode (/home/amar/xvb/node_modules/multer/lib/make-middleware.js:77:22)
at FileStream. (/home/amar/xvb/node_modules/multer/lib/make-middleware.js:139:11)
at emitNone (events.js:67:13)
at FileStream.emit (events.js:166:7)
at PartStream.onData (/home/amar/xvb/node_modules/busboy/lib/types/multipart.js:220:18)
at emitOne (events.js:77:13)
at PartStream.emit (events.js:169:7)
at readableAddChunk (_stream_readable.js:153:18)
at PartStream.Readable.push (_stream_readable.js:111:10)
at Dicer._oninfo (/home/amar/xvb/node_modules/dicer/lib/Dicer.js:191:36)
at SBMH. (/home/amar/xvb/node_modules/dicer/lib/Dicer.js:127:10)
at emitMany (events.js:108:13)
at SBMH.emit (events.js:182:7)
at SBMH._sbmh_feed (/home/amar/xvb/node_modules/streamsearch/lib/sbmh.js:188:10)
at SBMH.push (/home/amar/xvb/node_modules/streamsearch/lib/sbmh.js:56:14)
at Dicer._write (/home/amar/xvb/node_modules/dicer/lib/Dicer.js:109:17)
at doWrite (_stream_writable.js:300:12)
at writeOrBuffer (_stream_writable.js:286:5)
at Dicer.Writable.write (_stream_writable.js:214:11)
at Multipart.write (/home/amar/xvb/node_modules/busboy/lib/types/multipart.js:290:24)
at Busboy._write (/home/amar/xvb/node_modules/busboy/lib/main.js:74:16)

File: make-middleware.js
change "function done(err)" at end of this function

Replace
Line 52: onFinished(req, function () { next(err) })

With:
Line 52: onFinished(req, function () { if( typeof err != "undefined" && err.code == 'LIMIT_FILE_SIZE') { req.fileSizeError = 1; next() } else next(err) })

And in app file you can change the code to

app.post('/upload', upload.single('upload'), function (req, res, next) {
if(typeof req.fileSizeError != "undefined") {
res.send({"error":"File too large"});// to display filesize error
} else {
res.send({"file":req.file}); // when file uploaded successfully
}
});

@amarkantk14
Copy link

I found most appropriate way to fix above issue

var storage = multer.diskStorage({
destination: function (req, file, callback) {
var avatarMimeType = ['image/png','image/jpg','image/jpeg'];
if(file.fieldname === 'avatar' && avatarMimeType.indexOf(file.mimetype) >= 0){
callback(null, './uploads/users/profile/images/');
}else{
callback(null, './uploads');
}
},
filename: function (req, file, callback) {
callback(null, req.authUser._id + path.extname(file.originalname);
}
});

var upload = multer({ storage: storage, limits:{ fileSize: 1048576 } });

// To upload user profile images 
var userProfileImageUpload = upload.fields([{ name: 'avatar', maxCount: 1 }, { name: 'gallery', maxCount: 8 }]);

api.post('/users/profile' ,function ( req, res, next) {
userProfileImageUpload(req, res, function (err) {
if (err) {
res.status(413).send({"error":err});
}else{
res.status(200).send({"file":req.files});
}
});
});

Their is no need to chnage middleare [ multer ] file
File: make-middleware.js
change "function done(err)" at end of this function

Replace
Line 52: onFinished(req, function () { next(err) })

With:
Line 52: onFinished(req, function () { if( typeof err != "undefined" && err.code == 'LIMIT_FILE_SIZE') { req.fileSizeError = 1; next() } else next(err) })

@halfcab123
Copy link

I can confirm the above works, it worked for me, it will work for other broken limits encountered by multer as well

@ericzon
Copy link

ericzon commented Feb 13, 2018

Is there any possibility to make it work without having to modify the lib?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

10 participants