fix: recover media uploads that fail server-side post-processing by dcalhoun · Pull Request #594 · wordpress-mobile/GutenbergKit

dcalhoun · 2026-08-21T12:02:35Z

What?

Registers @wordpress/api-fetch's mediaUploadMiddleware, which retries post-process when a media upload fails server-side, and makes both the native upload path and the orphan cleanup work with it.

Why?

Ref CMM-2151. Ref CMM-2274.

When wp_generate_attachment_metadata() fatals server-side — commonly a PHP memory_limit or max_execution_time fatal on a large image — the upload surfaces as a permanent failure. The user is left with an orphaned gray attachment, and retrying duplicates it (the -2/-3 filename suffixes seen in the field).

WordPress already supports recovering from this: it creates the attachment row, sends X-WP-Upload-Attachment-ID, and expects the client to retry POST /wp/v2/media/<id>/post-process. Gutenberg's mediaUploadMiddleware implements that. WordPress core registers it, but GutenbergKit never did.

How?

Renames the local middleware to stripDraftPostIdMiddleware and registers apiFetch.mediaUploadMiddleware. apiFetch.use unshifts, so registration order is the reverse of execution order: core's is registered after the native one so it runs before it, wrapping it — the native middleware handles an upload without calling next, so core's would never run at all from below. Both stay above auth, namespacing, and the root URL so the post-process requests core issues stay authenticated.

Two things then had to change for that retry to work in practice:

relayResponse rebuilt the response with a hardcoded Content-Type, dropping x-wp-upload-attachment-id. It is now relayed via an allowlist and exposed through CORS, and nativeMediaUploadMiddleware honors parse: false by yielding the Response core inspects.
The orphan cleanup was blocked at CORS preflight: api-fetch tunnels DELETE as a POST carrying X-HTTP-Method-Override, which core's rest_allowed_cors_headers omits. Media deletions now route through the loopback server, which permits DELETE. This was a pre-existing bug, independent of the retry work.

Also fixes the namespace guard: new RegExp('(' + [].join('|') + ')') is /()/, which matches every string, so alreadyHasSiteNamespace was unconditionally true for self-hosted sites. That was load-bearing — it suppressed a rewrite that would otherwise interpolate undefined into every path.

Adds a wp-env media failure simulator (make wp-env-media-failure MODE=off|recover|always) so the retry can be exercised locally. See docs/code/local-wordpress.md.

Known limitation

Recovery requires reading x-wp-upload-attachment-id, which is only possible same-origin or when the server exposes it via CORS. The native upload server exposes it, so uploads routed through it recover on both platforms. A direct upload depends on the site: core's rest_send_cors_headers() does not expose the header, so it recovers only where the editor is genuinely same-origin with the site. There is no client-side fallback — core sends the header before wp_generate_attachment_metadata(), so the fatal leaves no body to parse the ID from.

Testing Instructions

Requires the local WordPress environment and a build with native media upload enabled.

make wp-env-start (add RESET=1 if credentials are stale)
make wp-env-media-failure MODE=recover
Build and run a demo app, open the editor against Local WordPress (wp-env), and insert an Image block with any image.
Expect: the upload 500s, a post-process request follows and succeeds, and the image renders with no error notice. The attachment has all sub-sizes and there is exactly one of it.
make wp-env-media-failure MODE=always, then upload again.
Expect: the upload 500s, five post-process attempts fail, then a DELETE to localhost:<port>/media/<id> returns 200 and the media library is left empty.
make wp-env-media-failure MODE=off when finished.

Verified end to end on both platforms in all four combinations (iOS and Android × recover and always).

Accessibility Testing Instructions

Not applicable — no user interface changes.

The local `mediaUploadMiddleware` shadowed the same-named export from `@wordpress/api-fetch`, so the file read as though core's post-process retry behavior was registered when only the draft post ID stripping was. Rename it to `stripDraftPostIdMiddleware` to describe what it does. No behavior change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ULMqNwTEWty4MeNrr94MuF

When `wp_generate_attachment_metadata()` fails server-side (commonly a PHP memory_limit or max_execution_time fatal on large images), WordPress returns a 5xx carrying an `x-wp-upload-attachment-id` header. Core's `mediaUploadMiddleware` recovers from this by retrying `POST /wp/v2/media/<id>/post-process` up to five times, then deleting the orphaned attachment if every attempt fails. That middleware was never registered, so these uploads surfaced as failures that left an orphaned gray attachment behind and duplicated the attachment on retry. `apiFetch.use` unshifts, so registration order is the reverse of execution order. Core's middleware is registered before the native one so that it runs after it, and below auth, namespacing, and the root URL so the `post-process` requests it issues through `next` stay authenticated and correctly addressed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ULMqNwTEWty4MeNrr94MuF

`new RegExp('(' + [].join('|') + ')')` is `/()/`, which matches every string, so `alreadyHasSiteNamespace` was unconditionally true whenever a site configured no namespace — as self-hosted sites do. That was load-bearing rather than merely benign: it suppressed a rewrite that would otherwise interpolate `siteApiNamespace[0]` — `undefined` for an empty namespace — into every path, producing `/wp/v2/undefinedposts`. Gate the rewrite on a configured namespace so the guard no longer has to, and escape the namespaces so each is matched literally. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ULMqNwTEWty4MeNrr94MuF

The native upload path circumvented core's post-process retry entirely, so a metadata fatal on a delegate-handled upload stayed a permanent failure with an orphaned attachment left behind. Two things blocked it: - `relayResponse` rebuilt the response with a hardcoded Content-Type, dropping `x-wp-upload-attachment-id` — the header core's middleware needs to identify the attachment to retry. Relay it (via an allowlist, since the body is re-sent with a recomputed length) and expose it through CORS, without which the WebView cannot read it cross-origin regardless. - `nativeMediaUploadMiddleware` always parsed the body, so it never yielded the `Response` that core's middleware inspects. Honor `parse: false` by resolving or rejecting with the `Response` itself. `MediaUploadResponse` gains a `headers` property on both platforms, defaulted so existing host callers are unaffected. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ULMqNwTEWty4MeNrr94MuF

The retry depends on reading `x-wp-upload-attachment-id`, which is only possible same-origin or where the server exposes it via CORS. Record which combinations recover, and why there is no client-side fallback, so the iOS direct-upload case is not mistaken for a bug in this registration. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ULMqNwTEWty4MeNrr94MuF

Reproduces the server-side image processing fatal the post-process retry recovers from, so the middleware can be exercised locally without a large image or a resource-starved host. Adapted from the approach in WordPress/gutenberg#17858, with the random failure rate replaced by an explicit mode (`recover`/`always`/`off`) so both the recovery and the exhaust-and-delete paths are reproducible. The mode is an option rather than per-request state, since the upload and each retry are separate requests and a native-server upload carries no browser cookie. The plugin sets the 500 itself: a real fatal under FPM surfaces as a 500, but the Playground runtime returns 200, which the retry would ignore. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ULMqNwTEWty4MeNrr94MuF

Reading or flipping the mode meant a curl invocation with an inline credentials lookup, which is easy to get wrong mid-debug and silently no-ops if it fails — leaving a passing upload that looks like the retry never fired. Wrap it in `make wp-env-media-failure [MODE=off|recover|always]`, matching the existing VAR=value convention and the thin-target-plus-bin-script pattern. Each precondition reports the fix that applies to it: missing credentials, a rejected 401 (stale after a Playground restart), an unreachable server, and an unregistered endpoint. Also document the orphaned-server and 401 cases in troubleshooting; both came up repeatedly while testing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ULMqNwTEWty4MeNrr94MuF

A bundled Android build serves the editor from the site's host without a port (`http://10.0.2.2`), which was not in the allowlist, so its REST requests were rejected before reaching WordPress. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ULMqNwTEWty4MeNrr94MuF

Testing a bundled Android build against wp-env disproved the claim that Android direct uploads are same-origin and therefore recover. `GutenbergView` derives the asset domain from the site's host, and `host` drops the port — so the editor at `http://10.0.2.2` is cross-origin with a site at `http://10.0.2.2:8888`, and the attachment ID header stays unreadable. The claim holds only when the site runs on the scheme's default port, as production sites do. Say that, rather than implying every Android site recovers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ULMqNwTEWty4MeNrr94MuF

`always` mode fatalled on every sub-size pass, and core's `force=true` delete path runs sub-size handling too — so the editor's orphan cleanup fatalled as well, leaving the orphan behind. The retry logic was correct; the simulator refused the cleanup it had correctly requested. Exempt deletes, including the `POST` + `X-Http-Method-Override: DELETE` form api-fetch sends, so the bare request method alone is not enough to identify one. Also document that a simulated fatal aborts the request before WordPress adds CORS headers, so these responses surface as CORS errors rather than readable 500s. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ULMqNwTEWty4MeNrr94MuF

When every post-process retry fails, core's middleware deletes the orphaned attachment. A cross-origin editor cannot make that request: api-fetch tunnels DELETE as a POST carrying `X-HTTP-Method-Override`, and core's `rest_allowed_cors_headers` omits that header, so the browser blocks it at preflight and the orphan survives. Route media deletions through the loopback upload server instead, which sets its own CORS policy and already permits DELETE. The middleware intercepts before api-fetch's `httpV1` adds the override header, so what reaches the native server is a plain DELETE. Both servers gain a single narrow route — `DELETE /media/<id>` with a numeric ID — rather than a general proxy, matching the existing `POST /upload`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ULMqNwTEWty4MeNrr94MuF

wpmobilebot · 2026-08-21T12:15:36Z

XCFramework Build

This PR's XCFramework is available for testing. Add the following to your Package.swift:

.package(url: "https://github.com/wordpress-mobile/GutenbergKit", branch: "pr-build/594")

_{Built from 6140749}

`handleDelete` went straight to the default uploader, unlike `handleUpload` which offers the work to the delegate first. A host whose `uploadFile` uploads to its own media service holds an ID only it can resolve, so deleting through the default uploader would address the wrong site. Add `deleteFile(attachmentId:)` to `MediaUploadDelegate` on both platforms, defaulted to nil so existing hosts are unaffected, and try it before falling back. Rename the handler to `handleMediaDelete`, since it deletes an attachment rather than an upload and no longer mirrors `handleUpload`'s signature. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ULMqNwTEWty4MeNrr94MuF

Drop the two notes explaining how the simulator produces its 500 and why a fatal response reads as a CORS error — implementation detail that belongs in the plugin, not the guide. Drop the orphaned-server and stale-credential troubleshooting entries; those are environment problems to address on their own. Also drop a comment restating what the adjacent condition already says, and reword the make target's help text. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ULMqNwTEWty4MeNrr94MuF

`nativeMediaUploadMiddleware` mixed dispatch with the whole upload implementation, so adding the deletion path left the two handled asymmetrically — one extracted, one inline. Extract `nativeMediaUpload` alongside `nativeMediaDelete`, both returning null when a request is not theirs, leaving the middleware as a short dispatcher. No behavior change. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ULMqNwTEWty4MeNrr94MuF

`handleMediaDelete` caught only `IOException`, so a delegate's `deleteFile` throwing anything else — `IllegalStateException`, a JSON error — escaped to `HttpServer.resolveResponse` and returned a plain-text 500. The editor's `nativeMediaDelete` then failed on `response.json()` and reported `invalid_json` rather than the delegate's actual failure. Catch `Exception` and rethrow `CancellationException`, matching `passthroughResponse` and iOS's untyped `catch`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EusJFgEHAvZNESsD3xHbvr

`relayResponse` prepended `Content-Type: application/json` to an array of the response's own headers, and `HTTPResponse` serializes every entry it is given. A delegate returning its own `Content-Type` therefore put the header on the wire twice, which URLSession surfaces as "application/json, text/plain". Android's map merge already overrode instead, so the two platforms disagreed on the same public API. Skip the default when the response already carries the name, matching Android. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EusJFgEHAvZNESsD3xHbvr

`deleteFile` is called for every deletion, including attachments the delegate declined at upload time — an attachment ID carries no MIME type or filename, so there is no `handlesFile` gate to apply. A delegate answering for one of those leaves the real WordPress attachment undeleted, which is the orphan the cleanup exists to remove. Returning nil already falls through to the default uploader; document that as the signal for an unrecognized ID. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EusJFgEHAvZNESsD3xHbvr

The credentials path was interpolated into the `node -e` source as a single-quoted JS string literal, so a checkout under a path containing a quote or backslash produced a SyntaxError stack trace instead of the intended "could not read authHeader" message. Pass it through `process.argv` and single-quote the script body so the shell does not expand it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EusJFgEHAvZNESsD3xHbvr

`relayResponse` merged the JSON default with the response's own headers via Kotlin's map merge, which only overrides on an exact key match. A delegate returning `content-type` therefore produced a two-entry map, and `serializeResponse` writes every entry, putting the header on the wire twice — the WebView sees "application/json, text/plain". This is the same defect `0bf40ad3` fixed on iOS, which the map merge was believed to already handle. Skip the default when the response carries the name under any casing, matching iOS and the case-insensitive lookups `HttpServer` already uses. Add the Android counterpart to the iOS `delegateContentTypeWins` test. It asserts on the raw header lines rather than the parsed map, which lowercases keys into a map and would collapse the duplicate — hiding the very bug. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LVdVBubbCDp7mRXSWkcHHm

dcalhoun · 2026-08-21T12:54:29Z

-		'http://appassets.androidplatform.net',    // Android production build (HTTP site)
+		'http://localhost:5173',                 // Vite dev server
+		'http://localhost:4173',                 // Vite preview server
+		'http://10.0.2.2',                       // wp-env site origin (Android emulator)


In prior work, wordpress-rs began configuring the site URL based upon discovery. This adds the local wp-env site's Android emulator domain to avoid CORS errors. Strictly a dev environment fix, no production implications.

dcalhoun · 2026-08-21T19:28:03Z

+		siteApiNamespace.length > 0 &&
 		! namespaceExcludedPaths.some( ( path ) =>
 			options.path.startsWith( path )
 		);

+	// Escape the namespaces so each is matched literally rather than as a
+	// pattern.
 	const alreadyHasSiteNamespace =
-		namespaceRegex.test( options.path ) ||
-		/\/sites\/[^/]+\//.test( options.path );
+		new RegExp(
+			`(${ siteApiNamespace.map( escapeRegExp ).join( '|' ) })`
+		).test( options.path ) || /\/sites\/[^/]+\//.test( options.path );


Address latent bugs by preventing matches from unescaped namespaces.

dcalhoun · 2026-08-21T19:29:12Z

+	// Each helper returns `null` when the request is not its concern, so an
+	// unhandled request falls through to the default path.
+	return (
+		nativeMediaDelete( options, nativeUploadPort, nativeUploadToken ) ??


We must relay the media delete request through the proxy to avoid CORS errors due to the critical X-WP-Upload-Attachment-ID header.

dcalhoun and others added 11 commits August 20, 2026 15:26

github-actions Bot added the [Type] Bug An existing feature does not function as intended label Aug 21, 2026

dcalhoun and others added 3 commits August 21, 2026 09:12

dcalhoun force-pushed the fix/register-core-media-upload-middleware branch from e02ddde to ebb67a5 Compare August 21, 2026 13:22

dcalhoun and others added 5 commits August 21, 2026 14:20

dcalhoun commented Aug 21, 2026

View reviewed changes

dcalhoun marked this pull request as ready for review August 21, 2026 20:06

dcalhoun requested a review from jkmassel August 21, 2026 20:15

Navigation Menu

Uh oh!

fix: recover media uploads that fail server-side post-processing - #594

fix: recover media uploads that fail server-side post-processing#594
dcalhoun wants to merge 19 commits into
trunkfrom
fix/register-core-media-upload-middleware

dcalhoun commented Aug 21, 2026 •

edited

Loading

Uh oh!

wpmobilebot commented Aug 21, 2026 •

edited

Loading

Uh oh!

dcalhoun Aug 21, 2026

Uh oh!

dcalhoun Aug 21, 2026 •

edited

Loading

Uh oh!

dcalhoun Aug 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

dcalhoun commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What?

Why?

How?

Known limitation

Testing Instructions

Accessibility Testing Instructions

Uh oh!

wpmobilebot commented Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

XCFramework Build

Uh oh!

dcalhoun Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

dcalhoun Aug 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dcalhoun Aug 21, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dcalhoun commented Aug 21, 2026 •

edited

Loading

wpmobilebot commented Aug 21, 2026 •

edited

Loading

dcalhoun Aug 21, 2026 •

edited

Loading