Mark attachment as corrupted if audio load failed

Sending corrupted audio should not leave user with non-functional
UI. Mark attachment as corrupted and show generic attachment UI for it
instead.
This commit is contained in:
Fedor Indutny 2021-03-22 11:51:53 -07:00 committed by GitHub
parent d6063d71e5
commit 9fa3359477
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
12 changed files with 98 additions and 5 deletions

View file

@ -111,6 +111,7 @@ const createProps = (overrideProps: Partial<Props> = {}): Props => ({
isTapToViewError: overrideProps.isTapToViewError,
isTapToViewExpired: overrideProps.isTapToViewExpired,
kickOffAttachmentDownload: action('kickOffAttachmentDownload'),
markAttachmentAsCorrupted: action('markAttachmentAsCorrupted'),
openConversation: action('openConversation'),
openLink: action('openLink'),
previews: overrideProps.previews || [],

View file

@ -90,6 +90,7 @@ export type AudioAttachmentProps = {
withContentBelow: boolean;
kickOffAttachmentDownload(): void;
onCorrupted(): void;
};
export type PropsData = {
@ -185,6 +186,10 @@ export type PropsActions = {
attachment: AttachmentType;
messageId: string;
}) => void;
markAttachmentAsCorrupted: (options: {
attachment: AttachmentType;
messageId: string;
}) => void;
showVisualAttachment: (options: {
attachment: AttachmentType;
messageId: string;
@ -686,6 +691,7 @@ export class Message extends React.PureComponent<Props, State> {
i18n,
id,
kickOffAttachmentDownload,
markAttachmentAsCorrupted,
quote,
showVisualAttachment,
isSticker,
@ -773,6 +779,12 @@ export class Message extends React.PureComponent<Props, State> {
messageId: id,
});
},
onCorrupted() {
markAttachmentAsCorrupted({
attachment: firstAttachment,
messageId: id,
});
},
});
}
const { pending, fileName, fileSize, contentType } = firstAttachment;

View file

@ -25,6 +25,7 @@ export type Props = {
buttonRef: React.RefObject<HTMLButtonElement>;
kickOffAttachmentDownload(): void;
onCorrupted(): void;
activeAudioID: string | undefined;
setActiveAudioID: (id: string | undefined) => void;
@ -208,6 +209,7 @@ export const MessageAudio: React.FC<Props> = (props: Props) => {
buttonRef,
kickOffAttachmentDownload,
onCorrupted,
audio,
audioContext,
@ -275,14 +277,27 @@ export const MessageAudio: React.FC<Props> = (props: Props) => {
setPeaks(newPeaks);
setDuration(Math.max(newDuration, 1e-23));
} catch (err) {
window.log.error('MessageAudio: loadAudio error', err);
window.log.error(
'MessageAudio: loadAudio error, marking as corrupted',
err
);
onCorrupted();
}
})();
return () => {
canceled = true;
};
}, [attachment, audioContext, setDuration, setPeaks, state, waveformCache]);
}, [
attachment,
audioContext,
setDuration,
setPeaks,
onCorrupted,
state,
waveformCache,
]);
// This effect attaches/detaches event listeners to the global <audio/>
// instance that we reuse from the GlobalAudioContext.

View file

@ -36,6 +36,7 @@ const defaultMessage: MessageProps = {
isBlocked: false,
isMessageRequestAccepted: true,
kickOffAttachmentDownload: action('kickOffAttachmentDownload'),
markAttachmentAsCorrupted: action('markAttachmentAsCorrupted'),
openConversation: () => null,
openLink: () => null,
previews: [],

View file

@ -39,6 +39,7 @@ const defaultMessageProps: MessagesProps = {
isBlocked: false,
isMessageRequestAccepted: true,
kickOffAttachmentDownload: () => null,
markAttachmentAsCorrupted: () => null,
openConversation: () => null,
openLink: () => null,
previews: [],

View file

@ -235,6 +235,7 @@ const actions = () => ({
showContactDetail: action('showContactDetail'),
showContactModal: action('showContactModal'),
kickOffAttachmentDownload: action('kickOffAttachmentDownload'),
markAttachmentAsCorrupted: action('markAttachmentAsCorrupted'),
showVisualAttachment: action('showVisualAttachment'),
downloadAttachment: action('downloadAttachment'),
displayTapToViewMessage: action('displayTapToViewMessage'),

View file

@ -47,6 +47,7 @@ const getDefaultProps = () => ({
deleteMessage: action('deleteMessage'),
deleteMessageForEveryone: action('deleteMessageForEveryone'),
kickOffAttachmentDownload: action('kickOffAttachmentDownload'),
markAttachmentAsCorrupted: action('markAttachmentAsCorrupted'),
showMessageDetail: action('showMessageDetail'),
openConversation: action('openConversation'),
showContactDetail: action('showContactDetail'),

View file

@ -2967,6 +2967,47 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
return false;
}
markAttachmentAsCorrupted(attachment: AttachmentType): void {
if (!attachment.path) {
throw new Error(
"Attachment can't be marked as corrupted because it wasn't loaded"
);
}
// We intentionally don't check in quotes/stickers/contacts/... here,
// because this function should be called only for something that can
// be displayed as a generic attachment.
const attachments: ReadonlyArray<AttachmentType> =
this.get('attachments') || [];
let changed = false;
const newAttachments = attachments.map(existing => {
if (existing.path !== attachment.path) {
return existing;
}
changed = true;
return {
...existing,
isCorrupted: true,
};
});
if (!changed) {
throw new Error(
"Attachment can't be marked as corrupted because it wasn't found"
);
}
window.log.info(
'markAttachmentAsCorrupted: marking an attachment as corrupted'
);
this.set({
attachments: newAttachments,
});
}
// eslint-disable-next-line class-methods-use-this
async copyFromQuotedMessage(message: WhatIsThis): Promise<boolean> {
const { quote } = message;

View file

@ -25,6 +25,7 @@ export type Props = {
buttonRef: React.RefObject<HTMLButtonElement>;
kickOffAttachmentDownload(): void;
onCorrupted(): void;
};
const mapStateToProps = (state: StateType, props: Props) => {

View file

@ -49,6 +49,7 @@ export type AttachmentType = {
contentType: MIME.MIMEType;
path: string;
};
isCorrupted?: boolean;
};
// UI-focused functions
@ -87,6 +88,7 @@ export function isAudio(
attachments &&
attachments[0] &&
attachments[0].contentType &&
!attachments[0].isCorrupted &&
MIME.isAudio(attachments[0].contentType)
);
}

View file

@ -14658,7 +14658,7 @@
"rule": "React-createRef",
"path": "ts/components/conversation/Message.tsx",
"line": " public focusRef: React.RefObject<HTMLDivElement> = React.createRef();",
"lineNumber": 237,
"lineNumber": 242,
"reasonCategory": "usageTrusted",
"updated": "2021-03-05T19:57:01.431Z",
"reasonDetail": "Used for managing focus only"
@ -14667,7 +14667,7 @@
"rule": "React-createRef",
"path": "ts/components/conversation/Message.tsx",
"line": " public audioButtonRef: React.RefObject<HTMLButtonElement> = React.createRef();",
"lineNumber": 239,
"lineNumber": 244,
"reasonCategory": "usageTrusted",
"updated": "2021-03-05T19:57:01.431Z",
"reasonDetail": "Used for propagating click from the Message to MessageAudio's button"
@ -14676,7 +14676,7 @@
"rule": "React-createRef",
"path": "ts/components/conversation/Message.tsx",
"line": " > = React.createRef();",
"lineNumber": 243,
"lineNumber": 248,
"reasonCategory": "usageTrusted",
"updated": "2021-03-05T19:57:01.431Z",
"reasonDetail": "Used for detecting clicks outside reaction viewer"

View file

@ -9,6 +9,7 @@ import { MediaItemType } from '../components/LightboxGallery';
import { MessageType } from '../state/ducks/conversations';
import { ConversationModel } from '../models/conversations';
import { MessageModel } from '../models/messages';
import { assert } from '../util/assert';
type GetLinkPreviewImageResult = {
data: ArrayBuffer;
@ -27,6 +28,11 @@ type GetLinkPreviewResult = {
date: number | null;
};
type AttachmentOptions = {
messageId: string;
attachment: AttachmentType;
};
const FIVE_MINUTES = 1000 * 60 * 5;
const LINK_PREVIEW_TIMEOUT = 60 * 1000;
@ -756,6 +762,16 @@ Whisper.ConversationView = Whisper.View.extend({
const message = this.model.messageCollection.get(options.messageId);
await message.queueAttachmentDownloads();
};
const markAttachmentAsCorrupted = (options: AttachmentOptions) => {
if (!this.model.messageCollection) {
throw new Error('Message collection does not exist');
}
const message: MessageModel = this.model.messageCollection.get(
options.messageId
);
assert(message, 'Message not found');
message.markAttachmentAsCorrupted(options.attachment);
};
const showVisualAttachment = (options: any) => {
this.showLightbox(options);
};
@ -949,6 +965,7 @@ Whisper.ConversationView = Whisper.View.extend({
downloadAttachment,
downloadNewVersion,
kickOffAttachmentDownload,
markAttachmentAsCorrupted,
loadNewerMessages,
loadNewestMessages: this.loadNewestMessages.bind(this),
loadAndScroll: this.loadAndScroll.bind(this),