[wp-trac] [WordPress Trac] #47987: REST API: Add handling of PHP fatal errors while resizing images after upload

[WordPress Trac]

#47987: REST API: Add handling of PHP fatal errors while resizing images after
upload
-----------------------------+--------------------------------
 Reporter:  azaozz           |       Owner:  TimothyBlynJacobs
     Type:  task (blessed)   |      Status:  accepted
 Priority:  highest omg bbq  |   Milestone:  5.3
Component:  REST API         |     Version:
 Severity:  normal           |  Resolution:
 Keywords:  needs-patch      |     Focuses:
-----------------------------+--------------------------------

Comment (by TimothyBlynJacobs):

 > Sure but... Why in the header? The upload ID/reference is still part of
 the data sent to the server. It is not that different than the rest of the
 data that the server needs in order to process the upload?

 REST semantics wise, the reference isn't describing the actual resource
 itself. It provides information about how the request should be processed;
 metadata. Which would typically be in the header. I'm not strongly opposed
 to it being in the request body, but I think it makes more sense for
 supplementary information to be in the headers.

 > The upload is complete by that point and the attachment post is already
 created (or at least is expected to have been created).

 > None of the original fields are needed any more.

 The attachment is created, but the alt text hasn't been saved, the
 additional fields haven't been set, and the `rest_after_insert_attachment`
 hook hasn't fired yet. So we'd either need to make that happen before
 `wp_update_attachment_metadata` is called, in which case the state of the
 attachment will be different during all of those callbacks.

 Or, duplicate those calls in the other endpoint. Duplicating wouldn't
 really be an option because the `WP_REST_Request` object will be different
 than expected for creating a new attachment.

 > Re-sending the same original data is redundant or may potentially result
 in a duplicate?

 It is redundant, but wouldn't result in a duplicate because of the
 presence of the header.

 > At this point we are just monitoring or triggering post-processing of
 the uploaded file. Eventually that can be used for other post-processing,
 not just image resizing.

 I personally don't think we have enough time to scope out how that would
 work before the deadline for 5.3. I think it'd also be easier to add a new
 endpoint at the point it is required, than trying to modify a very
 specifically designed endpoint just for doing resizing after 500 errors.

 For instance, the image ref isn't permanently stored and at that point
 we'd have the attachment ID. So it'd make sense to be an endpoint off of
 the single attachment route. ie `/wp/v2/media/{id}/process` ( or some
 other better named endpoint ).

 I think it'd also give us the opportunity to explore a stepped-processing
 endpoint that isn't specific to media. I think that'd be the best way to
 handle this, but if we need something by Monday :)

 For just this feature, I personally don't think we get much of a benefit
 from creating a separate endpoint.

 > Actually I'm not entirely sure how uploads are supposed to work through
 a REST API.

 The REST API supports `$_FILES` or passing the data in the request body.
 So it should be possible to do a direct POST.

 -----

 > I like the stripe idempotency idea @TimothyBlynJacobs mentioned. It
 would definitely require some agreement that it's the right approach
 across the API, but it seems a close fit to the retries model being
 proposed.

 I don't think it'd be something that we could implement in time for 5.3,
 and thus solve this feature, but I agree it'd be interesting.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/47987#comment:15>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform