Conversation: Prevent getProps errors on initial link

Co-authored-by: Scott Nonnenberg <scott@signal.org> Co-authored-by: Evan Hahn <69474926+EvanHahn-Signal@users.noreply.github.com>
signalapp · Jun 8, 2021 · a9b980d · a9b980d
1 parent 3b13d57
commit a9b980d
Show file tree

Hide file tree

Showing 8 changed files with 51 additions and 36 deletions.
diff --git a/ts/background.ts b/ts/background.ts
@@ -11,6 +11,7 @@ import { DataMessageClass } from './textsecure.d';
 import { MessageAttributesType } from './model-types.d';
 import { WhatIsThis } from './window.d';
 import { getTitleBarVisibility, TitleBarVisibility } from './types/Settings';
+import { DEFAULT_CONVERSATION_COLOR } from './types/Colors';
 import { ChallengeHandler } from './challenge';
 import { isWindowDragElement } from './util/isWindowDragElement';
 import { assert } from './util/assert';
@@ -81,9 +82,10 @@ export async function startApp(): Promise<void> {
 
   window.storage.onready(() => {
     if (!window.storage.get('defaultConversationColor')) {
-      window.storage.put('defaultConversationColor', {
-        color: 'ultramarine',
-      });
+      window.storage.put(
+        'defaultConversationColor',
+        DEFAULT_CONVERSATION_COLOR
+      );
     }
   });
 

diff --git a/ts/model-types.d.ts b/ts/model-types.d.ts
@@ -26,6 +26,7 @@ import { ConversationModel } from './models/conversations';
 import { ProfileNameChangeType } from './util/getStringForProfileChange';
 import { CapabilitiesType } from './textsecure/WebAPI';
 import { GroupNameCollisionsWithIdsByTitle } from './util/groupMemberNameCollisions';
+import { ConversationColorType } from './types/Colors';
 
 export type WhatIsThis = any;
 
@@ -196,7 +197,7 @@ export type ConversationAttributesType = {
   addedBy?: string;
   capabilities?: CapabilitiesType;
   color?: string;
-  conversationColor?: string;
+  conversationColor?: ConversationColorType;
   customColor?: CustomColorType;
   customColorId?: string;
   discoveredUnregisteredAt?: number;

diff --git a/ts/models/conversations.ts b/ts/models/conversations.ts
@@ -25,6 +25,7 @@ import {
   AvatarColors,
   ConversationColorType,
   CustomColorType,
+  DEFAULT_CONVERSATION_COLOR,
 } from '../types/Colors';
 import { MessageModel } from './messages';
 import { isMuted } from '../util/isMuted';
@@ -1379,23 +1380,26 @@ export class ConversationModel extends window.Backbone
     // We don't want to crash or have an infinite loop if we loop back into this function
     //   again. We'll log a warning and returned old cached props or throw an error.
     this.format = () => {
-      const { stack } = new Error('for stack');
-      window.log.warn(
-        `Conversation.format()/${this.idForLogging()} reentrant call! ${stack}`
-      );
       if (!this.oldCachedProps) {
         throw new Error(
           `Conversation.format()/${this.idForLogging()} reentrant call, no old cached props!`
         );
       }
-      return this.oldCachedProps;
-    };
 
-    this.cachedProps = this.getProps();
+      const { stack } = new Error('for stack');
+      window.log.warn(
+        `Conversation.format()/${this.idForLogging()} reentrant call! ${stack}`
+      );
 
-    this.format = oldFormat;
+      return this.oldCachedProps;
+    };
 
-    return this.cachedProps;
+    try {
+      this.cachedProps = this.getProps();
+      return this.cachedProps;
+    } finally {
+      this.format = oldFormat;
+    }
   }
 
   // Note: this should never be called directly. Use conversation.format() instead, which
@@ -4879,7 +4883,8 @@ export class ConversationModel extends window.Backbone
 
   getConversationColor(): ConversationColorType {
     const defaultConversationColor = window.storage.get(
-      'defaultConversationColor'
+      'defaultConversationColor',
+      DEFAULT_CONVERSATION_COLOR
     );
 
     return this.get('conversationColor') || defaultConversationColor.color;
@@ -4890,7 +4895,8 @@ export class ConversationModel extends window.Backbone
     customColorId?: string;
   } {
     const defaultConversationColor = window.storage.get(
-      'defaultConversationColor'
+      'defaultConversationColor',
+      DEFAULT_CONVERSATION_COLOR
     );
 
     if (this.getConversationColor() !== 'custom') {

diff --git a/ts/state/ducks/conversations.ts b/ts/state/ducks/conversations.ts
@@ -26,6 +26,8 @@ import {
   AvatarColorType,
   ConversationColorType,
   CustomColorType,
+  DefaultConversationColorType,
+  DEFAULT_CONVERSATION_COLOR,
 } from '../../types/Colors';
 import { ConversationAttributesType } from '../../model-types.d';
 import { BodyRangeType } from '../../types/Util';
@@ -394,13 +396,7 @@ type CustomColorRemovedActionType = {
   type: typeof CUSTOM_COLOR_REMOVED;
   payload: {
     colorId: string;
-    defaultConversationColor: {
-      color: ConversationColorType;
-      customColorData?: {
-        id: string;
-        value: CustomColorType;
-      };
-    };
+    defaultConversationColor: DefaultConversationColorType;
   };
 };
 type SetPreJoinConversationActionType = {
@@ -765,7 +761,8 @@ function removeCustomColorOnConversations(
     }
 
     const defaultConversationColor = window.storage.get(
-      'defaultConversationColor'
+      'defaultConversationColor',
+      DEFAULT_CONVERSATION_COLOR
     );
 
     dispatch({
@@ -800,7 +797,8 @@ function resetAllChatColors(): ThunkAction<
     });
 
     const defaultConversationColor = window.storage.get(
-      'defaultConversationColor'
+      'defaultConversationColor',
+      DEFAULT_CONVERSATION_COLOR
     );
 
     dispatch({

diff --git a/ts/state/ducks/items.ts b/ts/state/ducks/items.ts
@@ -1,4 +1,4 @@
-// Copyright 2019-2020 Signal Messenger, LLC
+// Copyright 2019-2021 Signal Messenger, LLC
 // SPDX-License-Identifier: AGPL-3.0-only
 
 import { omit } from 'lodash';
@@ -11,6 +11,7 @@ import {
   ConversationColors,
   ConversationColorType,
   CustomColorType,
+  DefaultConversationColorType,
 } from '../../types/Colors';
 import { reloadSelectedConversation } from '../../shims/reloadSelectedConversation';
 
@@ -22,13 +23,7 @@ export type ItemsStateType = {
   readonly [key: string]: unknown;
 
   // This property should always be set and this is ensured in background.ts
-  readonly defaultConversationColor?: {
-    color: ConversationColorType;
-    customColorData?: {
-      id: string;
-      value: CustomColorType;
-    };
-  };
+  readonly defaultConversationColor?: DefaultConversationColorType;
 
   readonly customColors?: {
     readonly colors: Record<string, CustomColorType>;

diff --git a/ts/state/selectors/items.ts b/ts/state/selectors/items.ts
@@ -1,4 +1,4 @@
-// Copyright 2019-2020 Signal Messenger, LLC
+// Copyright 2019-2021 Signal Messenger, LLC
 // SPDX-License-Identifier: AGPL-3.0-only
 
 import { createSelector } from 'reselect';
@@ -8,9 +8,9 @@ import { ITEM_NAME as UNIVERSAL_EXPIRE_TIMER_ITEM } from '../../util/universalEx
 import { StateType } from '../reducer';
 import { ItemsStateType } from '../ducks/items';
 import {
-  ConversationColors,
   ConversationColorType,
   CustomColorType,
+  DEFAULT_CONVERSATION_COLOR,
 } from '../../types/Colors';
 
 export const getItems = (state: StateType): ItemsStateType => state.items;
@@ -41,5 +41,5 @@ export const getDefaultConversationColor = createSelector(
       id: string;
       value: CustomColorType;
     };
-  } => state.defaultConversationColor ?? { color: ConversationColors[0] }
+  } => state.defaultConversationColor ?? DEFAULT_CONVERSATION_COLOR
 );
diff --git a/ts/types/Colors.ts b/ts/types/Colors.ts
@@ -1,4 +1,4 @@
-// Copyright 2020 Signal Messenger, LLC
+// Copyright 2020-2021 Signal Messenger, LLC
 // SPDX-License-Identifier: AGPL-3.0-only
 
 export const AvatarColors = [
@@ -93,3 +93,15 @@ export type AvatarColorType = typeof AvatarColors[number];
 export type ConversationColorType =
   | typeof ConversationColors[number]
   | 'custom';
+
+export type DefaultConversationColorType = {
+  color: ConversationColorType;
+  customColorData?: {
+    id: string;
+    value: CustomColorType;
+  };
+};
+
+export const DEFAULT_CONVERSATION_COLOR: DefaultConversationColorType = {
+  color: 'ultramarine',
+};
diff --git a/ts/window.d.ts b/ts/window.d.ts
@@ -109,6 +109,7 @@ import { SignalProtocolStore } from './SignalProtocolStore';
 import { StartupQueue } from './util/StartupQueue';
 import * as synchronousCrypto from './util/synchronousCrypto';
 import SyncRequest from './textsecure/SyncRequest';
+import { ConversationColorType, CustomColorType } from './types/Colors';
 
 export { Long } from 'long';