fix(mod queue): keep error details out of visible text

This commit is contained in:
Tommaso Casaburi
2026-05-13 17:54:58 +07:00
parent 9f6c073264
commit a4e058c6ac
7 changed files with 88 additions and 32 deletions
@@ -421,10 +421,25 @@ describe('CommentContent', () => {
});
const errorDisplay = container.querySelector('[data-testid="error-display"]');
expect(errorDisplay?.textContent).toBe('Error');
expect(errorDisplay?.getAttribute('data-display-message')).toBe('Error');
expect(errorDisplay?.textContent).toBe('spam blocker server error');
expect(errorDisplay?.getAttribute('data-display-message')).toBe('spam blocker server error');
expect(errorDisplay?.getAttribute('data-inline')).toBe('true');
expect(errorDisplay?.getAttribute('data-show-immediately')).toBe('true');
expect(container.querySelector('[data-testid="loading-ellipsis"]')).toBeNull();
});
it('falls back to a short label for failed unpublished comments without a message', async () => {
testState.stateString = 'Failed';
await renderContent({
content: 'still pending',
errors: [{ details: { provider: 'gateway', reason: 'timeout' } }],
postCid: 'post-2',
state: 'failed',
});
const errorDisplay = container.querySelector('[data-testid="error-display"]');
expect(errorDisplay?.textContent).toBe('Error');
expect(errorDisplay?.getAttribute('data-display-message')).toBe('Error');
expect(container.textContent).not.toContain('provider: gateway');
});
});
@@ -18,6 +18,7 @@ import Tooltip from '../../components/tooltip';
import styles from '../../views/post/post.module.css';
import capitalize from 'lodash/capitalize';
import { getCommentCommunityAddress, withResolvedCommentCommunityAddress } from '../../lib/utils/comment-utils';
import { formatErrorMessageForDisplay } from '../../lib/utils/error-utils';
const QuotedCidLink = ({ cid, postCid }: { cid: string; postCid: string }) => {
const quotedNumber = usePostNumberStore((state) => state.cidToNumber[cid]);
@@ -131,12 +132,13 @@ const CommentContent = ({ comment: post, prependContent }: { comment: Comment; p
const stateString = useStateString(resolvedPost);
const hasFailedState = state === 'failed';
const failedError = getFailedCommentError(resolvedPost);
const failedErrorMessage = formatErrorMessageForDisplay(failedError);
const shouldShowUnpublishedStateDetails = !cid && (!hasFailedState || Boolean(failedError));
const loadingString = (
<div className={styles.stateString}>
{failedError ? (
<ErrorDisplay error={failedError} displayMessage={capitalize(t('error'))} inline={true} showImmediately={true} />
<ErrorDisplay error={failedError} displayMessage={failedErrorMessage ?? capitalize(t('error'))} inline={true} showImmediately={true} />
) : !hasFailedState ? (
<LoadingEllipsis string={stateString || t('loading')} />
) : (
@@ -67,22 +67,24 @@ describe('ErrorDisplay', () => {
});
const button = container.querySelector('button');
expect(container.textContent).toContain('error: network down: code: 500');
expect(button?.textContent).toBe('copy full error');
expect(container.textContent).toContain('error: network down');
expect(container.textContent).not.toContain('code: 500');
expect(button?.textContent).toBe('Copy full error');
await act(async () => {
button?.dispatchEvent(new MouseEvent('click', { bubbles: true }));
});
expect(testState.copyToClipboardMock).toHaveBeenCalledWith(JSON.stringify(error, null, 2));
expect(container.textContent).toContain('copied');
expect(container.textContent).toContain('Copied');
act(() => {
vi.advanceTimersByTime(1500);
});
expect(container.textContent).toContain('error: network down: code: 500');
expect(button?.textContent).toBe('copy full error');
expect(container.textContent).toContain('error: network down');
expect(container.textContent).not.toContain('code: 500');
expect(button?.textContent).toBe('Copy full error');
});
it('shows copy failure feedback and logs the clipboard error', async () => {
@@ -95,14 +97,14 @@ describe('ErrorDisplay', () => {
const button = container.querySelector('button');
expect(button).toBeTruthy();
expect(button?.textContent).toBe('copy full error');
expect(button?.textContent).toBe('Copy full error');
await act(async () => {
button?.dispatchEvent(new MouseEvent('click', { bubbles: true }));
});
expect(consoleErrorSpy).toHaveBeenCalledWith('Failed to copy error: ', expect.any(Error));
expect(container.textContent).toContain('copy failed');
expect(container.textContent).toContain('Copy failed');
});
it('copies native Error instances with message, stack, and extra fields', async () => {
@@ -118,8 +120,9 @@ describe('ErrorDisplay', () => {
});
const button = container.querySelector('button');
expect(container.textContent).toContain('error: native failure: status: 504');
expect(button?.textContent).toBe('copy full error');
expect(container.textContent).toContain('error: native failure');
expect(container.textContent).not.toContain('status: 504');
expect(button?.textContent).toBe('Copy full error');
await act(async () => {
button?.dispatchEvent(new MouseEvent('click', { bubbles: true }));
@@ -213,14 +216,14 @@ describe('ErrorDisplay', () => {
const button = container.querySelector('button');
expect(container.textContent).toContain('failed');
expect(button?.textContent).toBe('copy full error');
expect(button?.textContent).toBe('Copy full error');
await act(async () => {
button?.dispatchEvent(new MouseEvent('click', { bubbles: true }));
});
expect(testState.copyToClipboardMock).toHaveBeenCalledWith('All pubsub providers throw an error and unable to publish or subscribe');
expect(container.textContent).toContain('copied');
expect(container.textContent).toContain('Copied');
});
it('renders plain string errors after the delay and hides again when the error clears', async () => {
@@ -24,7 +24,6 @@
display: inline-flex;
align-items: baseline;
gap: 1px;
color: var(--button-desktop-text-color);
white-space: nowrap;
}
@@ -1,5 +1,6 @@
import { useReducer, useEffect } from 'react';
import { useTranslation } from 'react-i18next';
import capitalize from 'lodash/capitalize';
import { copyToClipboard } from '../../lib/utils/clipboard-utils';
import { formatErrorForDisplay, serializeErrorForClipboard } from '../../lib/utils/error-utils';
import styles from './error-display.module.css';
@@ -65,6 +66,7 @@ const ErrorDisplay = ({ error, displayMessage, inline = false, showImmediately =
const copyButtonLabel =
state.feedbackMessageKey === 'copied' ? t('copied') : state.feedbackMessageKey === 'failed' ? t('copyFailed', 'copy failed') : t('copyFullError', 'copy full error');
const displayCopyButtonLabel = capitalize(copyButtonLabel);
const copyButtonClassNames = [styles.copyErrorButton];
if (state.feedbackMessageKey === 'copied') {
copyButtonClassNames.push(styles.feedbackSuccessMessage);
@@ -79,7 +81,7 @@ const ErrorDisplay = ({ error, displayMessage, inline = false, showImmediately =
<span className={styles.copyErrorButtonWrapper}>
[
<button type='button' className={copyButtonClassNames.join(' ')} onClick={handleCopyError} title={t('copyFullError', 'copy full error')}>
{copyButtonLabel}
{displayCopyButtonLabel}
</button>
]
</span>
+31 -8
View File
@@ -1,12 +1,12 @@
import { describe, expect, it } from 'vitest';
import { formatErrorForDisplay, serializeErrorForClipboard } from '../error-utils';
import { formatErrorForDisplay, formatErrorMessageForDisplay, serializeErrorForClipboard } from '../error-utils';
describe('error utils', () => {
it('returns plain string errors unchanged', () => {
expect(formatErrorForDisplay('plain failure')).toBe('plain failure');
});
it('appends structured details to the message', () => {
it('uses the message without appending structured details', () => {
expect(
formatErrorForDisplay({
details: {
@@ -15,10 +15,10 @@ describe('error utils', () => {
},
message: 'All pubsub providers throw an error and unable to publish or subscribe',
}),
).toBe('All pubsub providers throw an error and unable to publish or subscribe: plebpubsub: timeout; pubsubprovider: connection refused');
).toBe('All pubsub providers throw an error and unable to publish or subscribe');
});
it('falls back to cause when details are missing', () => {
it('uses the message without appending the cause', () => {
expect(
formatErrorForDisplay({
cause: {
@@ -27,10 +27,10 @@ describe('error utils', () => {
},
message: 'publish failed',
}),
).toBe('publish failed: provider: plebpubsub; reason: timeout');
).toBe('publish failed');
});
it('formats nested Error details without dropping the nested message', () => {
it('keeps the visible message compact when details contain nested errors', () => {
expect(
formatErrorForDisplay({
details: {
@@ -38,7 +38,7 @@ describe('error utils', () => {
},
message: 'publish failed',
}),
).toBe('publish failed: reason: provider timeout');
).toBe('publish failed');
});
it('formats cyclic errors without recursing forever', () => {
@@ -49,7 +49,30 @@ describe('error utils', () => {
});
Object.assign(error.details, { self: error.details });
expect(formatErrorForDisplay(error)).toBe('publish failed: elapsedMs: 5000; self: [Circular]');
expect(formatErrorForDisplay(error)).toBe('publish failed');
});
it('falls back to details when an object has no message', () => {
expect(
formatErrorForDisplay({
details: {
provider: 'gateway',
reason: 'timeout',
},
}),
).toBe('provider: gateway; reason: timeout');
});
it('extracts only the direct message for compact display labels', () => {
expect(
formatErrorMessageForDisplay({
details: {
provider: 'gateway',
},
message: 'gateway failed',
}),
).toBe('gateway failed');
expect(formatErrorMessageForDisplay({ details: { provider: 'gateway' } })).toBeUndefined();
});
it('serializes cyclic errors as valid JSON for copying', () => {
+20 -8
View File
@@ -172,26 +172,38 @@ const normalizeUnknownErrorPart = (value: unknown, seen = new WeakSet<object>())
return undefined;
};
export const formatErrorMessageForDisplay = (error: unknown): string | undefined => {
if (!error) {
return undefined;
}
if (typeof error === 'string') {
return normalizeUnknownErrorPart(error);
}
const { message } = error as ErrorLike;
return normalizeUnknownErrorPart(message);
};
export const formatErrorForDisplay = (error: unknown): string | undefined => {
if (!error) {
return undefined;
}
const normalizedMessage = formatErrorMessageForDisplay(error);
if (normalizedMessage) {
return normalizedMessage;
}
const normalizedString = normalizeUnknownErrorPart(error);
if (typeof error === 'string') {
return normalizedString;
}
const { cause, details, message } = error as ErrorLike;
const normalizedMessage = normalizeUnknownErrorPart(message);
const { cause, details } = error as ErrorLike;
const normalizedDetails = normalizeUnknownErrorPart(details);
const normalizedCause = normalizeUnknownErrorPart(cause);
const detailParts = [normalizedDetails, normalizedCause].filter(Boolean);
if (normalizedMessage && detailParts.length) {
const detailText = detailParts.join('; ');
return normalizedMessage.includes(detailText) ? normalizedMessage : `${normalizedMessage}: ${detailText}`;
}
return normalizedMessage || detailParts[0] || normalizedString;
return detailParts[0] || normalizedString;
};