Fix handling of HTTP 500 errors while uploading images

[azaozz]

The main reason an image upload would fail is the server running out of resources while post-processing (creating sub-sizes). In WP 5.3 the resizing of images after upload was updated/fixed and it is now possible to retry creating the sub-sizes, even after an upload request ends with a server "crash" (timeout or out-of-memory fatal error).

On the client side an unique "upload ID" is sent with each image upload request. Then if the original request ends with HTTP 500 error, the client does another request including the same upload ID so the server (PHP) can try to create any missing image sub-sizes. This can be repeated several times.

Then, if all attempts fail, the client sends a "cleanup" request so the server can delete any orphaned sub-sizes, etc. The client should also show appropriate error message, something like:

Post-processing of the image failed. If this is a photo or a large image, please scale it down to 2500 pixels and upload it again.

[azaozz]

This is already implemented in the Media modal and on the Media Library screen, see https://core.trac.wordpress.org/ticket/47872 and https://core.trac.wordpress.org/changeset/45934. The actual "old style" js is quite straightforward and is mostly here: https://core.trac.wordpress.org/browser/trunk/src/js/_enqueues/vendor/plupload/wp-plupload.js?rev=45934#L110.

It reuses the Plupload file.id as the upload ID but that can be any sufficiently random alpha-numeric string. The randomness is only needed as several images may be uploaded at the same time.

Related: https://core.trac.wordpress.org/ticket/47987#comment:1 for adding an API endpoint for the retry requests.

[youknowriad]

Few questions:

• Are the API things commited, ready? how can we test this while developping it?
• I guess we need a fallback somehow for previous WordPress versions in the plugin or will it just work automatically?
• Any thoughts on the implementation? Is this something that should be a wp.apiFetch middleware or implemented more specifically per use-case?

[azaozz]

Sorry for the delay in responding. Some problems with how that was working needed fixing.

Are the API things commited, ready?

Not yet. Working on https://core.trac.wordpress.org/ticket/47987 with @TimothyBJacobs. Just added https://core.trac.wordpress.org/ticket/48200 which change how all of this works.

A custom header with the attachment_id is added after an upload is complete and attachment post is created, but before post-processing of the image starts. That way all responses will contain the attachment_id, even when they are HTTP 500.

When an upload request ends with a HTTP 500 error, the client should look for that header and get the attachment ID.

Something like: id = response.responseHeaders.match( /x-wp-upload-attachment-id:\s*(\d+)/i );

Then, if there's an ID, do another request to resume creation of image sub-sizes. That request should be to the same "create attachment/upload" end point but should have another custom header (as this is considered "request meta" in REST). This can be repeated 2-3 times if the response is still HTTP 500. Finally if it is still HTTP 500 the last request should be to delete the new attachment.

I guess we need a fallback somehow for previous WordPress versions in the plugin or will it just work automatically?

It should "just work" as previous versions won't have the custom header with the attachment ID in HTTP 500 responses.

Any thoughts on the implementation? Is this something that should be a wp.apiFetch middleware or implemented more specifically per use-case?

Good question :) It should be able to look at the "raw" responses from the API and access the HTTP response headers. Can probably be part (or exception) of the general error handling.

[youknowriad]

Is there a way we can virtually trigger the errors to easily test the behavior and work on the patch?

[TimothyBJacobs]

I've been adding a die call to the top of wp_generate_attachment_metadata which has been working fairly well for me to simulate an error. But that isn't generating the exact error conditions.

https://core.trac.wordpress.org/ticket/47872#comment:1 also has testing instructions that will get you closer to the real behavior.

[azaozz]

Best way is to tweak php.ini on the test server, find and change
max_execution_time=4 or even 3 (default is 30) and memory_limit=64M, (default is 128M) then restart the server and try to upload big image.

I've also made a small testing plugin: https://gist.github.com/azaozz/91101d686127137b0f5b850f0d269972. It is mostly to show the attachment meta on the Media Library list view, but also adds a lot of image sub-sizes in order to overload the server (tweak the code at the top as needed).

Of course a filter on wp_generate_attachment_metadata will work too. It will "fail" the first upload request forcing it to do a retry.

The new post-process end point is already in Core: https://core.trac.wordpress.org/changeset/46422. The "flow" is described here: https://core.trac.wordpress.org/ticket/47987#comment:34.