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

Transloadit Assemblies are being cancelled by Uppy when the component unmounts #3547

Closed
todda00 opened this issue Mar 8, 2022 · 4 comments · Fixed by #3575
Closed

Transloadit Assemblies are being cancelled by Uppy when the component unmounts #3547

todda00 opened this issue Mar 8, 2022 · 4 comments · Fixed by #3575
Labels

Comments

@todda00
Copy link

todda00 commented Mar 8, 2022

We are utilizing <DashboardModal> via @uppy/react and the Transloadit plugin via @uppy/transloadit. After a file is uploaded successfully, and the page is navigated away from causing the <DashboardModal /> to unmount, an assembly cancel request is automatically called. I can see this in the browser's network console:
Konvert

If this occurs before the file is done being processed, this causes the assembly to cancel.

Here is the code in use:

const UppyTest = ({ onUpload }) => {
  const [uppyOpen, setUppyOpen] = useState(false);
  const [authorizeMutation] = useMutation(AUTHORIZE_UPLOAD_MUTATION, {
    variables: { input: { collectionName: "Platforms", fieldName: "appearance.logo" } },
  });

  const uppy = useUppy(() =>
    new Uppy().use(Transloadit, {
      async getAssemblyOptions() {
        const { data, error } = await authorizeMutation();

        console.log("data :>> ", data);
        console.log("error :>> ", error);

        return {
          params: data?.authorizeUpload?.transloaditParams,
          fields: JSON.parse(data?.authorizeUpload?.transloaditFields),
          signature: data?.authorizeUpload?.transloaditSignature,
        };
      },
    })
  );

  return (
    <>
      <Button onClick={() => setUppyOpen(true)}>Upload File</Button>
      <DashboardModal
        uppy={uppy}
        open={uppyOpen}
        onRequestClose={() => setUppyOpen(false)}
        closeAfterFinish
      />
    </>
  );
};
@mifi
Copy link
Contributor

mifi commented Mar 15, 2022

Hi. This is working as intended because a react hook (useUppy) is designed to clean up after it's done. If you want to keep the assembly running even after the user navigates away from the Uppy dialog, we would recommend one of the following:

  1. Make sure the react component is not being unmounted when nevigating away (instead hide it)
  2. Don't use useUppy, but instead implement your own ref to keep track of the Uppy instance, and be sure to call close on it when you are done with it (e.g. when the assembly has finished processing.)

You can see that the hook is quite simple and the reason why it cancels everything is because of the unmount close() call:

uppy.current.close()

@Murderlon
Copy link
Member

Another alternative, and arguably better, is to set waitForEncoding to true and then don't let users close the modal until you have received the transloadit:complete event.

@todda00
Copy link
Author

todda00 commented Mar 15, 2022

As a newer user of Transloadit / Uppy, I have gone through the documentation for Uppy and Transloadit fairly extensively to try to establish the best in class experience for our users. We are planning on using Transloadit for both video and image / document upload in various areas of our app. Much of the Transloadit documentation steers developers towards not using waitForEncoding: true and establishing an endpoint to receive and process assembly notifications from Transloadit when the assembly is complete.

Async Mode is slightly more work to set up, but recommended as it results in:
A smoother user experience — the user does not have to wait on encoding.
Reliability — we can retry encoding as well as sending Notifications.

As far as Uppy / the uploading user is concerned, their job is done when the file is uploaded. Especially since they could be uploading large videos which may take considerable time to process, we don't want to tie them up. I understand cancelling uploads when unmounting, but sending an assembly cancel notification is unexpected, undocumented, and undesired when waitForEncoding: false.

We can re-work things to keep useUppy around, but I suspect I am not the first, or last developer to get tripped up on this situation. I feel the ergonomics of Uppy would be better if waitForEncoding:false was treated as more of a first class option, as Transloadit seems to promote the benefits of async processing.

If nothing will change, at the very least update the documentation to state that assemblies will be cancelled when useUppy is unmounted, regardless if uploads are complete.

@mifi
Copy link
Contributor

mifi commented Mar 15, 2022

Thanks for your valuable feedback. I agree that the user should not have to sit around and wait for the assembly to complete, and this default behaviour of canceling the assembly even with waitForEncoding: false is indeed confusing/counter-intuitive for a developer starting out with uppy. We will discuss a solution and get back to you shortly.

mifi added a commit that referenced this issue Mar 16, 2022
to make canceling assemblies optional
possibly fixes #3547
@transloadit transloadit deleted a comment from cmsnegar Mar 21, 2022
vymao pushed a commit to vymao/uppy that referenced this issue Mar 29, 2022
to make canceling assemblies optional
possibly fixes transloadit#3547
@mifi mifi closed this as completed in #3575 May 5, 2022
mifi added a commit that referenced this issue May 5, 2022
* change cancel logic

to make canceling assemblies optional
possibly fixes #3547

* add forgotten file

* rewrite to reason='user'

* try to fix crash

* change reason to unmount

* Apply suggestions from code review

* add close+unmount in more code examples

also fix rule of hooks issue

* improve reason docs

* add tests

* add options also to reset

* update doc

* also prevent canceling uploads on unmount

* Update website/src/docs/transloadit.md

Co-authored-by: Merlijn Vos <merlijn@soverin.net>

* remove conflicting file

Co-authored-by: Merlijn Vos <merlijn@soverin.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants