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

Feature suggestion: ability to cancel the upload process #27

Open
endymion00 opened this issue Oct 25, 2013 · 11 comments
Open

Feature suggestion: ability to cancel the upload process #27

endymion00 opened this issue Oct 25, 2013 · 11 comments

Comments

@endymion00
Copy link

Provide an event (i.e: 'cancel') in order that when emitted the upload process were canceled.

@andrewrk
Copy link
Collaborator

I'm not sure what you mean about this being an event, but I agree we need to be able to abort the upload process.

@endymion00
Copy link
Author

I meant that maybe some logic could decide the file upload should be canceled for some reason.
For example: a logic that monitorizes file size and if it is greater than "x" then needs to cancel the upload.
Let's suppose that this logic is placed inside a event listener where has access to data chunks or file info in general and it is required to be able to stop the upload.
I suppose this would be done by emitting an event that when emitted the library could stop the upload process. It is just an idea :)

@nmschulte-aviture
Copy link

I see this working a few ways:

  1. Check the return value of the 'field' and 'part' event handlers to determine whether or not to cancel parsing.
  2. Provide a method on the Form object or 'field' and 'part' event handler arguments that does the same.

Right now, I'm having to use closure and ReadableStream's resume method to continue the parsing and handle the state properly in the 'close' and 'error' events.

@meaku
Copy link

meaku commented Jan 18, 2014

I implemented something similar in my application-logic, piping to black-hole-stream in order to cancel the upload.
This seems like the only way to discard a readable stream. Correct me if i'm wrong :)

 form
        .on('part', function (fileStream) {
            //discard if wrong imageType
            if (allowedImageTypes.indexOf(req.params.imageType) === -1) {
                fileStream.pipe(new BlackHoleStream());
                return;
            }

           //save if everything worked fine
           saveFile(...)
   });

It might make extend part with a cancel method, which pipes part.pipe(New BlockHoleStream());.
This method could also be called if the part event handlers returns an error or false.

@Starefossen
Copy link

I'm interested in this as well. Currently doing the /dev/null approach:

form.on('part', function (part) {
  if (!part.filename || /\.(gif|jpe?g|png)$/i.test(part.filename)) {
    return part.pipe(fs.createWriteStream('/dev/null'));
  }
});

@dougwilson
Copy link
Contributor

@Starefossen your example shows that you want to be able to ignore certain parts. That seems like a desirable feature to me. We could also add that to the file hook so people could (if they acted sync on the event) prevent the temporary file from even being written.

@onury
Copy link

onury commented Dec 14, 2014

I think, a cancel method is very fundamental for this library. Is there any news on this? Thanks.

@jamesmorgan
Copy link

This would be a great a useful feature! +1 👍

@jamesmorgan
Copy link

@Starefossen @meaku I have been trying to cancel the streams using both ways which you suggested and this seems to not work as I expect? What I am finding is that the stream seems to timeout before any response is sent to the client. I have tried to many ways like form.emit('error), form.handlerError() and various pause, cancel methods etc. Do you know of a way to fail fast the upload process if something?

@nmschulte-aviture
Copy link

To clarify, none of the examples shown here actually cancel parsing of the request, they just toss the bits to /dev/null or such. As I described, if you actually want to bail due to some logic around a part of the form, you have to track that fact via some variable (via e.g. closure), and then in your 'close' (and 'error'!) handlers, you should check that variable first, so you can emit the proper response (that there was an error with a part, rather than a subsequent request stream error or the finishing of parsing of the form).

Further, I found that simply calling resume() on the part (the first argument of the 'part' event handler) has the same effect as piping the part to a black hole. Is that not the case? Why the suggestion of a black hole pipe?

Regardless, the black hole piping and my resume() suggestion still don't resolve the issue of being able to bail out of parsing (and respond accordingly; with an unambiguous [error] message!) the form due to an error. Rather, they just allow you to work around the issue (of not being able to cancel), by continuing form parsing while ignoring the current part's (and future parts') data. You still have to finish parsing the form and handle response generation accordingly.

What is being suggested here is a way to stop parsing of the rest of the form due to an logic error during earlier parsing of the form. There is still no supported way to do this, and the user must jump through hoops to handle these cases. I still stand by my original suggestions for supporting this.

@AleCaste
Copy link

Here is a solution that works for me.
VERY IMPORTANT: you MUST set the Connection header to close!

form.on('part', function(part) {
  if (![some condition I want to be met]) {
    form.emit('error', new Error('File upload canceled by the server.'));
    return;
  }
  ... code to handle the uploading process normally ...
});

form.on('error', function(err) {
  console.log('ERROR uploading the file:',err.message);
  res.header('Connection', 'close');
  res.status(400).send({ status:'error' });
  setTimeout(function() { res.end(); }, 500);
  next(err);
});

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

No branches or pull requests

9 participants