From 2ecf2fb693f413fd91eff02a3289e571c176b017 Mon Sep 17 00:00:00 2001 From: TallblokeUK Date: Sat, 29 Aug 2026 08:35:57 +0100 Subject: [PATCH] fix: tell people when a snippet action fails 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. --- .../SnippetsTable/ActionFeedback.tsx | 35 +++++++++ .../SnippetsTable/ManageSnippetCard.tsx | 18 +++-- .../SnippetsTable/SnippetsTable.tsx | 12 ++- .../ManageMenu/SnippetsTable/TableColumns.tsx | 10 ++- .../SnippetsTable/useApplyBulkAction.ts | 78 +++++++++++++++---- src/js/hooks/useActionFeedback.tsx | 68 ++++++++++++++++ src/js/utils/errors.ts | 61 ++++++++++++++- 7 files changed, 256 insertions(+), 26 deletions(-) create mode 100644 src/js/components/ManageMenu/SnippetsTable/ActionFeedback.tsx create mode 100644 src/js/hooks/useActionFeedback.tsx diff --git a/src/js/components/ManageMenu/SnippetsTable/ActionFeedback.tsx b/src/js/components/ManageMenu/SnippetsTable/ActionFeedback.tsx new file mode 100644 index 000000000..282f3454a --- /dev/null +++ b/src/js/components/ManageMenu/SnippetsTable/ActionFeedback.tsx @@ -0,0 +1,35 @@ +import React from 'react' +import { __ } from '@wordpress/i18n' +import { failureMessage, useActionFeedback } from '../../../hooks/useActionFeedback' +import { DismissibleNotice } from '../../common/Notice' + +/** + * Show any snippet action that did not complete. + * + * Rendered above the table so a failure appears where the person is already + * looking, rather than only in the browser console. + */ +export const ActionFeedback: React.FC = () => { + const { failures, dismissFailure } = useActionFeedback() + + if (0 === failures.length) { + return null + } + + return ( + <> + {failures.map(failure => + dismissFailure(failure.id)} + > +

+ {failureMessage(failure)}{' '} + {__('Nothing has been changed.', 'code-snippets')} +

+ )} + + ) +} diff --git a/src/js/components/ManageMenu/SnippetsTable/ManageSnippetCard.tsx b/src/js/components/ManageMenu/SnippetsTable/ManageSnippetCard.tsx index 7e766f5e9..430d8b1a6 100644 --- a/src/js/components/ManageMenu/SnippetsTable/ManageSnippetCard.tsx +++ b/src/js/components/ManageMenu/SnippetsTable/ManageSnippetCard.tsx @@ -4,9 +4,9 @@ import { RawHTML } from '@wordpress/element' import { __, sprintf } from '@wordpress/i18n' import React, { useState } from 'react' import { ConfirmDeleteDialog, useDeleteSnippet } from '../../common/snippets/ConfirmDeleteDialog' +import { useActionFeedback } from '../../../hooks/useActionFeedback' import { useSnippetsAPI } from '../../../hooks/useSnippetsAPI' import { useSnippetsList } from '../../../hooks/useSnippetsList' -import { handleUnknownError } from '../../../utils/errors' import { downloadSnippetExportFile } from '../../../utils/files' import { canModifySnippet, cloneSnippetObject, getSnippetDisplayName, getSnippetEditUrl, getSnippetType, isNetworkOnlySnippet, isSnippetActive } from '../../../utils/snippets/snippets' import { Button } from '../../common/Button' @@ -39,6 +39,7 @@ const CardPreviewButton: React.FC = ({ snippet }) => { const CloneExportMenuItems: React.FC = ({ snippet }) => { const api = useSnippetsAPI() const { refreshSnippetsList } = useSnippetsList() + const { reportFailure } = useActionFeedback() return ( <> @@ -46,7 +47,7 @@ const CloneExportMenuItems: React.FC = ({ snippet }) => onSelect={() => { api.create(cloneSnippetObject(snippet)) .then(refreshSnippetsList) - .catch(handleUnknownError) + .catch((error: unknown) => reportFailure(__('clone this snippet', 'code-snippets'), error)) }} > {__('Clone', 'code-snippets')} @@ -56,7 +57,7 @@ const CloneExportMenuItems: React.FC = ({ snippet }) => onSelect={() => { api.export(snippet) .then(response => downloadSnippetExportFile(response, snippet)) - .catch(handleUnknownError) + .catch((error: unknown) => reportFailure(__('export this snippet', 'code-snippets'), error)) }} > {__('Export', 'code-snippets')} @@ -77,6 +78,7 @@ const RestoreDeleteMenuItems: React.FC = ({ }) => { const api = useSnippetsAPI() const { refreshSnippetsList } = useSnippetsList() + const { reportFailure } = useActionFeedback() return ( <> @@ -87,7 +89,7 @@ const RestoreDeleteMenuItems: React.FC = ({ onSelect={() => { api.restore(snippet) .then(refreshSnippetsList) - .catch(handleUnknownError) + .catch((error: unknown) => reportFailure(__('restore this snippet', 'code-snippets'), error)) }} > {__('Restore', 'code-snippets')} @@ -103,11 +105,17 @@ const RestoreDeleteMenuItems: React.FC = ({ const CardActionsMenu: React.FC = ({ snippet }) => { const { refreshSnippetsList } = useSnippetsList() + const { reportFailure } = useActionFeedback() const canModify = canModifySnippet(snippet) const { requestDelete, deleteDialogProps } = useDeleteSnippet({ snippet, onSuccess: refreshSnippetsList, - onError: handleUnknownError + onError: (error: unknown) => reportFailure( + snippet.trashed + ? __('delete this snippet', 'code-snippets') + : __('move this snippet to the trash', 'code-snippets'), + error + ) }) return ( diff --git a/src/js/components/ManageMenu/SnippetsTable/SnippetsTable.tsx b/src/js/components/ManageMenu/SnippetsTable/SnippetsTable.tsx index 2be7c018e..41f9cc4f4 100644 --- a/src/js/components/ManageMenu/SnippetsTable/SnippetsTable.tsx +++ b/src/js/components/ManageMenu/SnippetsTable/SnippetsTable.tsx @@ -12,10 +12,12 @@ import { isLicensed } from '../../../utils/screen' import { SNIPPET_TYPE_LABELS, getSnippetAddNewUrl, getSnippetType, isProType } from '../../../utils/snippets/snippets' import { buildUrl } from '../../../utils/urls' import { Badge } from '../../common/Badge' +import { WithActionFeedbackContext } from '../../../hooks/useActionFeedback' import { Notice } from '../../common/Notice' import { ScreenMetaSlot } from '../../common/ScreenMetaSlot' import { UpsellPage } from '../../common/UpsellDialog' import { WithSnippetsTableFilters, useSnippetsFilters } from './WithSnippetsTableFilters' +import { ActionFeedback } from './ActionFeedback' import { WithFilteredSnippetsContext } from './WithFilteredSnippetsContext' import { SnippetsListTable } from './SnippetsListTable' import type { SnippetType } from '../../../types/Snippet' @@ -121,6 +123,8 @@ const SnippetsTableInner = () => { return ( <> + +
- - - + + + + + diff --git a/src/js/components/ManageMenu/SnippetsTable/TableColumns.tsx b/src/js/components/ManageMenu/SnippetsTable/TableColumns.tsx index d44968392..30879b547 100644 --- a/src/js/components/ManageMenu/SnippetsTable/TableColumns.tsx +++ b/src/js/components/ManageMenu/SnippetsTable/TableColumns.tsx @@ -3,8 +3,8 @@ import { RawHTML } from '@wordpress/element' import { __ } from '@wordpress/i18n' import React, { Fragment } from 'react' import { useSnippetsAPI } from '../../../hooks/useSnippetsAPI' +import { useActionFeedback } from '../../../hooks/useActionFeedback' import { useSnippetsList } from '../../../hooks/useSnippetsList' -import { handleUnknownError } from '../../../utils/errors' import { isNetworkAdmin } from '../../../utils/screen' import { getSnippetDisplayName, getSnippetEditUrl, getSnippetType } from '../../../utils/snippets/snippets' import { buildUrl } from '../../../utils/urls' @@ -35,6 +35,7 @@ const RunOnceButton: React.FC = ({ snippet }) => const ActivationSwitch: React.FC = ({ snippet }) => { const { activate, deactivate } = useSnippetsAPI() const { refreshSnippetsList } = useSnippetsList() + const { reportFailure } = useActionFeedback() const actionText = snippet.network && !snippet.shared_network ? snippet.active ? __('Network Deactivate', 'code-snippets') : __('Network Activate', 'code-snippets') @@ -53,7 +54,12 @@ const ActivationSwitch: React.FC = ({ snippet }) => { onChange={() => { (snippet.active ? deactivate(snippet) : activate(snippet)) .then(refreshSnippetsList) - .catch(handleUnknownError) + .catch((error: unknown) => reportFailure( + snippet.active + ? __('deactivate this snippet', 'code-snippets') + : __('activate this snippet', 'code-snippets'), + error + )) }} /> ) diff --git a/src/js/components/ManageMenu/SnippetsTable/useApplyBulkAction.ts b/src/js/components/ManageMenu/SnippetsTable/useApplyBulkAction.ts index 6fa763444..7d8d10bf8 100644 --- a/src/js/components/ManageMenu/SnippetsTable/useApplyBulkAction.ts +++ b/src/js/components/ManageMenu/SnippetsTable/useApplyBulkAction.ts @@ -1,7 +1,7 @@ -import { __ } from '@wordpress/i18n' +import { __, sprintf } from '@wordpress/i18n' import { useSnippetsAPI } from '../../../hooks/useSnippetsAPI' +import { useActionFeedback } from '../../../hooks/useActionFeedback' import { useSnippetsList } from '../../../hooks/useSnippetsList' -import { handleUnknownError } from '../../../utils/errors' import { downloadBulkSnippetExportFile } from '../../../utils/files' import { cloneSnippetObject } from '../../../utils/snippets/snippets' import type { ListTableAction } from '../../common/ListTable' @@ -81,22 +81,71 @@ const submitBulkSnippetDownloadsIndividually = (snippets: readonly Snippet[]): P const applyAndRefresh = async ( targets: Snippet[], action: (snippet: Snippet) => Promise | Promise, - refresh: () => Promise + refresh: () => Promise, + onFailure: (failed: number, total: number, error: unknown) => void ): Promise => { if (0 < targets.length) { + let failed = 0 + let firstError: unknown + + // Every snippet is attempted even when one fails, so a single bad + // snippet does not silently halt the rest of the batch. Failures used + // to be discarded here, which is why a bulk action that did nothing + // looked exactly like one that had worked. for (const snippet of targets) { - await action(snippet).catch(handleUnknownError) + try { + await action(snippet) + } catch (error: unknown) { + failed += 1 + firstError ??= error + } } await refresh() + + if (0 < failed) { + onFailure(failed, targets.length, firstError) + } } } +/** + * Build the failure reporter for one bulk action. + * + * The count is included because a batch can partly succeed, and "three of ten + * failed" is a very different situation to "nothing happened". + */ +const bulkFailureReporter = ( + reportFailure: (action: string, error: unknown) => void, + label: string +) => (failed: number, total: number, error: unknown) => + reportFailure( + sprintf( + /* translators: 1: what was being done, 2: number that failed, 3: number attempted. */ + __('%1$s (%2$d of %3$d failed)', 'code-snippets'), + label, + failed, + total + ), + error + ) + +/** + * Send the selected snippets to the browser as downloads. + * + * Falls back to one download per snippet where the server cannot build a zip. + */ +const submitBulkDownload = (selectedSnippets: Snippet[]): Promise => + 1 < selectedSnippets.length && !window.CODE_SNIPPETS_MANAGE?.supportsZipDownloads + ? submitBulkSnippetDownloadsIndividually(selectedSnippets) + : submitBulkSnippetDownload(selectedSnippets) + export const useApplyBulkAction = ( allSnippets: Snippet[] ): (action: SnippetsTableAction | undefined, selected: Set) => Promise => { const api = useSnippetsAPI() const { refreshSnippetsList } = useSnippetsList() + const { reportFailure } = useActionFeedback() return async (action, selected) => { switch (action) { @@ -104,41 +153,40 @@ export const useApplyBulkAction = ( await applyAndRefresh( allSnippets.filter(snippet => selected.has(snippet.id) && !snippet.active), snippet => api.activate({ id: snippet.id, network: snippet.network }), - refreshSnippetsList) + refreshSnippetsList, + bulkFailureReporter(reportFailure, __('activate the selected snippets', 'code-snippets'))) break case 'deactivate': await applyAndRefresh( allSnippets.filter(snippet => selected.has(snippet.id) && snippet.active), snippet => api.deactivate({ id: snippet.id, network: snippet.network }), - refreshSnippetsList) + refreshSnippetsList, + bulkFailureReporter(reportFailure, __('deactivate the selected snippets', 'code-snippets'))) break case 'clone': await applyAndRefresh( allSnippets.filter(snippet => selected.has(snippet.id) && !snippet.trashed), snippet => api.create(cloneSnippetObject(snippet)), - refreshSnippetsList) + refreshSnippetsList, + bulkFailureReporter(reportFailure, __('clone the selected snippets', 'code-snippets'))) break case 'export': downloadBulkSnippetExportFile(allSnippets.filter(snippet => selected.has(snippet.id))) break - case 'download': { - const selectedSnippets = allSnippets.filter(snippet => selected.has(snippet.id)) - - return 1 < selectedSnippets.length && !window.CODE_SNIPPETS_MANAGE?.supportsZipDownloads - ? submitBulkSnippetDownloadsIndividually(selectedSnippets) - : submitBulkSnippetDownload(selectedSnippets) - } + case 'download': + return submitBulkDownload(allSnippets.filter(snippet => selected.has(snippet.id))) case 'trash': case 'delete': await applyAndRefresh( allSnippets.filter(snippet => selected.has(snippet.id)), snippet => api.delete({ id: snippet.id, network: snippet.network }), - refreshSnippetsList) + refreshSnippetsList, + bulkFailureReporter(reportFailure, __('remove the selected snippets', 'code-snippets'))) break case undefined: diff --git a/src/js/hooks/useActionFeedback.tsx b/src/js/hooks/useActionFeedback.tsx new file mode 100644 index 000000000..ec33a80a3 --- /dev/null +++ b/src/js/hooks/useActionFeedback.tsx @@ -0,0 +1,68 @@ +import React, { useCallback, useMemo, useState } from 'react' +import { __, sprintf } from '@wordpress/i18n' +import { createContextHook } from '../utils/bootstrap' +import { describeError, handleUnknownError } from '../utils/errors' +import type { PropsWithChildren } from 'react' + +export interface ActionFailure { + id: number + /** What the person was trying to do, already translated. */ + action: string + /** What went wrong, in terms they can act on. */ + detail: string +} + +export interface ActionFeedbackContext { + failures: readonly ActionFailure[] + /** + * Report that an action did not complete. + * + * Every snippet action used to send its error to the console and nothing + * else, so a failed request looked identical to a click that had never + * registered: the row did not change and nothing explained why. That left + * people unable to tell a permissions problem from a plugin conflict, and + * left us unable to ask them anything useful. + */ + reportFailure: (action: string, error: unknown) => void + dismissFailure: (id: number) => void +} + +const [Context, useActionFeedback] = createContextHook('useActionFeedback') + +export const WithActionFeedbackContext: React.FC = ({ children }) => { + const [failures, setFailures] = useState([]) + + const reportFailure = useCallback((action: string, error: unknown) => { + // Still logged, so the full object remains available in the console. + handleUnknownError(error) + + setFailures(current => [ + ...current.filter(failure => failure.action !== action), + { id: Date.now() + current.length, action, detail: describeError(error) } + ]) + }, []) + + const dismissFailure = useCallback((id: number) => { + setFailures(current => current.filter(failure => failure.id !== id)) + }, []) + + const value = useMemo( + () => ({ failures, reportFailure, dismissFailure }), + [failures, reportFailure, dismissFailure] + ) + + return {children} +} + +/** + * Build the sentence shown to the person, given what they were doing. + */ +export const failureMessage = (failure: ActionFailure): string => + sprintf( + /* translators: 1: what the user was trying to do, 2: reason it did not work. */ + __('Could not %1$s. %2$s', 'code-snippets'), + failure.action, + failure.detail + ) + +export { useActionFeedback } diff --git a/src/js/utils/errors.ts b/src/js/utils/errors.ts index 7df075577..dc55562fc 100644 --- a/src/js/utils/errors.ts +++ b/src/js/utils/errors.ts @@ -1,6 +1,10 @@ -import { __ } from '@wordpress/i18n' +import { __, sprintf } from '@wordpress/i18n' import { isAxiosError } from 'axios' +const HTTP_FORBIDDEN = 403 +const HTTP_NOT_FOUND = 404 +const HTTP_SERVER_ERROR = 500 + export const handleUnknownError = (error: unknown) => { console.error(error) } @@ -20,3 +24,58 @@ export const unpackErrorResponse = (error: unknown): string => { return __('An unknown error occurred.', 'code-snippets') } + +/** + * Describe a failed request in terms the person reading it can act on. + * + * The HTTP status is deliberately included. Requests to the snippets API are + * blocked by security rules and firewalls often enough that "it did nothing" + * is impossible to diagnose without it, and asking people to open developer + * tools is a poor substitute for the plugin simply saying what happened. + */ +export const describeError = (error: unknown): string => { + if (isAxiosError(error)) { + if (!error.response) { + return __( + 'The request did not reach your site. It may have been blocked by a firewall or security plugin.', + 'code-snippets' + ) + } + + const status = error.response.status + const message = unpackErrorResponse(error) + + if (HTTP_FORBIDDEN === status) { + return sprintf( + /* translators: %s: error message returned by the site. */ + __('Your site refused the request (403). Try reloading the page, and check whether a security plugin is blocking it. %s', 'code-snippets'), + message + ) + } + + if (HTTP_NOT_FOUND === status) { + return __( + 'The snippets API could not be found (404). It may be disabled or blocked on your site.', + 'code-snippets' + ) + } + + if (HTTP_SERVER_ERROR <= status) { + return sprintf( + /* translators: 1: HTTP status code, 2: error message returned by the site. */ + __('Your site returned an error (%1$d). %2$s', 'code-snippets'), + status, + message + ) + } + + return sprintf( + /* translators: 1: HTTP status code, 2: error message returned by the site. */ + __('Your site returned %1$d. %2$s', 'code-snippets'), + status, + message + ) + } + + return unpackErrorResponse(error) +}