Groups: On log fetch error from current revision, use joined_at_version

.github/workflows/benchmark.yml

@@ -65,6 +65,7 @@ jobs:
         NODE_ENV: production
         RUN_COUNT: 10
         ELECTRON_ENABLE_STACK_DUMPING: on
+        DEBUG: 'mock:benchmarks'
         ARTIFACTS_DIR: artifacts/startup
 
     - name: Run send benchmarks
@@ -78,6 +79,7 @@ jobs:
         NODE_ENV: production
         RUN_COUNT: 100
         ELECTRON_ENABLE_STACK_DUMPING: on
+        # DEBUG: 'mock:benchmarks'
         ARTIFACTS_DIR: artifacts/send
 
     - name: Run group send benchmarks
@@ -93,6 +95,7 @@ jobs:
         RUN_COUNT: 100
         CONVERSATION_SIZE: 500
         ELECTRON_ENABLE_STACK_DUMPING: on
+        # DEBUG: 'mock:benchmarks'
         ARTIFACTS_DIR: artifacts/group-send
 
     - name: Run large group send benchmarks with delivery receipts
@@ -112,6 +115,7 @@ jobs:
         RUN_COUNT: 20
         CONVERSATION_SIZE: 50
         ELECTRON_ENABLE_STACK_DUMPING: on
+        # DEBUG: 'mock:benchmarks'
         ARTIFACTS_DIR: artifacts/large-group-send
 
     - name: Run conversation open benchmarks
@@ -126,6 +130,7 @@ jobs:
         NODE_ENV: production
         RUN_COUNT: 100
         ELECTRON_ENABLE_STACK_DUMPING: on
+        # DEBUG: 'mock:benchmarks'
         ARTIFACTS_DIR: artifacts/convo-open
 
     - name: Upload benchmark logs on failure

ts/groups.ts

@@ -751,7 +751,7 @@ export async function buildAddMembersChange(
         addPendingMembers.push(addPendingMemberAction);
       }
 
-      const doesMemberNeedUnban = conversation.bannedMembersV2?.find(
+      const doesMemberNeedUnban = conversation.bannedMembersV2?.some(
         bannedMember => bannedMember.serviceId === serviceId
       );
       if (doesMemberNeedUnban) {
@@ -1020,7 +1020,7 @@ export function _maybeBuildAddBannedMemberActions({
   'addMembersBanned' | 'deleteMembersBanned'
 > {
   const doesMemberNeedBan =
-    !group.bannedMembersV2?.find(member => member.serviceId === serviceId) &&
+    !group.bannedMembersV2?.some(member => member.serviceId === serviceId) &&
     serviceId !== ourAci;
   if (!doesMemberNeedBan) {
     return {};
@@ -1191,7 +1191,7 @@ export function buildAddMember({
   actions.version = (group.revision || 0) + 1;
   actions.addMembers = [addMember];
 
-  const doesMemberNeedUnban = group.bannedMembersV2?.find(
+  const doesMemberNeedUnban = group.bannedMembersV2?.some(
     member => member.serviceId === serviceId
   );
   if (doesMemberNeedUnban) {
@@ -3501,7 +3501,7 @@ async function getGroupUpdates({
   const ourAci = window.storage.user.getCheckedAci();
 
   const isInitialCreationMessage = isFirstFetch && newRevision === 0;
-  const weAreAwaitingApproval = (group.pendingAdminApprovalV2 || []).find(
+  const weAreAwaitingApproval = (group.pendingAdminApprovalV2 || []).some(
     item => item.aci === ourAci
   );
   const isOneVersionUp =
@@ -3565,18 +3565,18 @@ async function getGroupUpdates({
     );
   }
 
-  if (
+  const areWeMember = (group.membersV2 || []).some(item => item.aci === ourAci);
-    (!isFirstFetch || isNumber(newRevision)) &&
+  const isReJoin = !isFirstFetch && !areWeMember;
-    window.Flags.GV2_ENABLE_CHANGE_PROCESSING
+
-  ) {
+  if (window.Flags.GV2_ENABLE_CHANGE_PROCESSING) {
     try {
       return await updateGroupViaLogs({
         group,
         newRevision,
       });
     } catch (error) {
-      const nextStep = isFirstFetch
+      const nextStep = isReJoin
-        ? `fetching logs since ${newRevision}`
+        ? 'attempting to fetch from re-join revision'
         : 'fetching full state';
 
       if (error.code === TEMPORAL_AUTH_REJECTED_CODE) {
@@ -3595,6 +3595,30 @@ async function getGroupUpdates({
     }
   }
 
+  if (isReJoin && window.Flags.GV2_ENABLE_CHANGE_PROCESSING) {
+    try {
+      return await updateGroupViaLogs({
+        group,
+        newRevision,
+        isReJoin,
+      });
+    } catch (error) {
+      if (error.code === TEMPORAL_AUTH_REJECTED_CODE) {
+        // We will fail over to the updateGroupViaState call below
+        log.info(
+          `getGroupUpdates/${logId}: Temporal credential failure, now fetching full state`
+        );
+      } else if (error.code === GROUP_ACCESS_DENIED_CODE) {
+        // We will fail over to the updateGroupViaState call below
+        log.info(
+          `getGroupUpdates/${logId}: Log access denied, now fetching full state`
+        );
+      } else {
+        throw error;
+      }
+    }
+  }
+
   if (window.Flags.GV2_ENABLE_STATE_PROCESSING) {
     try {
       return await updateGroupViaState({
@@ -3887,9 +3911,11 @@ async function updateGroupViaSingleChange({
 async function updateGroupViaLogs({
   group,
   newRevision,
+  isReJoin,
 }: {
   group: ConversationAttributesType;
   newRevision: number | undefined;
+  isReJoin?: boolean;
 }): Promise<UpdatesResultType> {
   const logId = idForLogging(group.groupId);
   const { publicParams, secretParams } = group;
@@ -3900,15 +3926,15 @@ async function updateGroupViaLogs({
     throw new Error('updateGroupViaLogs: group was missing secretParams!');
   }
 
+  const currentRevision = isReJoin ? undefined : group.revision;
+  let includeFirstState = true;
+
   log.info(
     `updateGroupViaLogs/${logId}: Getting group delta from ` +
-      `${group.revision ?? '?'} to ${newRevision ?? '?'} for group ` +
+      `${currentRevision ?? '?'} to ${newRevision ?? '?'} for group ` +
       `groupv2(${group.groupId})...`
   );
 
-  const currentRevision = group.revision;
-  let includeFirstState = true;
-
   // The range is inclusive so make sure that we always request the revision
   // that we are currently at since we might want the latest full state in
   // `integrateGroupChanges`.
@@ -4217,9 +4243,13 @@ async function integrateGroupChange({
 
   const isFirstFetch = !isNumber(group.revision);
   const ourAci = window.storage.user.getCheckedAci();
-  const weAreAwaitingApproval = (group.pendingAdminApprovalV2 || []).find(
+  const weAreAwaitingApproval = (group.pendingAdminApprovalV2 || []).some(
     item => item.aci === ourAci
   );
+  const weAreInGroup = (group.membersV2 || []).some(
+    item => item.aci === ourAci
+  );
+  const isReJoin = !isFirstFetch && !weAreInGroup;
 
   // These need to be populated from the groupChange. But we might not get one!
   let isChangeSupported = false;
@@ -4347,6 +4377,7 @@ async function integrateGroupChange({
         isChangePresent: Boolean(groupChange),
         isChangeSupported,
         isFirstFetch,
+        isReJoin,
         isSameVersion,
         isMoreThanOneVersionUp,
         weAreAwaitingApproval,
@@ -4366,13 +4397,14 @@ async function integrateGroupChange({
     } = await applyGroupState({
       group: attributes,
       groupState: decryptedGroupState,
-      sourceServiceId: isFirstFetch ? sourceServiceId : undefined,
+      sourceServiceId: isFirstFetch || isReJoin ? sourceServiceId : undefined,
     });
 
     const groupChangeMessages = extractDiffs({
       old: attributes,
       current: newAttributes,
-      sourceServiceId: isFirstFetch ? sourceServiceId : undefined,
+      sourceServiceId: isFirstFetch || isReJoin ? sourceServiceId : undefined,
+      isReJoin,
     });
 
     const newProfileKeys = profileKeysToMap(newProfileKeysList);
@@ -4419,15 +4451,17 @@ async function integrateGroupChange({
 function extractDiffs({
   current,
   dropInitialJoinMessage,
+  isReJoin,
   old,
-  sourceServiceId,
   promotedAciToPniMap,
+  sourceServiceId,
 }: {
   current: ConversationAttributesType;
   dropInitialJoinMessage?: boolean;
+  isReJoin?: boolean;
   old: ConversationAttributesType;
-  sourceServiceId?: ServiceIdString;
   promotedAciToPniMap?: ReadonlyMap<AciString, PniString>;
+  sourceServiceId?: ServiceIdString;
 }): Array<GroupChangeMessageType> {
   const logId = idForLogging(old.groupId);
   const details: Array<GroupV2ChangeDetailType> = [];
@@ -4941,11 +4975,11 @@ function extractDiffs({
     timerNotification = {
       ...generateBasicMessage(),
       type: 'timer-notification',
-      sourceServiceId,
+      sourceServiceId: isReJoin ? undefined : sourceServiceId,
       flags: Proto.DataMessage.Flags.EXPIRATION_TIMER_UPDATE,
       expirationTimerUpdate: {
         expireTimer,
-        sourceServiceId,
+        sourceServiceId: isReJoin ? undefined : sourceServiceId,
       },
     };
   }

ts/models/conversations.ts

@@ -264,9 +264,9 @@ export class ConversationModel extends window.Backbone
 
   throttledBumpTyping?: () => void;
 
-  throttledFetchSMSOnlyUUID?: () => Promise<void> | undefined;
+  throttledFetchSMSOnlyUUID?: () => Promise<void>;
 
-  throttledMaybeMigrateV1Group?: () => Promise<void> | undefined;
+  throttledMaybeMigrateV1Group?: () => Promise<void>;
 
   throttledGetProfiles?: () => Promise<void>;
 

ts/state/ducks/conversations.ts

@@ -4297,6 +4297,7 @@ function onConversationOpened(
   | SetQuotedMessageActionType
 > {
   return async (dispatch, getState) => {
+    const promises: Array<Promise<void>> = [];
     const conversation = window.ConversationController.get(conversationId);
     if (!conversation) {
       throw new Error('onConversationOpened: Conversation not found');
@@ -4328,7 +4329,7 @@ function onConversationOpened(
       );
     };
 
-    drop(loadAndUpdate());
+    promises.push(loadAndUpdate());
 
     dispatch(setComposerFocus(conversation.id));
 
@@ -4341,17 +4342,17 @@ function onConversationOpened(
       );
     }
 
-    drop(conversation.fetchLatestGroupV2Data());
+    promises.push(conversation.fetchLatestGroupV2Data());
     strictAssert(
       conversation.throttledMaybeMigrateV1Group !== undefined,
       'Conversation model should be initialized'
     );
-    drop(conversation.throttledMaybeMigrateV1Group());
+    promises.push(conversation.throttledMaybeMigrateV1Group());
     strictAssert(
       conversation.throttledFetchSMSOnlyUUID !== undefined,
       'Conversation model should be initialized'
     );
-    drop(conversation.throttledFetchSMSOnlyUUID());
+    promises.push(conversation.throttledFetchSMSOnlyUUID());
 
     const ourAci = window.textsecure.storage.user.getAci();
     if (
@@ -4367,13 +4368,18 @@ function onConversationOpened(
       });
     }
 
-    drop(conversation.updateVerified());
+    promises.push(conversation.updateVerified());
 
     replaceAttachments(
       conversation.get('id'),
       conversation.get('draftAttachments') || []
     )(dispatch, getState, undefined);
     dispatch(resetComposer(conversationId));
+
+    await Promise.all(promises);
+    if (window.SignalCI) {
+      window.SignalCI.handleEvent('conversationOpenComplete', null);
+    }
   };
 }
 

ts/test-mock/benchmarks/group_send_bench.ts

@@ -21,7 +21,6 @@ import {
 } from './fixtures';
 import { stats } from '../../util/benchmark/stats';
 import { sleep } from '../../util/sleep';
-import { MINUTE } from '../../util/durations';
 import { typeIntoInput } from '../helpers';
 
 const LAST_MESSAGE = 'start sending messages now';
@@ -31,8 +30,9 @@ Bootstrap.benchmark(async (bootstrap: Bootstrap): Promise<void> => {
 
   const members = [...contacts].slice(0, GROUP_SIZE);
 
+  const GROUP_NAME = 'Mock Group';
   const group = await phone.createGroup({
-    title: 'Mock Group',
+    title: GROUP_NAME,
     members: [phone, ...members],
   });
 
@@ -115,10 +115,18 @@ Bootstrap.benchmark(async (bootstrap: Bootstrap): Promise<void> => {
     const item = leftPane
       .locator(
         '.module-conversation-list__item--contact-or-conversation' +
-          `>> text="${LAST_MESSAGE}"`
+          `>> text="${GROUP_NAME}"`
       )
       .first();
-    await item.click({ timeout: 3 * MINUTE });
+    await item.click();
+  }
+
+  debug('finding message in timeline');
+  {
+    const item = window
+      .locator(`.module-message >> text="${LAST_MESSAGE}"`)
+      .first();
+    await item.click();
   }
 
   const deltaList = new Array<number>();

ts/test-mock/playwright.ts

@@ -184,6 +184,10 @@ export class App extends EventEmitter {
     return this.waitForEvent('unlinkCleanupComplete');
   }
 
+  public async waitForConversationOpenComplete(): Promise<void> {
+    return this.waitForEvent('conversationOpenComplete');
+  }
+
   // EventEmitter types
 
   public override on(type: 'close', callback: () => void): this;

ts/test-mock/pnp/accept_gv2_invite_test.ts

@@ -84,6 +84,9 @@ describe('pnp/accept gv2 invite', function (this: Mocha.Suite) {
 
     debug('Opening group');
     await leftPane.locator(`[data-testid="${group.id}"]`).click();
+
+    debug('Wait for conversation open fetches to complete');
+    await app.waitForConversationOpenComplete();
   });
 
   afterEach(async function (this: Mocha.Context) {