[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:
-----------------------------+--------------------------------
Changes (by TimothyBlynJacobs):

 * owner:  (none) => TimothyBlynJacobs
 * status:  new => accepted


Comment:

 Instead of a separate endpoint for continuing, I think we need to have all
 of this done on the `POST` `create_item()` route, so that the
 `WP_REST_Request` object has all the expected data for the additional
 fields that are processed, and the `*_insert` actions.

 So I was thinking the request would be the normal `POST` but with an added
 header, something like `X-WP-UploadRef: randomReference`. Then, if the
 server responds with a 500, you’d make the same POST request, but with the
 header `X-WP-RetryUploadRef: randomReference`. The POST fields should be
 identical to the initial request, but the file data could be omitted.

 It feels a bit weird to me that we are controlling the request flow based
 on a header name, but it seemed like the most straightforward way to
 implement the feature while still maintaining as full compatibility as
 possible with existing code.

 Additionally, as @kadamwhite mentioned, we'd be introducing more arbitrary
 header names.

 Initially, the description of the feature made me think a lot about
 [https://stripe.com/docs/api/idempotent_requests Stripe's Idempotent
 Request] API. Not an exact match ( we'd ''need'' to handle server errors
 ), but could be a single header key that we'd use for the entire API.
 However, I think that'd be too complex for 5.3 and I don't think getting
 in a solution specific to media would block us from considering a more
 extensive feature in the future.

 We could consolidate into a single header name, and decide whether to
 continue or create a new attachment based on if the upload reference is
 valid. If the upload reference was persistent, perhaps post meta on the
 attachment that would be removed once the attachment is fully processed,
 I'd be more comfortable with that. But with transients being transient, it
 seemed like it could result in an unclear error, or a double attachment (
 since the request would be otherwise the same ) if the transient was
 evicted before the client was finished with it.

 Thoughts? Looping in @rmccue and @joehoyle.

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