From 6dd6a64d6c8ed4253d54cc38041f5ec8a534aa26 Mon Sep 17 00:00:00 2001 From: Scott Nonnenberg Date: Fri, 19 Aug 2022 11:35:40 -0700 Subject: [PATCH] ErrorBoundary improvements, StickerCreator logging/resiliency --- sticker-creator/app/stages/UploadStage.tsx | 6 +++++- .../window/phase3-sticker-functions.ts | 15 ++++++++++++--- ts/components/App.tsx | 14 ++++++++++---- ts/components/ErrorBoundary.tsx | 19 +++++++++++++++---- ts/components/conversation/Message.tsx | 5 ++++- ts/groups.ts | 6 +++++- ts/state/roots/createConversationView.tsx | 13 ++++++++++++- ts/state/smart/App.tsx | 8 ++++---- 8 files changed, 67 insertions(+), 19 deletions(-) diff --git a/sticker-creator/app/stages/UploadStage.tsx b/sticker-creator/app/stages/UploadStage.tsx index 8c756007d..b7498b810 100644 --- a/sticker-creator/app/stages/UploadStage.tsx +++ b/sticker-creator/app/stages/UploadStage.tsx @@ -12,6 +12,7 @@ import { Button } from '../../elements/Button'; import { stickersDuck } from '../../store'; import { encryptAndUpload } from '../../util/preload'; import { useI18n } from '../../util/i18n'; +import * as Errors from '../../../ts/types/errors'; const handleCancel = () => { history.push('/add-meta'); @@ -42,7 +43,10 @@ export const UploadStage: React.ComponentType = () => { actions.setPackMeta(packMeta); history.push('/share'); } catch (e) { - window.SignalContext.log.error('Error uploading image:', e); + window.SignalContext.log.error( + 'Error uploading pack:', + Errors.toLogFormat(e) + ); actions.addToast({ key: 'StickerCreator--Toasts--errorUploading', subs: [e.message], diff --git a/sticker-creator/window/phase3-sticker-functions.ts b/sticker-creator/window/phase3-sticker-functions.ts index 53fceea65..b059f782b 100644 --- a/sticker-creator/window/phase3-sticker-functions.ts +++ b/sticker-creator/window/phase3-sticker-functions.ts @@ -185,16 +185,25 @@ window.encryptAndUpload = async ( return s; }); - const coverStickerId = - uniqueStickers.length === stickers.length ? 0 : uniqueStickers.length - 1; + const coverStickerId = 0; const coverStickerData = stickers[coverStickerId]; + + if (!coverStickerData) { + window.SignalContext.log.warn( + 'encryptAndUpload: No coverStickerData with ' + + `index ${coverStickerId} and ${stickers.length} total stickers` + ); + } + const coverSticker = new Proto.StickerPack.Sticker(); coverSticker.id = coverStickerId; - if (coverStickerData.emoji) { + + if (coverStickerData?.emoji && coverSticker) { coverSticker.emoji = coverStickerData.emoji; } else { coverSticker.emoji = ''; } + manifestProto.cover = coverSticker; const encryptedManifest = await encrypt( diff --git a/ts/components/App.tsx b/ts/components/App.tsx index 6c3182684..250bf3f79 100644 --- a/ts/components/App.tsx +++ b/ts/components/App.tsx @@ -10,6 +10,7 @@ import type { ExecuteMenuRoleType } from './TitleBarContainer'; import type { LocaleMessagesType } from '../types/I18N'; import type { MenuOptionsType, MenuActionType } from '../types/menu'; import type { ToastType } from '../state/ducks/toast'; +import type { ViewStoryActionCreatorType } from '../state/ducks/stories'; import { AppViewType } from '../state/ducks/app'; import { Inbox } from './Inbox'; import { SmartInstallScreen } from '../state/smart/InstallScreen'; @@ -28,9 +29,9 @@ type PropsType = { renderCallManager: () => JSX.Element; renderGlobalModalContainer: () => JSX.Element; isShowingStoriesView: boolean; - renderStories: () => JSX.Element; + renderStories: (closeView: () => unknown) => JSX.Element; hasSelectedStoryData: boolean; - renderStoryViewer: () => JSX.Element; + renderStoryViewer: (closeView: () => unknown) => JSX.Element; requestVerification: ( type: 'sms' | 'voice', number: string, @@ -48,6 +49,8 @@ type PropsType = { titleBarDoubleClick: () => void; toastType?: ToastType; hideToast: () => unknown; + toggleStoriesView: () => unknown; + viewStory: ViewStoryActionCreatorType; } & ComponentProps; export const App = ({ @@ -82,6 +85,8 @@ export const App = ({ theme, titleBarDoubleClick, toastType, + toggleStoriesView, + viewStory, }: PropsType): JSX.Element => { let contents; @@ -168,8 +173,9 @@ export const App = ({ {renderGlobalModalContainer()} {renderCallManager()} - {isShowingStoriesView && renderStories()} - {hasSelectedStoryData && renderStoryViewer()} + {isShowingStoriesView && renderStories(toggleStoriesView)} + {hasSelectedStoryData && + renderStoryViewer(() => viewStory({ closeViewer: true }))} {contents} diff --git a/ts/components/ErrorBoundary.tsx b/ts/components/ErrorBoundary.tsx index 2a15f2910..e93a90969 100644 --- a/ts/components/ErrorBoundary.tsx +++ b/ts/components/ErrorBoundary.tsx @@ -1,7 +1,7 @@ // Copyright 2021 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only -import type { ReactNode } from 'react'; +import type { ReactNode, ErrorInfo } from 'react'; import React from 'react'; import * as Errors from '../types/errors'; @@ -10,6 +10,8 @@ import { ToastType } from '../state/ducks/toast'; export type Props = { children: ReactNode; + name: string; + closeView?: () => unknown; }; export type State = { @@ -24,14 +26,23 @@ export class ErrorBoundary extends React.PureComponent { } public static getDerivedStateFromError(error: Error): State { + return { error }; + } + + public override componentDidCatch(error: Error, errorInfo: ErrorInfo): void { + const { closeView, name } = this.props; + log.error( - 'ErrorBoundary: captured rendering error', - Errors.toLogFormat(error) + `ErrorBoundary/${name}: ` + + `captured rendering error ${Errors.toLogFormat(error)}` + + `\nerrorInfo: ${errorInfo.componentStack}` ); if (window.reduxActions) { window.reduxActions.toast.showToast(ToastType.Error); } - return { error }; + if (closeView) { + closeView(); + } } public override render(): ReactNode { diff --git a/ts/components/conversation/Message.tsx b/ts/components/conversation/Message.tsx index 1c1bdd35f..c2f768278 100644 --- a/ts/components/conversation/Message.tsx +++ b/ts/components/conversation/Message.tsx @@ -972,7 +972,10 @@ export class Message extends React.PureComponent { ); } - if (isImage(attachments) || isVideo(attachments)) { + if ( + isImage(attachments) || + (isVideo(attachments) && hasVideoScreenshot(attachments)) + ) { const bottomOverlay = !isSticker && !collapseMetadata; // We only want users to tab into this if there's more than one const tabIndex = attachments.length > 1 ? 0 : -1; diff --git a/ts/groups.ts b/ts/groups.ts index f1fa820ba..347ec5de9 100644 --- a/ts/groups.ts +++ b/ts/groups.ts @@ -4633,13 +4633,17 @@ function extractDiffs({ Boolean(current.expireTimer) && old.expireTimer !== current.expireTimer) ) { + const expireTimer = current.expireTimer || 0; + log.info( + `extractDiffs/${logId}: generating change notifcation for new ${expireTimer} timer` + ); timerNotification = { ...generateBasicMessage(), type: 'timer-notification', sourceUuid, flags: Proto.DataMessage.Flags.EXPIRATION_TIMER_UPDATE, expirationTimerUpdate: { - expireTimer: current.expireTimer || 0, + expireTimer, sourceUuid, }, }; diff --git a/ts/state/roots/createConversationView.tsx b/ts/state/roots/createConversationView.tsx index a484845df..775eeb044 100644 --- a/ts/state/roots/createConversationView.tsx +++ b/ts/state/roots/createConversationView.tsx @@ -5,6 +5,7 @@ import React from 'react'; import { Provider } from 'react-redux'; import type { Store } from 'redux'; +import { ErrorBoundary } from '../../components/ErrorBoundary'; import type { PropsType } from '../smart/ConversationView'; import { SmartConversationView } from '../smart/ConversationView'; @@ -14,6 +15,16 @@ export const createConversationView = ( props: PropsType ): React.ReactElement => ( - + { + window.reduxActions.conversations.showConversation({ + conversationId: undefined, + messageId: undefined, + }); + }} + > + + ); diff --git a/ts/state/smart/App.tsx b/ts/state/smart/App.tsx index 4341b8b55..cf96f4012 100644 --- a/ts/state/smart/App.tsx +++ b/ts/state/smart/App.tsx @@ -51,14 +51,14 @@ const mapStateToProps = (state: StateType) => { renderGlobalModalContainer: () => , renderLeftPane: () => , isShowingStoriesView: shouldShowStoriesView(state), - renderStories: () => ( - + renderStories: (closeView: () => unknown) => ( + ), hasSelectedStoryData: hasSelectedStoryData(state), - renderStoryViewer: () => ( - + renderStoryViewer: (closeView: () => unknown) => ( + ),