From 56223905da669357ef6bb39aa106783827a30cc7 Mon Sep 17 00:00:00 2001 From: Jamie Kyle <113370520+jamiebuilds-signal@users.noreply.github.com> Date: Thu, 24 Jul 2025 10:18:29 -0700 Subject: [PATCH] Reduce number of log.error calls Co-authored-by: ayumi-signal Co-authored-by: Scott Nonnenberg --- ts/background.ts | 7 ++- ts/components/CallManager.tsx | 7 +-- ts/components/CallsList.tsx | 16 +++--- ts/components/CompositionInput.tsx | 41 +++++++------- ts/components/DebugLogWindow.tsx | 2 +- ts/components/GroupCallRemoteParticipants.tsx | 5 +- ts/components/I18n.tsx | 10 +--- ts/components/Lightbox.tsx | 5 +- ts/components/MediaEditor.tsx | 16 +++++- ts/components/StoryImage.tsx | 5 +- .../conversation/CallingNotification.tsx | 4 +- ts/components/conversation/Emojify.tsx | 2 +- ts/components/conversation/GIF.tsx | 11 ++-- ts/components/conversation/Message.tsx | 5 +- .../media-gallery/groupMediaItemsByDate.ts | 9 +-- .../fun/FunEmojiLocalizationProvider.tsx | 4 ++ ts/groups.ts | 2 +- ts/logging/set_up_renderer_logging.ts | 15 +---- ts/services/profiles.ts | 22 +++++--- ts/services/storage.ts | 55 ++++++------------- ts/types/errors.ts | 17 ++++++ 21 files changed, 130 insertions(+), 130 deletions(-) diff --git a/ts/background.ts b/ts/background.ts index e5fc5980b1..3a1c933faa 100644 --- a/ts/background.ts +++ b/ts/background.ts @@ -1444,7 +1444,12 @@ export async function startApp(): Promise { async function enableStorageService({ andSync }: { andSync?: string } = {}) { log.info('enableStorageService: waiting for backupReady'); - await backupReady.promise; + try { + await backupReady.promise; + } catch (error) { + log.warn('enableStorageService: backup is not ready; returning early'); + return; + } log.info('enableStorageService: enabling and running'); StorageService.enableStorageService(); diff --git a/ts/components/CallManager.tsx b/ts/components/CallManager.tsx index a1faa0bde9..ee5eaf8567 100644 --- a/ts/components/CallManager.tsx +++ b/ts/components/CallManager.tsx @@ -59,6 +59,7 @@ import { shouldNotify, } from '../types/NotificationProfile'; import type { NotificationProfileType } from '../types/NotificationProfile'; +import { strictAssert } from '../util/assert'; const log = createLogger('CallManager'); @@ -292,11 +293,7 @@ function ActiveCallManager({ }, [callLink]); const handleShareCallLinkViaSignal = useCallback(() => { - if (!callLink) { - log.error('Missing call link'); - return; - } - + strictAssert(callLink != null, 'Missing call link'); showShareCallLinkViaSignal(callLink, i18n); }, [callLink, i18n, showShareCallLinkViaSignal]); diff --git a/ts/components/CallsList.tsx b/ts/components/CallsList.tsx index caae891deb..5f7976dafe 100644 --- a/ts/components/CallsList.tsx +++ b/ts/components/CallsList.tsx @@ -420,15 +420,15 @@ export function CallsList({ return; } - if (isGroupOrAdhocCallMode(callMode)) { - peekQueueArgsRef.current.set(peerId, { - callMode, - conversationId: peerId, - }); - queue.add(peerId); - } else { - log.error(`Trying to peek unsupported call mode ${callMode}`); + if (!isGroupOrAdhocCallMode(callMode)) { + return; } + + peekQueueArgsRef.current.set(peerId, { + callMode, + conversationId: peerId, + }); + queue.add(peerId); }, []); // Get the oldest inserted peerIds by iterating the Set in insertion order. diff --git a/ts/components/CompositionInput.tsx b/ts/components/CompositionInput.tsx index 0ea101d791..ae0a7a5a5f 100644 --- a/ts/components/CompositionInput.tsx +++ b/ts/components/CompositionInput.tsx @@ -60,7 +60,6 @@ import { DirectionalBlot } from '../quill/block/blot'; import { getClassNamesFor } from '../util/getClassNamesFor'; import { isNotNil } from '../util/isNotNil'; import { createLogger } from '../logging/log'; -import * as Errors from '../types/errors'; import type { LinkPreviewForUIType } from '../types/message/LinkPreviews'; import { StagedLinkPreview } from './conversation/StagedLinkPreview'; import type { DraftEditMessageType } from '../model-types.d'; @@ -951,31 +950,33 @@ export function CompositionInput(props: Props): React.ReactElement { const getClassName = getClassNamesFor(BASE_CLASS_NAME, moduleClassName); const onMouseDown = React.useCallback( - (event: MouseEvent) => { - const target = event.target as HTMLElement; - try { - // If the user is actually clicking the format menu, we drop this event - if (target.closest('.module-composition-input__format-menu')) { - return; - } - setIsMouseDown(true); - - const onMouseUp = () => { - setIsMouseDown(false); - window.removeEventListener('mouseup', onMouseUp); - }; - window.addEventListener('mouseup', onMouseUp); - } catch (error) { - log.error( - 'onMouseDown: Failed to check event target', - Errors.toLogFormat(error) - ); + (event: MouseEvent) => { + const { currentTarget } = event; + // If the user is actually clicking the format menu, we drop this event + if (currentTarget.closest('.module-composition-input__format-menu')) { + return; } setIsMouseDown(true); }, [setIsMouseDown] ); + React.useEffect(() => { + if (!isMouseDown) { + return; + } + + function onMouseUp() { + setIsMouseDown(false); + } + + window.addEventListener('mouseup', onMouseUp); + + return () => { + window.removeEventListener('mouseup', onMouseUp); + }; + }, [isMouseDown]); + return ( diff --git a/ts/components/DebugLogWindow.tsx b/ts/components/DebugLogWindow.tsx index 97cdc63f2f..06340104eb 100644 --- a/ts/components/DebugLogWindow.tsx +++ b/ts/components/DebugLogWindow.tsx @@ -98,7 +98,7 @@ export function DebugLogWindow({ const publishedLogURL = await uploadLogs(text); setPublicLogURL(publishedLogURL); } catch (error) { - log.error('error:', Errors.toLogFormat(error)); + log.error('Failed to upload logs:', Errors.toLogFormat(error)); setLoadState(LoadState.Loaded); setToast({ toastType: ToastType.DebugLogError }); } diff --git a/ts/components/GroupCallRemoteParticipants.tsx b/ts/components/GroupCallRemoteParticipants.tsx index df7b78f06e..9063c593fc 100644 --- a/ts/components/GroupCallRemoteParticipants.tsx +++ b/ts/components/GroupCallRemoteParticipants.tsx @@ -16,7 +16,6 @@ import type { import { CallViewMode } from '../types/Calling'; import { useGetCallingFrameBuffer } from '../calling/useGetCallingFrameBuffer'; import type { LocalizerType } from '../types/Util'; -import { toLogFormat } from '../types/errors'; import { usePageVisibility } from '../hooks/usePageVisibility'; import { useDevicePixelRatio } from '../hooks/useDevicePixelRatio'; import { nonRenderedRemoteParticipant } from '../util/ringrtc/nonRenderedRemoteParticipant'; @@ -452,9 +451,7 @@ export function GroupCallRemoteParticipants({ videoRequest = remoteParticipants.map(nonRenderedRemoteParticipant); break; default: - log.error(toLogFormat(missingCaseError(videoRequestMode))); - videoRequest = remoteParticipants.map(nonRenderedRemoteParticipant); - break; + throw missingCaseError(videoRequestMode); } setGroupCallVideoRequest( videoRequest, diff --git a/ts/components/I18n.tsx b/ts/components/I18n.tsx index 12214ebed6..e118c0a3c4 100644 --- a/ts/components/I18n.tsx +++ b/ts/components/I18n.tsx @@ -7,9 +7,7 @@ import type { LocalizerType, ICUJSXMessageParamsByKeyType, } from '../types/Util'; -import { createLogger } from '../logging/log'; - -const log = createLogger('I18n'); +import { strictAssert } from '../util/assert'; export type Props = { /** The translation string id */ @@ -29,11 +27,7 @@ export function I18n({ // Indirection for linter/migration tooling i18n: localizer, }: Props): JSX.Element | null { - if (!id) { - log.error('Error: id prop not provided'); - return null; - } - + strictAssert(id != null, 'Error: id prop not provided'); const intl = localizer.getIntl(); return <>{intl.formatMessage({ id }, components, {})}; } diff --git a/ts/components/Lightbox.tsx b/ts/components/Lightbox.tsx index 5caf59093d..fc19a57f5e 100644 --- a/ts/components/Lightbox.tsx +++ b/ts/components/Lightbox.tsx @@ -34,6 +34,7 @@ import { useReducedMotion } from '../hooks/useReducedMotion'; import { formatFileSize } from '../util/formatFileSize'; import { SECOND } from '../util/durations'; import { Toast } from './Toast'; +import { isAbortError } from '../util/isAbortError'; const log = createLogger('Lightbox'); @@ -310,7 +311,9 @@ export function Lightbox({ if (videoElement.paused) { onMediaPlaybackStart(); void videoElement.play().catch(error => { - log.error('Failed to play video', Errors.toLogFormat(error)); + if (!isAbortError(error)) { + log.error('Failed to play video', Errors.toLogFormat(error)); + } }); } else { videoElement.pause(); diff --git a/ts/components/MediaEditor.tsx b/ts/components/MediaEditor.tsx index 8b337f5f65..dab6ebd0f2 100644 --- a/ts/components/MediaEditor.tsx +++ b/ts/components/MediaEditor.tsx @@ -68,6 +68,7 @@ import { FunStickerPicker } from './fun/FunStickerPicker'; import type { FunStickerSelection } from './fun/panels/FunPanelStickers'; import { drop } from '../util/drop'; import type { FunTimeStickerStyle } from './fun/constants'; +import * as Errors from '../types/errors'; const log = createLogger('MediaEditor'); @@ -360,9 +361,20 @@ export function MediaEditor({ setImageState(newImageState); takeSnapshot('initial state', newImageState, canvas); }; - img.onerror = () => { + img.onerror = ( + event: Event | string, + source?: string, + line?: number, + column?: number, + error?: Error + ) => { // This is a bad experience, but it should be impossible. - log.error(': image failed to load. Closing'); + log.error( + ': image failed to load. Closing', + event, + Errors.toLocation(source, line, column), + Errors.toLogFormat(error) + ); onClose(); }; img.src = imageSrc; diff --git a/ts/components/StoryImage.tsx b/ts/components/StoryImage.tsx index d9ce3c41cd..01dd3cdfbe 100644 --- a/ts/components/StoryImage.tsx +++ b/ts/components/StoryImage.tsx @@ -23,6 +23,7 @@ import { getClassNamesFor } from '../util/getClassNamesFor'; import { isVideoTypeSupported } from '../util/GoogleChrome'; import { createLogger } from '../logging/log'; import * as Errors from '../types/errors'; +import { isAbortError } from '../util/isAbortError'; const log = createLogger('StoryImage'); @@ -80,7 +81,9 @@ export function StoryImage({ } else { onMediaPlaybackStart(); void videoRef.current.play().catch(error => { - log.error('Failed to play video', Errors.toLogFormat(error)); + if (!isAbortError(error)) { + log.error('Failed to play video', Errors.toLogFormat(error)); + } }); } }, [isPaused, onMediaPlaybackStart]); diff --git a/ts/components/conversation/CallingNotification.tsx b/ts/components/conversation/CallingNotification.tsx index 024e777efe..feef90fdd2 100644 --- a/ts/components/conversation/CallingNotification.tsx +++ b/ts/components/conversation/CallingNotification.tsx @@ -17,7 +17,6 @@ import { DirectCallStatus, GroupCallStatus, } from '../../types/CallDisposition'; -import { toLogFormat } from '../../types/errors'; import type { CallingNotificationType } from '../../util/callingNotification'; import { getCallingIcon, @@ -244,8 +243,7 @@ function renderCallingNotificationButton( log.warn('for adhoc call, should never happen'); return null; default: - log.error(toLogFormat(missingCaseError(props.callHistory.mode))); - return null; + throw missingCaseError(props.callHistory.mode); } const disabled = Boolean(disabledTooltipText); diff --git a/ts/components/conversation/Emojify.tsx b/ts/components/conversation/Emojify.tsx index 0a1ad208b3..aa9c3f74c5 100644 --- a/ts/components/conversation/Emojify.tsx +++ b/ts/components/conversation/Emojify.tsx @@ -39,7 +39,7 @@ export function Emojify({ if (type === 'emoji') { // If we don't recognize the emoji, render it as text. if (!isEmojiVariantValue(match)) { - log.error(`Found emoji that we did not recognize: ${match}`); + log.warn('Found emoji that we did not recognize', match.length); return renderNonEmoji({ text: match, key: index }); } diff --git a/ts/components/conversation/GIF.tsx b/ts/components/conversation/GIF.tsx index 1fbb9f9748..2361c560f2 100644 --- a/ts/components/conversation/GIF.tsx +++ b/ts/components/conversation/GIF.tsx @@ -20,6 +20,7 @@ import { useReducedMotion } from '../../hooks/useReducedMotion'; import { AttachmentDetailPill } from './AttachmentDetailPill'; import { getSpinner } from './Image'; import { useUndownloadableMediaHandler } from '../../hooks/useUndownloadableMediaHandler'; +import { isAbortError } from '../../util/isAbortError'; const log = createLogger('GIF'); @@ -96,10 +97,12 @@ export function GIF(props: Props): JSX.Element { if (isPlaying) { video.play().catch(error => { - log.info( - "Failed to match GIF playback to window's state", - Errors.toLogFormat(error) - ); + if (!isAbortError(error)) { + log.error( + "Failed to match GIF playback to window's state", + Errors.toLogFormat(error) + ); + } }); } else { video.pause(); diff --git a/ts/components/conversation/Message.tsx b/ts/components/conversation/Message.tsx index 84d1fc9a31..074be433ad 100644 --- a/ts/components/conversation/Message.tsx +++ b/ts/components/conversation/Message.tsx @@ -47,7 +47,6 @@ import type { import { ReactionViewer } from './ReactionViewer'; import { LinkPreviewDate } from './LinkPreviewDate'; import type { LinkPreviewForUIType } from '../../types/message/LinkPreviews'; -import { toLogFormat } from '../../types/errors'; import { shouldUseFullSizeLinkPreviewImage } from '../../linkPreviews/shouldUseFullSizeLinkPreviewImage'; import type { WidthBreakpoint } from '../_util'; import { OutgoingGiftBadgeModal } from '../OutgoingGiftBadgeModal'; @@ -1108,9 +1107,7 @@ export class Message extends React.PureComponent { isInline = false; break; default: - log.error(toLogFormat(missingCaseError(metadataPlacement))); - isInline = false; - break; + throw missingCaseError(metadataPlacement); } const { diff --git a/ts/components/conversation/media-gallery/groupMediaItemsByDate.ts b/ts/components/conversation/media-gallery/groupMediaItemsByDate.ts index e53d17649b..c2d956493a 100644 --- a/ts/components/conversation/media-gallery/groupMediaItemsByDate.ts +++ b/ts/components/conversation/media-gallery/groupMediaItemsByDate.ts @@ -3,14 +3,8 @@ import moment from 'moment'; import { compact, groupBy, sortBy } from 'lodash'; - -import { createLogger } from '../../../logging/log'; import type { MediaItemType } from '../../../types/MediaItem'; - import { missingCaseError } from '../../../util/missingCaseError'; -import { toLogFormat } from '../../../types/errors'; - -const log = createLogger('groupMediaItemsByDate'); type StaticSectionType = 'today' | 'yesterday' | 'thisWeek' | 'thisMonth'; type YearMonthSectionType = 'yearMonth'; @@ -86,8 +80,7 @@ const toSection = ( mediaItems, }; default: - log.error(toLogFormat(missingCaseError(firstMediaItemWithSection))); - return undefined; + throw missingCaseError(firstMediaItemWithSection); } }; diff --git a/ts/components/fun/FunEmojiLocalizationProvider.tsx b/ts/components/fun/FunEmojiLocalizationProvider.tsx index b5627602a1..aeb70476a1 100644 --- a/ts/components/fun/FunEmojiLocalizationProvider.tsx +++ b/ts/components/fun/FunEmojiLocalizationProvider.tsx @@ -27,6 +27,7 @@ import { } from './useFunEmojiSearch'; import type { LocalizerType } from '../../types/I18N'; import { strictAssert } from '../../util/assert'; +import { isTestOrMockEnvironment } from '../../environment'; const log = createLogger('FunEmojiLocalizationProvider'); @@ -101,6 +102,9 @@ function useLocaleEmojiList(i18n: LocalizerType): LocaleEmojiListType | null { useEffect(() => { let canceled = false; async function run(): Promise { + if (isTestOrMockEnvironment()) { + return; + } try { const list = await window.SignalContext.getLocalizedEmojiList(locale); if (!canceled) { diff --git a/ts/groups.ts b/ts/groups.ts index c4035b14dc..c6ea6e9a99 100644 --- a/ts/groups.ts +++ b/ts/groups.ts @@ -1656,7 +1656,7 @@ export async function modifyGroupV2({ // Fetch credentials only once refreshedCredentials = true; } else if (error.code === 409) { - log.error( + log.warn( `modifyGroupV2/${logId}: Conflict while updating. Timed out; not retrying.` ); // We don't wait here because we're breaking out of the loop immediately. diff --git a/ts/logging/set_up_renderer_logging.ts b/ts/logging/set_up_renderer_logging.ts index 865c8cf283..26bf59769a 100644 --- a/ts/logging/set_up_renderer_logging.ts +++ b/ts/logging/set_up_renderer_logging.ts @@ -48,24 +48,11 @@ export function initialize(): void { setPinoDestination(stream, redactAll); } -function toLocation(source?: string, line?: number, column?: number) { - if (source == null) { - return '(@ unknown)'; - } - if (line != null && column != null) { - return `(@ ${source}:${line}:${column})`; - } - if (line != null) { - return `(@ ${source}:${line})`; - } - return `(@ ${source})`; -} - window.onerror = (message, source, line, column, error) => { const errorInfo = Errors.toLogFormat(error); log.error( `Top-level unhandled error: ${message}, ${errorInfo}`, - toLocation(source, line, column) + Errors.toLocation(source, line, column) ); }; diff --git a/ts/services/profiles.ts b/ts/services/profiles.ts index aa2a4d5666..25ec4b8978 100644 --- a/ts/services/profiles.ts +++ b/ts/services/profiles.ts @@ -43,6 +43,7 @@ import { maybeCreateGroupSendEndorsementState, onFailedToSendWithEndorsements, } from '../util/groupSendEndorsements'; +import { ProfileDecryptError } from '../types/errors'; const log = createLogger('profiles'); @@ -134,17 +135,17 @@ export class ProfileService { await this.fetchProfile(conversation, groupId); resolve(); } catch (error) { - log.error( - `ProfileServices.get: Error was thrown fetching ${conversation.idForLogging()}!`, - Errors.toLogFormat(error) - ); resolve(); if (this.#isPaused) { return; } - if (isRecord(error) && 'code' in error) { + if (error instanceof ProfileDecryptError) { + log.warn( + `ProfileServices.get: Failed to decrypt profile for ${conversation.idForLogging()}` + ); + } else if (isRecord(error) && 'code' in error) { if (error.code === -1) { this.clearAll('Failed to connect to the server'); } else if (error.code === 413 || error.code === 429) { @@ -152,6 +153,11 @@ export class ProfileService { const time = findRetryAfterTimeFromError(error); void this.pause(time); } + } else { + log.error( + `ProfileServices.get: Error was thrown fetching ${conversation.idForLogging()}!`, + Errors.toLogFormat(error) + ); } } finally { this.#jobsByConversationId.delete(conversationId); @@ -546,6 +552,8 @@ async function doGetProfile( }); } } + + return; } // Not Found @@ -558,9 +566,9 @@ async function doGetProfile( log.info(`${logId}: Marking conversation unregistered`); c.setUnregistered(); } - } - return; + return; + } } // throw all unhandled errors diff --git a/ts/services/storage.ts b/ts/services/storage.ts index 7404a4c12c..3dff3a8558 100644 --- a/ts/services/storage.ts +++ b/ts/services/storage.ts @@ -767,11 +767,12 @@ async function generateManifest( ); if (deleteKeys.size !== pendingDeletes.size) { - const localDeletes = Array.from(deleteKeys).map(key => - redactStorageID(key) - ); - const remoteDeletes: Array = []; - pendingDeletes.forEach(id => remoteDeletes.push(redactStorageID(id))); + const localDeletes = Array.from(deleteKeys, key => { + return redactStorageID(key); + }); + const remoteDeletes = Array.from(pendingDeletes, id => { + return redactStorageID(id); + }); log.error( `upload(${version}): delete key sizes do not match`, 'local', @@ -937,8 +938,6 @@ async function uploadManifest( // update conversations with the new storageID postUploadUpdateFunctions.forEach(fn => fn()); } catch (err) { - log.error(`upload(${version}): failed!`, Errors.toLogFormat(err)); - if (err.code === 409) { if (conflictBackOff.isFull()) { log.error(`upload(${version}): exceeded maximum consecutive conflicts`); @@ -954,6 +953,7 @@ async function uploadManifest( throw err; } + log.error(`upload(${version}): failed!`, Errors.toLogFormat(err)); throw err; } @@ -962,14 +962,7 @@ async function uploadManifest( conflictBackOff.reset(); backOff.reset(); - try { - await singleProtoJobQueue.add(MessageSender.getFetchManifestSyncMessage()); - } catch (error) { - log.error( - `upload(${version}): Failed to queue sync message`, - Errors.toLogFormat(error) - ); - } + await singleProtoJobQueue.add(MessageSender.getFetchManifestSyncMessage()); } async function stopStorageServiceSync(reason: Error) { @@ -991,14 +984,7 @@ async function stopStorageServiceSync(reason: Error) { ); return; } - try { - await singleProtoJobQueue.add(MessageSender.getRequestKeySyncMessage()); - } catch (error) { - log.error( - 'stopStorageServiceSync: Failed to queue sync message', - Errors.toLogFormat(error) - ); - } + await singleProtoJobQueue.add(MessageSender.getRequestKeySyncMessage()); }); } @@ -1046,7 +1032,8 @@ async function decryptManifest( async function fetchManifest( manifestVersion: number ): Promise { - log.info('sync: fetch start'); + const logId = `sync(${manifestVersion})`; + log.info(`${logId}: fetch start`); if (!window.textsecure.messaging) { throw new Error('storageService.sync: we are offline!'); @@ -1072,17 +1059,17 @@ async function fetchManifest( } } catch (err) { if (err.code === 204) { - log.info('sync: no newer manifest, ok'); + log.info(`${logId}: no newer manifest, ok`); return undefined; } - log.error('sync: failed!', Errors.toLogFormat(err)); - if (err.code === 404) { + log.info(`${logId}: missing`); await createNewManifest(); return undefined; } + log.error(`${logId}: failed!`, err.code); throw err; } @@ -1690,7 +1677,7 @@ async function fetchRemoteRecords( ); } catch (err) { log.error( - `process(${storageVersion}): Error decrypting storage item`, + `process(${storageVersion}): Error decrypting storage item ${redactStorageID(base64ItemID)}`, Errors.toLogFormat(err) ); await stopStorageServiceSync(err); @@ -2057,11 +2044,12 @@ async function sync({ manifestVersion: version, }); } + + log.info('sync: complete'); } catch (err) { log.error('sync: error processing manifest', Errors.toLogFormat(err)); } - log.info('sync: complete'); return manifest; } @@ -2103,14 +2091,7 @@ async function upload({ return; } - try { - await singleProtoJobQueue.add(MessageSender.getRequestKeySyncMessage()); - } catch (error) { - log.error( - `${logId}: Failed to queue sync message`, - Errors.toLogFormat(error) - ); - } + await singleProtoJobQueue.add(MessageSender.getRequestKeySyncMessage()); return; } diff --git a/ts/types/errors.ts b/ts/types/errors.ts index b0e655787d..2f6b580ca0 100644 --- a/ts/types/errors.ts +++ b/ts/types/errors.ts @@ -18,4 +18,21 @@ export function toLogFormat(error: unknown): string { return result; } +export function toLocation( + source?: string, + line?: number, + column?: number +): string { + if (source == null) { + return '(@ unknown)'; + } + if (line != null && column != null) { + return `(@ ${source}:${line}:${column})`; + } + if (line != null) { + return `(@ ${source}:${line})`; + } + return `(@ ${source})`; +} + export class ProfileDecryptError extends Error {}