Skip to content

fix: tell people when a snippet action fails - #488

Open
TallblokeUK wants to merge 1 commit into
core-betafrom
fix/surface-action-errors/core
Open

fix: tell people when a snippet action fails#488
TallblokeUK wants to merge 1 commit into
core-betafrom
fix/surface-action-errors/core

Conversation

@TallblokeUK

@TallblokeUK TallblokeUK commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Draft, opened for the reasoning as much as the code. Prompted by this report of snippets that cannot be trashed, deleted or disabled.

The problem

Every action on the snippets list ends here:

export const handleUnknownError = (error: unknown) => {
	console.error(error)
}

Twelve call sites route failures into it: activate, deactivate, trash, delete, restore, clone, export, priority, the view toggle, the preview modal, and every bulk action. Whatever goes wrong — a 403, a 500, a request blocked before it left the browser — the row does not change and nothing explains why.

A failed action is indistinguishable from a click that never registered. That is the exact phrasing in the report: "they stay exactly the same and won't move", "shows no page refresh".

Why this matters beyond the UX

I spent a while trying to work out what was wrong on that user's site and ruled out, with tests:

  • object caching serving a stale list — with a persistent drop-in installed, deactivate, trash and permanent delete all invalidated correctly
  • HTTP response caching — our REST responses send no-cache, no-store, private
  • the DELETE verb / fix: send REST writes as POST with a method override #467 — his disable fails too, and that is a plain POST
  • his own snippet corrupting the response — tested unconditional echo, header(), a forced Content-Type, a PHP warning, and headers via send_headers; all returned valid JSON. I confirmed his snippet genuinely does execute during REST requests first, so those were real tests
  • snippet cache headers poisoning the nonce — WordPress overrides them

Every one of those was a guess, because the plugin discarded the only piece of information that would have identified the cause. There is nothing for the user to report and nothing for us to ask.

What this does

Failures are surfaced in a notice above the table, saying what did not happen, why, and that nothing was changed.

The HTTP status is included deliberately. Requests to the snippets API get blocked by security rules and firewalls often enough that "it did nothing" cannot be diagnosed without it, and asking people to open developer tools is a poor substitute for the plugin simply saying what happened.

Bulk actions previously discarded each snippet's error inside the loop. They now count failures and report once with the number affected, because a batch can partly succeed and "three of ten failed" is a very different situation to nothing having happened.

Errors are still logged to the console, so the full object remains available.

Verified

Simulated exactly that user's situation with an mu-plugin rejecting REST writes with 403, then clicked the activation toggle. Before, nothing at all. After:

Could not deactivate this snippet. Your site refused the request (403). Try reloading the page, and check whether a security plugin is blocking it. Blocked by site security rules. Nothing has been changed.

That is a message a user can act on and a support thread can quote.

eslint clean across the project.

Open questions for review

  • Notice placement. Currently above the table. It could arguably sit next to the affected row, though that gets awkward for bulk actions.
  • Wording. The 403 message guesses at a cause. Worth deciding whether we want to be that opinionated, or stay neutral and just report the status.
  • Scope. This covers the manage screen only. The edit screen and preview modal have the same pattern and are untouched here.
  • Target branch. Opened against core-beta per the policy for shared changes, but there is an argument for core, since the people who need it most are on released versions right now.

Fixes #516

Every action on the snippets list sent its error to console.error and
nothing else:

  export const handleUnknownError = (error: unknown) => {
    console.error(error)
  }

Activate, deactivate, trash, delete, restore, clone, export, priority and
all the bulk actions route failures there. Whatever goes wrong, a 403, a
500, a request that never left the browser, the row does not change and
nothing explains why. A failed action looks exactly like a click that
never registered.

That is bad on its own, and it is also why this class of problem cannot
be supported. Someone reports that snippets cannot be deleted, and there
is nothing for them to tell us and nothing for us to ask, because the
plugin discarded the one piece of information that would have identified
the cause.

Surface failures where the person is already looking. Actions now report
into a notice above the table, saying what did not happen, why, and that
nothing was changed. The HTTP status is included deliberately: requests
to the snippets API are blocked by security rules often enough that
"it did nothing" is impossible to diagnose without it.

Bulk actions previously discarded each snippet's error inside the loop.
They now count the failures and report once, with the number affected,
because a batch can partly succeed and "three of ten failed" is a very
different situation to nothing having happened.

Errors are still logged to the console, so the full object stays
available.

Prompted by a support report of snippets that could not be trashed,
deleted or disabled, where every theory was untestable because the
plugin reported nothing at all.
@TallblokeUK
TallblokeUK marked this pull request as ready for review August 30, 2026 11:41
@TallblokeUK

Copy link
Copy Markdown
Contributor Author

Verified in a combined 3.10.2 test build, installed from a zip on a clean WordPress 7.1 / PHP 8.1 site. Forcing a request to fail produces:

Could not deactivate this snippet. Your site returned an error (500). Simulated failure. Nothing has been changed.

Confirmed on both the free and Pro builds.

One thing for whoever merges this: it overlaps with #490. Both add a near-duplicate function to src/js/utils/errors.ts — this PR adds describeError() (HTTP status handling) and #490 adds describeRequestError() (expired sessions). Whichever merges second will conflict.

They combine cleanly by checking the session-expiry codes first and falling back to the status branch, with both call sites (useActionFeedback and useSubmitSnippet) pointing at the single function. I resolved it that way in the test build and exercised both paths — a forced 500 gives the status message above, and a rest_cookie_invalid_nonce gives the "you have been signed out" message. Happy to push that reconciliation onto whichever branch merges second.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Show clear feedback when snippet activation fails

1 participant