balderdashy / sails

Realtime MVC Framework for Node.js
https://sailsjs.com
MIT License
22.82k stars 1.95k forks source link

Empty upload fields #4864

Closed sixpounder closed 5 years ago

sixpounder commented 9 years ago

If e.g. a form is uploaded with an empty input field skipper fails with

Error: ETIMEOUT: An Upstream (undefined) timed out waiting for file(s). No files were sent after waiting 500ms.

even if .upload() method is not called at all. It is enough to call req.file('fileId') somewhere in the code (for example, to check if an optional file field has files in it)

Shouldn't it be fail safe when an input file field is present but empty? (req.file('something')._files.length == 0)

mikermcneil commented 9 years ago

even if .upload() method is not called at all. It is enough to call req.file('fileId') somewhere in the code (for example, to check if an optional file field has files in it)

Completely agree w/ ya- that would be neat/cleaner. To clarify though-- all .upload() really does is start listening for file streams coming out of the specified field (i.e. pipes the Upstream returned by req.file() to the specified receiver, or skipper-disk by default)

So to pull that off we'd need to monitor when something tugs on the Upstream (i.e. it calls .read() on it). That's the main reason I didn't go w/ this approach initially -- and I wanted to avoid breaking anything because the streams, man... the streams...

Error: ETIMEOUT: An Upstream (undefined) timed out waiting for file(s). No files were sent after waiting 500ms.

See https://github.com/balderdashy/skipper/issues/24#issuecomment-50533597 - let me know if you have thoughts (or time to create a quick test which deterministically triggers this error)

uncletammy commented 9 years ago

@sixpounder

Are you still having this issue? I can't seem to replicate it. Can you post your sails and skipper versions along with a stripped down version of the problem code? Thanks.

pavlo commented 9 years ago

Hi guys, I am having this issue today as well with a form where a file upload field is optional. What pattern would you recommend to handle empty upload file fields? A boolean-like hidden field comes to mind but it looks silly as for me. How do you usually handle this?

I am okay to put together a quick app for you guys to demonstrate the issue, just let me know.

Pavlo

kvermeille commented 9 years ago

I'm having the same issue when Im submitting a form with an image and the user decides to leave the image empty

ReyesMagos commented 9 years ago

I Have the same isuee how could we solve it ?

mikermcneil commented 9 years ago

Hey guys because of how this works, you need to handle the .upload() callback even for optional uploads. I'll write up an example

radkodinev commented 9 years ago

@mikermcneil An example would be great, thanks in advance :)

mikermcneil commented 9 years ago

@radkodinev @ReyesMagos @kvermeille @pavlo @sixpounder guys I think this is finally solved in all (or at least most) cases. I tested extensively over the past few days.

In 0.5.5 i increased default ms before ETIMEOUT to 10 seconds (and also fixed an issue that was causing hanging issues when content-type errors from multiparty that occurred BEFORE .upload() was called on one or more upstreams were received)

mikermcneil commented 9 years ago

@radkodinev re example, try sending only text params (no files) to POST /file/upload in this sample app https://github.com/sails101/file-uploads/blob/master/api/controllers/FileController.js#L36

radkodinev commented 9 years ago

@mikermcneil Thanks. Version 0.5.5 works great in my application.

sixpounder commented 9 years ago

@mikermcneil works for me too. Thanks!

gregbarcza commented 9 years ago

Hello guys i've still got this error when file parameter is missing my code: https://gist.github.com/moglash/30723a81d10c873f21cb

Error: EMAXBUFFER: An Upstream (`NOOP_file`) timed out before it was plugged into a receiver. It was still unused after waiting 4500ms. You can configure this timeout by changing the `maxTimeToBuffer` option.
    at null.<anonymous> (/home/moglash/webz/berry/berry-server-dev/node_modules/sails/node_modules/skipper/standalone/Upstream/Upstream.js:86:15)
    at Timer.listOnTimeout [as ontimeout] (timers.js:112:15)
22 Feb 15:14:24 - [nodemon] app crashed - waiting for file changes before starting...
kvermeille commented 9 years ago

@mikermcneil excellent!!!

amberv0 commented 9 years ago

Yep, still happens in 0.5.5. But, in fact, when upload() is called, everything is ok. But when trying to do anything with req.file('') error occurs. Or if to do something with req.file() first, and then call upload(), error also occurs. I believe this is the reason for the error in the gist above. my sails is 0.11.0

Yet, what is weird in my case, the file should have been uploaded (since it was chosen in the form), but it doesn't seem to be. Figuring out why

amberv0 commented 9 years ago

UPD FOUND THE REASON: In my form I didn't put enctype="multipart/form-data" The problem is reproducible with https://github.com/sails101/file-uploads if to remove the enctype.

lentiummmx commented 9 years ago

It happens again, for me....

Error: EMAXBUFFER: An Upstream (NOOP_kuponFile) timed out before it was plugged into a receiver. It was still unused after waiting 4500ms. You can configure this timeout by changing the maxTimeToBuffer option. at null. (/home/phoenix/Repositories/git/miskupones/node_modules/sails/node_modules/skipper/standalone/Upstream/Upstream.js:86:15) at Timer.listOnTimeout (timers.js:110:15)

package,json

{ "name": "skipper", "version": "0.5.5", "description": "Bodyparser for Express/Sails. Exposes simple API for streaming multiple files to disk, S3, etc. without buffering to a .tmp directory.", "main": "index.js", "directories": {}, "scripts": { "test": "mocha && node -e \"require('skipper-adapter-tests')({module: require('skipper-disk')});\"" },

ArVan commented 8 years ago

Happens to me also. Skipper version: "0.5.5".

Error: EMAXBUFFER: An Upstream (`NOOP_audio`) timed out before it was plugged into a receiver. It was still unused after waiting 4500ms. You can configure this timeout by changing the `maxTimeToBuffer` option.
wfpaisa commented 8 years ago

Hi, I created a test error: https://github.com/wfpaisa/SailsJS-EMAXBUFFER

jamilnyc commented 8 years ago

I get this error if I upload no files in the form with sails 0.12.3 skipper-s3 0.5.6

How can I get it to ignore the input if the user doesn't upload anything? I tried uploading anyway (without any files), but that still causes the same error and crashes the app.

sebastialonso commented 8 years ago

I'm still having this bug, same situation as the rest, if file doesn't come, Error: EMAXBUFFER: An Upstream (NOOP_icon) timed out before it was plugged into a receiver. triggers.

Can't the req.file('NAME') just be null or undefined when no file is uploaded? It seems awful that the whole object it's still there when no file whatsoever is uploaded.

idoivri commented 8 years ago

I'm also encountering this issue. Please do something with it... I can't seem to be able to bypass the crash even when upload() is not being called.

carloslfu commented 7 years ago

I'm solved the problem of use req.file('something')._files.length using req._fileparser.upstreams.length instead.

bilal68 commented 5 years ago

where do i need to put this check..I am uploading from react and backend is sails...mgetting the same error in machine-as-action.js

usckuro commented 5 years ago

What I did is the same as @carloslfu but adding a for

//loop in upstream for( var x = 0, lg = req._fileparser.upstream.length; x < lg; x++){

            //validate _id is equal to the upstream field name
            if(req._fileparser.upstream[x].fieldName == fieldId){

              //after all valdiations get the file and upload it
              let url = await sails.helpers.uploadFile.with({folder: 'checklist', fileToUpload: req.file(fieldId);});

              //replace the value of the field
              body.sections[i].fields[e].value = url;

              break;
            }
          }

Maybe is useful for someone else, but we need a better approach

HKFiliatix commented 4 years ago

req._fileparser.upstream - undefined req._fileparser.upstreams - array req._fileparser - optional so for API I use: if (req._fileparser && req._fileparser.upstreams.length)