Race-condition when uploading multiple files

[LevelbossMike]

Hi and first of all thanks for your hard work on this addon!

We are experiencing problems with failing uploads due to an assumed race condition in the way file uploads are handled. The uploader will sometimes trigger a DOMException 12-error that is caused by the fact that we are trying to upload a file without a url despite the fact that all file objects we are using actually have a valid url. This was super annoying to debug and I wanted to get your opinion if this is really a race-condition.

Ember: 1.13.13 (yes I know 🤕 ) Plupload-version: 2.1.8 ember-plupload: 1.13.14 (but most likely more recent versions still have this problem)

We have the pluploader-component setup to allow multiple file uploads.

The way the code is structured in this addon is that for each file added via the uploader the onfileadd-action of the plupload-component will be called. Like in the README we have created an action-handler that will call file.upload.

file.upload will now set the settings on the system.file-object we called the upload-method on:

https://github.com/tim-evans/ember-plupload/blob/5646965e40d0e468aa3ad08dcfddf2c5a07ad4fd/addon/system/file.js#L152

After that has happened the addon calls the uploader#start method which will then trigger the BeforeUpload-event that will be picked up by an instance of system.upload-queue which will call its configureUpload-function:

https://github.com/tim-evans/ember-plupload/blob/5646965e40d0e468aa3ad08dcfddf2c5a07ad4fd/addon/system/upload-queue.js#L172

In the linked line the queue will update the uploader.settings and merge with the settings of the file that we have before 'found' via the pluploader-file's id (which of course works because the queue proxies the system.file-array).

What I think creates the described problem (DOMException 12) is the fact that in configureUpload we assume that the file-Object that we findBy the pluploader-file's id will have its settings already updated. We are not setting the system.file-Object's settings via a run-loop aware method though and imo this creates a race-condition where sometimes the settings will be present on the file but sometimes will not.

I assume this is either fixable by bumping the version of plupload where plupload-files have the upload-method available already and don't rely on this somewhat complicated internal event handling by plupload.js. Or we might execute the uploader.start method on the next run-loop tick.

Does this sound reasonable? I'll try to implement the latter option and let you know what comes out of this. Just wanted to give you a quick heads up that there might be a race condition in the library.

[tim-evans]

Could you provide some pseudocode for your file.upload hook?

[LevelbossMike]

The way we are using the addon (I didn't include all options we pass to pl-uploader)

<div class="component-that-wraps-pl-uploader">
  {{#pl-uploader multiple=true onfileadd="uploadImage" as |queue dropzone|}}
    {{yield queue dropzone}}
  {{/pl-uploader}}
</div>

// custom component
export default Ember.Component.extend({
  // ...
  actions: {
    uploadImage(file) {
      return somePromiseChain
        .then(function(modelThatContainsUploadUrl) {
          const {url, opts} = modelThatContainsUploadUrl.getProperties('url', 'opts');
          return file.upload(url, {data: opts});
        });
    }
  }
});

It's a little bit more complicated that that but I hope you get the idea.

[tim-evans]

This should work, so it looks like you found a bug 😢

[gustaff-weldon]

@tim-evans With regards to this issue and related pull request.

We have a very similar setup to @LevelBossMike when it comes to onfileadd and manually calling file.upload(url, options).

The difference is that we want to start uploading the file via file.upload but only that file, not the others until file.upload is called on them.

Now it is impossible to upload even 1 file until all files user selected have been set up via .upload() call. The uploader is waiting for all to be configured.

The use case we have is that we want to limit the number of uploaded files until a specific condition on a server is met. Say, a user selects 20 files, we upload 5, wait for the server to finish processing them, then send another 5 files, wait.. etc.

With previous version (We were using 1.3.17), an attempt to do this was causing the DOMException 12, since after 5 files were uploaded, 6th was starting to upload and failing on missing url.

With version 1.3.19, we configure 5 files to upload and none of them starts uploading.

What I would like to do, is when a specific option flag is set, require calling file.upload explicitly on file to upload it and prevent any .next() calls in uploader logic.

Before I start hacking my solution, I'm wondering if there will be anything on plupload.js side that would prevent us from implementing this?

[gustaff-weldon]

I have done some digging and have a working pause/resume solution that relies on plupload api. uploader.start/stop to the rescue. I will give a go a supposedly more proper onBeforeUpload/trigger('UploadFile') solution PR might follow.