Rejected promises are uncaught and don't trigger on('error')

[HemalR]

Initial checklist

• I understand this is a bug report and questions should be posted in the Community Forum
• I searched issues and couldn’t find anything (or linked relevant results below)

Link to runnable example

No response

Steps to reproduce

1. Create an Uppy uploader
2. Reject a promise to cause an error
3. Reject a method to trigger a promise rejection. Instead of being caught by the on('error') hook, the client throws an unhandled promise rejection.

In my case in production, the scenario that triggered this was when writing an integration test on Cypress, when the test tried to delete a file. The delete button being ‘pressed’ before any part of the file had been uploaded led to an unhandled promise rejection (and failed the test).

const uppy = new Uppy({
	autoProceed: true,
});

uppy.use( AwsS3Multipart, {
	// ...other methods as per the docs. 
	abortMultipartUpload(_file, { key, uploadId }) {
		return new Promise((resolve, reject) => {
			const methodData = {
				bucket: 'bucket-name',
				uploadId,
				key,
			};
			// this is getting rejected by my API because uploadId is null
			abortAwsMultipartUpload.call(methodData, (err) => {
				if (err) {
					reject(err);
				} else {
					resolve();
				}
			});
		});
	},
});

// This is not catching/firing when the promise is rejected
uppy.on('error', (error) => {
	console.error('Uppy-Error ********************', error);
});

// This is also not catching/firing when the promise is rejected
uppy.on('upload-error', (file, error, response) => {
	console.log('error with file:', file.id);
	console.log('error message:', error);
	console.log('Error response', response);
});

When testing manually, everything works fine. After some debugging, I realise the integration test is failing because the ‘delete’ button is pressed before any part of the file has been uploaded, which leads to the abortMultipartUpload function being called without an uploadId (null).

This in turn is leading to an error: Uncaught (in promise) ClientError: Upload ID is required

Issues I am having:

1. Why is the rejected promise coming up as uncaught and not being passed to the on(‘error’) event? How should I be catching rejected promises?
2. What is the recommended way to handle an upload that is aborted before it is started (whereby uploadId is null)?

Thanks in advance and thanks for a great library!

Note -Boilerplate Codesandbox link shows me a ModuleNotFoundError. I'll see if I can get one up to illustrate this.

Expected behavior

Promise rejections should not be uncaught.

I may be doing something wrong in my code? If so, please let me know and I can fix it as well as contribute to the docs as to how promise rejections should be dealt with.

Actual behavior

An unhandled promise rejection is thrown on the client and the on('error') does not fire.

[Murderlon]

Hi, I think this has been brought up and fixed in #3950. From the PR:

The docs say "Cancellation cannot fail in Uppy, so the result of this
function is ignored". So let's ignore it indeed.

[HemalR]

Hi, I think this has been brought up and fixed in #3950. From the PR:

The docs say "Cancellation cannot fail in Uppy, so the result of this
function is ignored". So let's ignore it indeed.

👍

I'll close and revisit once it's out in production.

Question - Should promise rejections be getting caught by the on('error') function (regardless of whether it is caused by a cancellation or something else) or is there another way to ensure promise rejections are caught?

[Murderlon]

Question - Should promise rejections be getting caught by the on('error') function (regardless of whether it is caused by a cancellation or something else) or is there another way to ensure promise rejections are caught?

Currently the error is ignored:

uppy/packages/@uppy/aws-s3-multipart/src/MultipartUploader.js

Lines 425 to 434 in a5c73c3

	#abortUpload () {
	this.abortController.abort()
	this.createdPromise.then(() => this.options.abortMultipartUpload({
	key: this.key,
	uploadId: this.uploadId,
	})).catch(() => {
	// if the creation failed we do not need to abort
	})
	}

We could emit the error event in the catch so you can act on it, but currently we don't separate internal and external events. Meaning if we do an error emit it would set the state in Uppy as well:

uppy/packages/@uppy/core/src/Uppy.js

Lines 944 to 960 in a5c73c3

	const errorHandler = (error, file, response) => {
	let errorMsg = error.message || 'Unknown error'
	if (error.details) {
	errorMsg += ` ${error.details}`
	}
	this.setState({ error: errorMsg })
	if (file != null && file.id in this.getState().files) {
	this.setFileState(file.id, {
	error: errorMsg,
	response,
	})
	}
	}
	this.on('error', errorHandler)

I'm not sure if that's desired. Thoughts @arturi @aduh95?

[mok419]

This is causing me issues as I am having uncaught errors on page refresh which in Mozilla causes my state not to be saved. As far as I have seen, the uncaught error is only really causing state not to be saved in Mozilla, (not causing issues in safari or chrome).
and it is occasional (like 2/3 out of 5 times it might not be saved) - I think it is because on refresh, Mozilla carries the error messages / state into the new page which means my uppy instantiates with the correct state but a bunch of uncaught errors suddenly appear from before the refresh and this clears the new state and removes the new file from recovery state.

This uncaught error crashes my app as I am using react-error-boundary and it gives me multiple uncaught-error logs, and pretty sure is causing my current uppy state to be discarded!

I need react-error boundary to catch async errors and am using golden retriever to store abandoned uploads, so if an uncaught error is causing state storage issues it is a problem!

[Murderlon]

There is not enough information to help. Can you create a new issue with a reproducible example on CodeSandbox/StackBlitz?

[mok419]

Just realised the loss of recovery state isn't related to the uncaught error but is caused by the AbortController, I think, it deletes the local storage state on page reload - the error only is present on Firefox.

https://codesandbox.io/p/github/mok419/uppy-next-S3-goldenRetriever/main?file=%2Fsrc%2Fapp%2Fpage.tsx%3A5%2C10&workspaceId=eb522c19-d447-4d56-a72a-cf682bf4b633

or

https://z6x97h-3000.csb.app/

Test case:
Upload a 20mb file to start a multipart upload - we doing 5mb chunks.
Halfway through refresh the page, in Mozilla the state is lost as the abort controller runs into the next page load.
If the page is closed instead, the abort controller doesn't run fully and the recovered state is available!

This doesn't happen in chrome or safari!

[Murderlon]

Thanks, but your codesandbox is not publicly available. Can you open a new issue so we can track it? This is closed and unrelated.