From be13311834322d439ddaf8968e360fcd5a1d6b01 Mon Sep 17 00:00:00 2001 From: Tommaso Casaburi Date: Wed, 22 Apr 2026 13:44:55 +0700 Subject: [PATCH] fix(post card): clarify full error copying --- .../__tests__/error-display.test.tsx | 81 +++++++++- .../error-display/error-display.module.css | 31 +++- .../error-display/error-display.tsx | 88 +++-------- src/lib/utils/__tests__/error-utils.test.ts | 53 ++++++- src/lib/utils/error-utils.ts | 141 +++++++++++++++++- 5 files changed, 313 insertions(+), 81 deletions(-) diff --git a/src/components/error-display/__tests__/error-display.test.tsx b/src/components/error-display/__tests__/error-display.test.tsx index 4a215fd0..8e94b37b 100644 --- a/src/components/error-display/__tests__/error-display.test.tsx +++ b/src/components/error-display/__tests__/error-display.test.tsx @@ -52,7 +52,7 @@ describe('ErrorDisplay', () => { vi.useRealTimers(); }); - it('waits before rendering, then copies structured errors and shows feedback', async () => { + it('waits before rendering, then shows a copy action for structured errors', async () => { testState.copyToClipboardMock.mockResolvedValue(undefined); const error = { details: { code: 500 }, @@ -67,20 +67,22 @@ describe('ErrorDisplay', () => { }); const button = container.querySelector('button'); - expect(button?.textContent).toContain('error: network down'); + expect(container.textContent).toContain('error: network down: 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('full error copied to the clipboard'); + expect(container.textContent).toContain('copied'); act(() => { vi.advanceTimersByTime(1500); }); - expect(container.textContent).toContain('error: network down'); + expect(container.textContent).toContain('error: network down: code: 500'); + expect(button?.textContent).toBe('copy full error'); }); it('shows copy failure feedback and logs the clipboard error', async () => { @@ -93,6 +95,7 @@ describe('ErrorDisplay', () => { const button = container.querySelector('button'); expect(button).toBeTruthy(); + expect(button?.textContent).toBe('copy full error'); await act(async () => { button?.dispatchEvent(new MouseEvent('click', { bubbles: true })); @@ -115,7 +118,8 @@ describe('ErrorDisplay', () => { }); const button = container.querySelector('button'); - expect(button?.textContent).toContain('error: native failure'); + expect(container.textContent).toContain('error: native failure: status: 504'); + expect(button?.textContent).toBe('copy full error'); await act(async () => { button?.dispatchEvent(new MouseEvent('click', { bubbles: true })); @@ -135,6 +139,68 @@ describe('ErrorDisplay', () => { ); }); + it('copies nested cyclic errors as readable valid JSON', async () => { + testState.copyToClipboardMock.mockResolvedValue(undefined); + const cause = new Error('provider timeout'); + cause.stack = 'Error: provider timeout\n at provider'; + const error = Object.assign(new Error('publish failed'), { + attempts: [ + { + elapsedMs: BigInt(5000), + provider: 'pubsub', + reason: cause, + }, + ], + }); + Object.assign(error, { self: error }); + error.stack = 'Error: publish failed\n at publish'; + + await renderDisplay(error); + act(() => { + vi.advanceTimersByTime(1000); + }); + + const button = container.querySelector('button'); + await act(async () => { + button?.dispatchEvent(new MouseEvent('click', { bubbles: true })); + }); + + const copied = testState.copyToClipboardMock.mock.calls[0]?.[0]; + expect(copied).toBeTypeOf('string'); + const parsed = JSON.parse(copied as string); + expect(parsed).toMatchObject({ + attempts: [ + { + elapsedMs: '5000', + provider: 'pubsub', + reason: { + message: 'provider timeout', + name: 'Error', + }, + }, + ], + message: 'publish failed', + name: 'Error', + self: '[Circular]', + }); + }); + + it('copies empty structured errors as valid JSON', async () => { + testState.copyToClipboardMock.mockResolvedValue(undefined); + + await renderDisplay({}); + act(() => { + vi.advanceTimersByTime(1000); + }); + + const button = container.querySelector('button'); + await act(async () => { + button?.dispatchEvent(new MouseEvent('click', { bubbles: true })); + }); + + expect(testState.copyToClipboardMock).toHaveBeenCalledWith('{}'); + }); + it('supports compact custom labels that copy plain string errors immediately', async () => { testState.copyToClipboardMock.mockResolvedValue(undefined); @@ -146,14 +212,15 @@ describe('ErrorDisplay', () => { }); const button = container.querySelector('button'); - expect(button?.textContent).toBe('failed'); + expect(container.textContent).toContain('failed'); + 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('full error copied to the clipboard'); + expect(container.textContent).toContain('copied'); }); it('renders plain string errors after the delay and hides again when the error clears', async () => { diff --git a/src/components/error-display/error-display.module.css b/src/components/error-display/error-display.module.css index ba1ebfd7..eb2e48ba 100644 --- a/src/components/error-display/error-display.module.css +++ b/src/components/error-display/error-display.module.css @@ -1,29 +1,50 @@ .error { padding: 10px; text-align: center; + display: flex; + align-items: baseline; + justify-content: center; + gap: 6px; + flex-wrap: wrap; } .inlineError { display: inline-flex; - align-items: center; + align-items: baseline; + gap: 6px; + flex-wrap: wrap; } .errorMessage { color: red; + overflow-wrap: anywhere; } -.clickableErrorMessage { +.copyErrorButtonWrapper { + display: inline-flex; + align-items: baseline; + gap: 1px; + color: var(--button-desktop-text-color); + white-space: nowrap; +} + +.copyErrorButton { all: unset; font: inherit; - color: red; + color: var(--button-desktop-text-color); + text-decoration: var(--button-text-decoration); cursor: pointer; display: inline; } -.clickableErrorMessage:hover { - text-decoration: underline; +.copyErrorButton:hover { + color: var(--button-desktop-text-color-hover); } .feedbackSuccessMessage { color: inherit; } + +.feedbackFailedMessage { + color: red; +} diff --git a/src/components/error-display/error-display.tsx b/src/components/error-display/error-display.tsx index b0172b31..73a580bb 100644 --- a/src/components/error-display/error-display.tsx +++ b/src/components/error-display/error-display.tsx @@ -1,43 +1,21 @@ import { useReducer, useEffect } from 'react'; import { useTranslation } from 'react-i18next'; import { copyToClipboard } from '../../lib/utils/clipboard-utils'; +import { formatErrorForDisplay, serializeErrorForClipboard } from '../../lib/utils/error-utils'; import styles from './error-display.module.css'; -type State = { showAfterDelay: boolean; feedbackMessageKey: string | null }; +type FeedbackMessageKey = 'copied' | 'failed' | null; +type State = { showAfterDelay: boolean; feedbackMessageKey: FeedbackMessageKey }; -function reducer(state: State, action: { type: 'RESET_DELAY' } | { type: 'SHOW' } | { type: 'FEEDBACK'; payload: string | null }): State { +function reducer(state: State, action: { type: 'RESET_DELAY' } | { type: 'SHOW' } | { type: 'FEEDBACK'; payload: FeedbackMessageKey }): State { if (action.type === 'RESET_DELAY') return { ...state, showAfterDelay: false }; if (action.type === 'SHOW') return { ...state, showAfterDelay: true }; if (action.type === 'FEEDBACK') return { ...state, feedbackMessageKey: action.payload }; return state; } -const serializeErrorForClipboard = (error: unknown): string => { - if (typeof error === 'string') { - return error; - } - - const serializableError = - error instanceof Error - ? { - name: error.name, - message: error.message, - stack: error.stack, - ...Object.fromEntries(Object.entries(error)), - ...('cause' in error && error.cause ? { cause: error.cause } : {}), - } - : error; - - try { - const serializedError = JSON.stringify(serializableError, null, 2); - return serializedError && serializedError !== '{}' ? serializedError : String(error); - } catch { - return String(error); - } -}; - type ErrorDisplayProps = { - error: any; + error: unknown; displayMessage?: string; inline?: boolean; showImmediately?: boolean; @@ -47,7 +25,7 @@ const ErrorDisplay = ({ error, displayMessage, inline = false, showImmediately = const { t } = useTranslation(); const [state, dispatch] = useReducer(reducer, { showAfterDelay: showImmediately, feedbackMessageKey: null }); - const hasError = !!(error?.message || error?.stack || error?.details || error); + const hasError = error !== null && error !== undefined && error !== ''; useEffect(() => { if (!hasError) { @@ -66,10 +44,11 @@ const ErrorDisplay = ({ error, displayMessage, inline = false, showImmediately = return null; } - const originalDisplayMessage = displayMessage || (error?.message ? `${t('error')}: ${error.message}` : typeof error === 'string' ? error : error ? t('error') : null); - const canCopyError = !!error && !!originalDisplayMessage; + const formattedError = formatErrorForDisplay(error); + const originalDisplayMessage = displayMessage ?? (formattedError ? (typeof error === 'string' ? formattedError : `${t('error')}: ${formattedError}`) : t('error')); + const canCopyError = hasError && !!originalDisplayMessage; - const handleMessageClick = async () => { + const handleCopyError = async () => { if (!canCopyError || state.feedbackMessageKey) return; const errorString = serializeErrorForClipboard(error); @@ -84,46 +63,27 @@ const ErrorDisplay = ({ error, displayMessage, inline = false, showImmediately = } }; - let currentDisplayMessage = ''; - const classNames: string[] = []; - let isClickable = false; - + const copyButtonLabel = + state.feedbackMessageKey === 'copied' ? t('copied') : state.feedbackMessageKey === 'failed' ? t('copyFailed', 'copy failed') : t('copyFullError', 'copy full error'); + const copyButtonClassNames = [styles.copyErrorButton]; if (state.feedbackMessageKey === 'copied') { - currentDisplayMessage = t('fullErrorCopiedToClipboard', 'full error copied to the clipboard'); - classNames.push(styles.feedbackSuccessMessage); + copyButtonClassNames.push(styles.feedbackSuccessMessage); } else if (state.feedbackMessageKey === 'failed') { - currentDisplayMessage = t('copyFailed', 'copy failed'); - classNames.push(styles.errorMessage); - } else if (originalDisplayMessage) { - currentDisplayMessage = originalDisplayMessage; - classNames.push(styles.errorMessage); - isClickable = canCopyError; - if (isClickable) { - classNames.push(styles.clickableErrorMessage); - } + copyButtonClassNames.push(styles.feedbackFailedMessage); } return (
- {currentDisplayMessage && - (isClickable ? ( - - ) : ( - {currentDisplayMessage} - ))} + ] + + )}
); }; diff --git a/src/lib/utils/__tests__/error-utils.test.ts b/src/lib/utils/__tests__/error-utils.test.ts index f8ed23f2..105c2443 100644 --- a/src/lib/utils/__tests__/error-utils.test.ts +++ b/src/lib/utils/__tests__/error-utils.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { formatErrorForDisplay } from '../error-utils'; +import { formatErrorForDisplay, serializeErrorForClipboard } from '../error-utils'; describe('error utils', () => { it('returns plain string errors unchanged', () => { @@ -29,4 +29,55 @@ describe('error utils', () => { }), ).toBe('publish failed: provider: plebpubsub; reason: timeout'); }); + + it('formats nested Error details without dropping the nested message', () => { + expect( + formatErrorForDisplay({ + details: { + reason: new Error('provider timeout'), + }, + message: 'publish failed', + }), + ).toBe('publish failed: reason: provider timeout'); + }); + + it('formats cyclic errors without recursing forever', () => { + const error = Object.assign(new Error('publish failed'), { + details: { + elapsedMs: BigInt(5000), + }, + }); + Object.assign(error.details, { self: error.details }); + + expect(formatErrorForDisplay(error)).toBe('publish failed: elapsedMs: 5000; self: [Circular]'); + }); + + it('serializes cyclic errors as valid JSON for copying', () => { + const cause = new Error('provider timeout'); + const error = Object.assign(new Error('publish failed'), { + attempts: [{ elapsedMs: BigInt(5000), provider: 'pubsub', reason: cause }], + }); + Object.assign(error, { self: error }); + + expect(JSON.parse(serializeErrorForClipboard(error))).toMatchObject({ + attempts: [ + { + elapsedMs: '5000', + provider: 'pubsub', + reason: { + message: 'provider timeout', + name: 'Error', + }, + }, + ], + message: 'publish failed', + name: 'Error', + self: '[Circular]', + }); + }); + + it('keeps string errors unchanged and empty objects as JSON', () => { + expect(serializeErrorForClipboard('plain failure')).toBe('plain failure'); + expect(serializeErrorForClipboard({})).toBe('{}'); + }); }); diff --git a/src/lib/utils/error-utils.ts b/src/lib/utils/error-utils.ts index 91cc8392..d49ef36e 100644 --- a/src/lib/utils/error-utils.ts +++ b/src/lib/utils/error-utils.ts @@ -4,27 +4,160 @@ type ErrorLike = { message?: unknown; }; -const normalizeUnknownErrorPart = (value: unknown): string | undefined => { +const CIRCULAR_REFERENCE_LABEL = '[Circular]'; +const UNDEFINED_LABEL = '[undefined]'; + +const normalizeErrorForClipboard = (value: unknown, seen = new WeakSet()): unknown => { + if (value === null) { + return null; + } + if (value === undefined) { + return UNDEFINED_LABEL; + } + if (typeof value === 'bigint') { + return value.toString(); + } + if (typeof value === 'number' || typeof value === 'boolean' || typeof value === 'string') { + return value; + } + if (typeof value === 'symbol' || typeof value === 'function') { + return String(value); + } + if (value instanceof Date) { + return Number.isNaN(value.getTime()) ? String(value) : value.toISOString(); + } + if (value instanceof Error) { + if (seen.has(value)) { + return CIRCULAR_REFERENCE_LABEL; + } + seen.add(value); + const normalized: Record = { + name: value.name, + message: value.message, + }; + if (value.stack) { + normalized.stack = value.stack; + } + for (const [key, entryValue] of Object.entries(value)) { + normalized[key] = normalizeErrorForClipboard(entryValue, seen); + } + if ('cause' in value && value.cause !== undefined) { + normalized.cause = normalizeErrorForClipboard(value.cause, seen); + } + seen.delete(value); + return normalized; + } + if (Array.isArray(value)) { + if (seen.has(value)) { + return CIRCULAR_REFERENCE_LABEL; + } + seen.add(value); + const normalized = value.map((entry) => normalizeErrorForClipboard(entry, seen)); + seen.delete(value); + return normalized; + } + if (value instanceof Map) { + if (seen.has(value)) { + return CIRCULAR_REFERENCE_LABEL; + } + seen.add(value); + const normalized = Object.fromEntries([...value.entries()].map(([key, entryValue]) => [String(key), normalizeErrorForClipboard(entryValue, seen)])); + seen.delete(value); + return normalized; + } + if (value instanceof Set) { + if (seen.has(value)) { + return CIRCULAR_REFERENCE_LABEL; + } + seen.add(value); + const normalized = [...value].map((entry) => normalizeErrorForClipboard(entry, seen)); + seen.delete(value); + return normalized; + } + if (typeof value === 'object') { + if (seen.has(value)) { + return CIRCULAR_REFERENCE_LABEL; + } + seen.add(value); + const normalized: Record = {}; + for (const [key, entryValue] of Object.entries(value as Record)) { + normalized[key] = normalizeErrorForClipboard(entryValue, seen); + } + seen.delete(value); + return normalized; + } + return String(value); +}; + +export const serializeErrorForClipboard = (error: unknown): string => { + if (typeof error === 'string') { + return error; + } + + const serializableError = normalizeErrorForClipboard(error); + + try { + return JSON.stringify(serializableError, null, 2) ?? String(error); + } catch { + return String(error); + } +}; + +const normalizeUnknownErrorPart = (value: unknown, seen = new WeakSet()): string | undefined => { if (value === null || value === undefined) { return undefined; } if (typeof value === 'string') { return value.trim() || undefined; } - if (typeof value === 'number' || typeof value === 'boolean') { + if (typeof value === 'number' || typeof value === 'boolean' || typeof value === 'bigint') { return String(value); } + if (typeof value === 'symbol' || typeof value === 'function') { + return String(value); + } + if (value instanceof Error) { + if (seen.has(value)) { + return CIRCULAR_REFERENCE_LABEL; + } + seen.add(value); + const parts = [normalizeUnknownErrorPart(value.message, seen)]; + for (const [key, entryValue] of Object.entries(value)) { + const normalizedValue = normalizeUnknownErrorPart(entryValue, seen); + if (normalizedValue) { + parts.push(`${key}: ${normalizedValue}`); + } + } + if ('cause' in value) { + const normalizedCause = normalizeUnknownErrorPart(value.cause, seen); + if (normalizedCause) { + parts.push(`cause: ${normalizedCause}`); + } + } + seen.delete(value); + return parts.filter(Boolean).join('; ') || value.name; + } if (Array.isArray(value)) { - const parts = value.map(normalizeUnknownErrorPart).filter(Boolean); + if (seen.has(value)) { + return CIRCULAR_REFERENCE_LABEL; + } + seen.add(value); + const parts = value.map((entryValue) => normalizeUnknownErrorPart(entryValue, seen)).filter(Boolean); + seen.delete(value); return parts.length ? parts.join('; ') : undefined; } if (typeof value === 'object') { + if (seen.has(value)) { + return CIRCULAR_REFERENCE_LABEL; + } + seen.add(value); const entries = Object.entries(value as Record) .map(([key, entryValue]) => { - const normalizedValue = normalizeUnknownErrorPart(entryValue); + const normalizedValue = normalizeUnknownErrorPart(entryValue, seen); return normalizedValue ? `${key}: ${normalizedValue}` : undefined; }) .filter(Boolean); + seen.delete(value); if (entries.length) { return entries.join('; ');