Skip to content

Commit

Permalink
On Sender Key distribution message failure, don't update send status
Browse files Browse the repository at this point in the history
  • Loading branch information
scottnonnenberg-signal committed Jun 3, 2022
1 parent db523f0 commit a407681
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 13 deletions.
11 changes: 10 additions & 1 deletion ts/models/messages.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1401,7 +1401,13 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
...(this.get('sendStateByConversationId') || {}),
};

const sendIsNotFinal =
'sendIsNotFinal' in result.value && result.value.sendIsNotFinal;
const sendIsFinal = !sendIsNotFinal;

// Capture successful sends
const successfulIdentifiers: Array<string> =
sendIsFinal &&
'successfulIdentifiers' in result.value &&
Array.isArray(result.value.successfulIdentifiers)
? result.value.successfulIdentifiers
Expand Down Expand Up @@ -1435,16 +1441,19 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
}
});

// Integrate sends via sealed sender
const previousUnidentifiedDeliveries =
this.get('unidentifiedDeliveries') || [];
const newUnidentifiedDeliveries =
sendIsFinal &&
'unidentifiedDeliveries' in result.value &&
Array.isArray(result.value.unidentifiedDeliveries)
? result.value.unidentifiedDeliveries
: [];

const promises: Array<Promise<unknown>> = [];

// Process errors
let errors: Array<CustomError>;
if (result.value instanceof SendMessageProtoError && result.value.errors) {
({ errors } = result.value);
Expand All @@ -1467,7 +1476,7 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
window.ConversationController.get(error.identifier) ||
window.ConversationController.get(error.number);

if (conversation && !saveErrors) {
if (conversation && !saveErrors && sendIsFinal) {
const previousSendState = getOwn(
sendStateByConversationId,
conversation.id
Expand Down
4 changes: 4 additions & 0 deletions ts/textsecure/Errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,8 @@ export class SendMessageProtoError extends Error implements CallbackResultType {

public readonly recipients?: Record<string, Array<number>>;

public readonly sendIsNotFinal?: boolean;

constructor({
successfulIdentifiers,
failoverIdentifiers,
Expand All @@ -214,6 +216,7 @@ export class SendMessageProtoError extends Error implements CallbackResultType {
contentProto,
timestamp,
recipients,
sendIsNotFinal,
}: CallbackResultType) {
super(`SendMessageProtoError: ${SendMessageProtoError.getMessage(errors)}`);

Expand All @@ -226,6 +229,7 @@ export class SendMessageProtoError extends Error implements CallbackResultType {
this.contentProto = contentProto;
this.timestamp = timestamp;
this.recipients = recipients;
this.sendIsNotFinal = sendIsNotFinal;
}

protected static getMessage(errors: CallbackResultType['errors']): string {
Expand Down
4 changes: 4 additions & 0 deletions ts/textsecure/Types.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,10 @@ export interface CallbackResultType {
unidentifiedDeliveries?: Array<string>;
dataMessage?: Uint8Array;

// If this send is not the final step in a multi-step send, we shouldn't treat its
// results we would treat a one-step send.
sendIsNotFinal?: boolean;

// Fields necessary for send log save
contentHint?: number;
contentProto?: Uint8Array;
Expand Down
37 changes: 25 additions & 12 deletions ts/util/sendToGroup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -412,18 +412,31 @@ export async function sendToGroupViaSenderKey(options: {
newToMemberUuids.length
} members: ${JSON.stringify(newToMemberUuids)}`
);
await handleMessageSend(
window.textsecure.messaging.sendSenderKeyDistributionMessage(
{
contentHint: ContentHint.RESENDABLE,
distributionId,
groupId,
identifiers: newToMemberUuids,
},
sendOptions ? { ...sendOptions, online: false } : undefined
),
{ messageIds: [], sendType: 'senderKeyDistributionMessage' }
);
try {
await handleMessageSend(
window.textsecure.messaging.sendSenderKeyDistributionMessage(
{
contentHint: ContentHint.RESENDABLE,
distributionId,
groupId,
identifiers: newToMemberUuids,
},
sendOptions ? { ...sendOptions, online: false } : undefined
),
{ messageIds: [], sendType: 'senderKeyDistributionMessage' }
);
} catch (error) {
// If we partially fail to send the sender key distribution message (SKDM), we don't
// want the successful SKDM sends to be considered an overall success.
if (error instanceof SendMessageProtoError) {
throw new SendMessageProtoError({
...error,
sendIsNotFinal: true,
});
}

throw error;
}

// Update memberDevices with new devices
const updatedMemberDevices = [...memberDevices, ...newToMemberDevices];
Expand Down

0 comments on commit a407681

Please sign in to comment.