Multer: Race condition in test

[LinusU]

This seems to be very very rare, found it on Travis today.

$ npm test

> [email protected] test /home/travis/build/expressjs/multer
> standard && mocha



  Disk Storage
    ✓ should process parser/form-data POST request
    1) should process empty fields and an empty file
    ✓ should process multiple files
    ✓ should remove uploaded files on error
    ✓ should report error when directory doesn't exist

  Error Handling
    ✓ should respect parts limit
    ✓ should respect file size limit
    ✓ should respect file count limit
    ✓ should respect file key limit
    ✓ should respect field key limit
    ✓ should respect field value limit
    ✓ should respect field count limit
    ✓ should respect fields given
    ✓ should report errors from storage engines
    ✓ should report errors from busboy constructor
    ✓ should report errors from busboy parsing

  Expected files
    ✓ should reject single unexpected file
    ✓ should reject array of multiple files
    ✓ should reject overflowing arrays
    ✓ should accept files with expected fieldname
    ✓ should reject files with unexpected fieldname

  Express Integration
    ✓ should work with express error handling (42ms)
    ✓ should work when receiving error from fileFilter

  Fields
    ✓ should process multiple fields
    ✓ should process empty fields
    ✓ should not process non-multipart POST request
    ✓ should not process non-multipart GET request
    ✓ should handle basic keys
    ✓ should handle multiple values
    ✓ should handle deeper structure
    ✓ should handle sparse arrays
    ✓ should handle even deeper
    ✓ should handle such deep
    ✓ should handle merge behaviour
    ✓ should handle bad fields
    ✓ should convert arrays into objects

  File Filter
    ✓ should skip some files
    ✓ should report errors from fileFilter

  File ordering
    ✓ should present files in same order as they came

  Functionality
    ✓ should upload the file to the `dest` dir
    ✓ should rename the uploaded file
    ✓ should ensure all req.files values (single-file per field) point to an array
    ✓ should ensure all req.files values (multi-files per field) point to an array
    ✓ should rename the destination directory to a different directory

  Issue #232
    ✓ should report limit errors

  Memory Storage
    ✓ should process multipart/form-data POST request
    ✓ should process empty fields and an empty file
    ✓ should process multiple files

  Reuse Middleware
    ✓ should accept multiple requests (48ms)

  Select Field
    ✓ should select the first file with fieldname
    ✓ should select all files with fieldname

  Unicode
    ✓ should handle unicode filenames


  51 passing (412ms)
  1 failing

  1) Disk Storage should process empty fields and an empty file:
     Uncaught Error: ENOENT: no such file or directory, stat '/tmp/gvQUuo2Ne54/154080e1a19a860bba7db8c648f4358f'
      at Error (native)
      at Object.fs.statSync (fs.js:849:18)
      at Object.fileSize (test/_util.js:11:13)
      at test/disk-storage.js:81:25
      at Immediate.<anonymous> (test/_util.js:32:37)



npm ERR! Test failed.  See above for more details.

The command "npm test" exited with 1.

Done. Your build exited with 1.

[gireeshpunathil]

@LinusU - are we seeing this still? (just trying to figure out whether we should keep it opened or not)

[Trott]

@LinusU - are we seeing this still? (just trying to figure out whether we should keep it opened or not)

@gireeshpunathil Definitely still seeing it. I've noticed it a fair amount in Node.js's ecosystem-testing utility CITGM. For example: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/2040/nodes=ubuntu1604-64/testReport/junit/(root)/citgm/multer_v1_4_2/

I also just reproduced it locally on macOS Mojave by adding mocha's .only() to the relevant test. In other words, I changed:
https://github.com/expressjs/multer/blob/59376904cf2317b3683368c8cbe3736356ffacd2/test/disk-storage.js#L51

...to be:

  it.only('should process empty fields and an empty file', function (done) {

...and then ran

$ while true; do
> node node_modules/.bin/mocha
>   if [[ "$?" -ne 0 ]]; then 
> break
> fi
> done

...and the failure seems to come up every twenty or thirty runs or so:

  1) Disk Storage should process empty fields and an empty file:
     Uncaught Error: ENOENT: no such file or directory, stat '/var/folders/7w/pzd4440x60d3gkc3cv7rj8lc0000gp/T/2MR49F8/a25a4ece9ac48e4fc21b525cd7edadd2'
      at Object.statSync (fs.js:926:3)
      at Object.fileSize (test/_util.js:11:13)
      at test/disk-storage.js:80:25
      at Immediate.<anonymous> (test/_util.js:32:37)
      at processImmediate (internal/timers.js:441:21)

[Trott]

Changing:

https://github.com/expressjs/multer/blob/59376904cf2317b3683368c8cbe3736356ffacd2/storage/disk.js#L42

...to:

      outStream.on('close', function () {

...makes it so I can't reproduce the test failure anymore.

[Trott]

Propose PR to fix this: https://github.com/expressjs/multer/pull/787

[ronag]

Probably bug in Node. Proposed PR in core https://github.com/nodejs/node/pull/29930.

[jonchurch]

The referenced fix in Node core has landed, this should be tested again and determined if it's fixed for multer.

[Trott]

The referenced fix in Node core has landed, this should be tested again and determined if it's fixed for multer.

I haven't noticed the CITGM failure mentioned in https://github.com/expressjs/multer/issues/238#issuecomment-540915547 in a long time so I'd guess that any Node.js part of this is fixed on the master branch at least.

[gireeshpunathil]

so closing as resolved; we can always re-open if necessary.