mirrors/signal-desktop
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages 2 Wiki Activity Actions

Allow manually retrying attachment downloads

Browse source
This commit is contained in:
Fedor Indutny 2022-05-23 16:07:41 -07:00 committed by GitHub
parent 59b45399e4
commit dfc310805a
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
16 changed files with 265 additions and 58 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
_locales/en/messages.json
View file

@ -919,6 +919,10 @@
"message": "Downloading",
"description": "Shown in the message bubble while a long message attachment is being downloaded"
},
"downloadFullMessage": {
"message": "Download Full Message",
"description": "Shown in the message bubble while a long message attachment is not downloaded"
},
"downloadAttachment": {
"message": "Download Attachment",
"description": "Shown in a message's triple-dot menu if there isn't room for a dedicated download button"

1
stylesheets/components/MessageBody.scss
View file

@ -6,6 +6,7 @@
font-weight: bold;
}
&__download-body,
&__read-more {
@include button-reset;
font-weight: bold;

29
ts/components/conversation/Message.stories.tsx
View file

@ -19,6 +19,7 @@ import {
IMAGE_PNG,
IMAGE_WEBP,
VIDEO_MP4,
LONG_MESSAGE,
stringToMIMEType,
IMAGE_GIF,
} from '../../types/MIME';
@ -205,7 +206,11 @@ const createProps = (overrideProps: Partial<Props> = {}): Props => ({
status: overrideProps.status || 'sent',
text: overrideProps.text || text('text', ''),
textDirection: overrideProps.textDirection || TextDirection.Default,
textPending: boolean('textPending', overrideProps.textPending || false),
textAttachment: overrideProps.textAttachment || {
contentType: LONG_MESSAGE,
size: 123,
pending: boolean('textPending', false),
},
theme: ThemeType.light,
timestamp: number('timestamp', overrideProps.timestamp || Date.now()),
});
@ -420,7 +425,27 @@ story.add('Will expire but still sending', () => {
story.add('Pending', () => {
const props = createProps({
text: 'Hello there from a pal! I am sending a long message so that it will wrap a bit, since I like that look.',
textPending: true,
textAttachment: {
contentType: LONG_MESSAGE,
size: 123,
pending: true,
},
});
return renderBothDirections(props);
});
story.add('Long body can be downloaded', () => {
const props = createProps({
text: 'Hello there from a pal! I am sending a long message so that it will wrap a bit, since I like that look.',
textAttachment: {
contentType: LONG_MESSAGE,
size: 123,
pending: false,
error: true,
digest: 'abc',
key: 'def',
},
});
return renderBothDirections(props);

25
ts/components/conversation/Message.tsx
View file

@ -201,7 +201,7 @@ export type PropsData = {
displayLimit?: number;
text?: string;
textDirection: TextDirection;
textPending?: boolean;
textAttachment?: AttachmentType;
isSticker?: boolean;
isSelected?: boolean;
isSelectedCounter?: number;
@ -818,7 +818,7 @@ export class Message extends React.PureComponent<Props, State> {
status,
i18n,
text,
textPending,
textAttachment,
timestamp,
id,
showMessageDetail,
@ -842,7 +842,7 @@ export class Message extends React.PureComponent<Props, State> {
onWidthMeasured={isInline ? this.updateMetadataWidth : undefined}
showMessageDetail={showMessageDetail}
status={status}
textPending={textPending}
textPending={textAttachment?.pending}
timestamp={timestamp}
/>
);
@ -903,7 +903,7 @@ export class Message extends React.PureComponent<Props, State> {
shouldCollapseBelow,
status,
text,
textPending,
textAttachment,
theme,
timestamp,
} = this.props;
@ -1031,7 +1031,7 @@ export class Message extends React.PureComponent<Props, State> {
played,
showMessageDetail,
status,
textPending,
textPending: textAttachment?.pending,
timestamp,
kickOffAttachmentDownload() {
@ -1206,6 +1206,7 @@ export class Message extends React.PureComponent<Props, State> {
width={72}
url={first.image.url}
attachment={first.image}
blurHash={first.image.blurHash}
onError={this.handleImageError}
i18n={i18n}
onClick={onPreviewImageClick}
@ -1699,10 +1700,11 @@ export class Message extends React.PureComponent<Props, State> {
id,
messageExpanded,
openConversation,
kickOffAttachmentDownload,
status,
text,
textDirection,
textPending,
textAttachment,
} = this.props;
const { metadataWidth } = this.state;
const isRTL = textDirection === TextDirection.RightToLeft;
@ -1741,8 +1743,17 @@ export class Message extends React.PureComponent<Props, State> {
id={id}
messageExpanded={messageExpanded}
openConversation={openConversation}
kickOffBodyDownload={() => {
if (!textAttachment) {
return;
}
kickOffAttachmentDownload({
attachment: textAttachment,
messageId: id,
});
}}
text={contents || ''}
textPending={textPending}
textAttachment={textAttachment}
/>
{!isRTL &&
this.getMetadataPlacement() === MetadataPlacement.InlineWithText && (

8
ts/components/conversation/MessageBody.stories.tsx
View file

@ -25,7 +25,9 @@ const createProps = (overrideProps: Partial<Props> = {}): Props => ({
direction: 'incoming',
i18n,
text: text('text', overrideProps.text || ''),
textPending: boolean('textPending', overrideProps.textPending || false),
textAttachment: overrideProps.textAttachment || {
pending: boolean('textPending', false),
},
});
story.add('Links Enabled', () => {
@ -91,7 +93,9 @@ story.add('Jumbomoji Disabled by Text', () => {
story.add('Text Pending', () => {
const props = createProps({
text: 'Check out https://www.signal.org',
textPending: true,
textAttachment: {
pending: true,
},
});
return <MessageBody {...props} />;

49
ts/components/conversation/MessageBody.tsx
View file

@ -4,6 +4,8 @@
import type { KeyboardEvent } from 'react';
import React from 'react';
import type { AttachmentType } from '../../types/Attachment';
import { canBeDownloaded } from '../../types/Attachment';
import type { SizeClassType } from '../emoji/lib';
import { getSizeClass } from '../emoji/lib';
import { AtMentionify } from './AtMentionify';
@ -25,7 +27,7 @@ type OpenConversationActionType = (
export type Props = {
direction?: 'incoming' | 'outgoing';
text: string;
textPending?: boolean;
textAttachment?: Pick<AttachmentType, 'pending' | 'digest' | 'key'>;
/** If set, all emoji will be the same size. Otherwise, just one emoji will be large. */
disableJumbomoji?: boolean;
/** If set, links will be left alone instead of turned into clickable `<a>` tags. */
@ -34,6 +36,7 @@ export type Props = {
bodyRanges?: BodyRangesType;
onIncreaseTextLength?: () => unknown;
openConversation?: OpenConversationActionType;
kickOffBodyDownload?: () => void;
};
const renderEmoji = ({
@ -71,10 +74,12 @@ export function MessageBody({
onIncreaseTextLength,
openConversation,
text,
textPending,
textAttachment,
kickOffBodyDownload,
}: Props): JSX.Element {
const hasReadMore = Boolean(onIncreaseTextLength);
const textWithSuffix = textPending || hasReadMore ? `${text}...` : text;
const textWithSuffix =
textAttachment?.pending || hasReadMore ? `${text}...` : text;
const sizeClass = disableJumbomoji ? undefined : getSizeClass(text);
const processedText = AtMentionify.preprocessMentions(
@ -103,6 +108,40 @@ export function MessageBody({
);
};
let pendingContent: React.ReactNode;
if (hasReadMore) {
pendingContent = null;
} else if (textAttachment?.pending) {
pendingContent = (
<span className="MessageBody__highlight"> {i18n('downloading')}</span>
);
} else if (
textAttachment &&
canBeDownloaded(textAttachment) &&
kickOffBodyDownload
) {
pendingContent = (
<span>
{' '}
<button
className="MessageBody__download-body"
onClick={() => {
kickOffBodyDownload();
}}
onKeyDown={(ev: KeyboardEvent) => {
if (ev.key === 'Space' || ev.key === 'Enter') {
kickOffBodyDownload();
}
}}
tabIndex={0}
type="button"
>
{i18n('downloadFullMessage')}
</button>
</span>
);
}
return (
<span>
{disableLinks ? (
@ -127,9 +166,7 @@ export function MessageBody({
}}
/>
)}
{textPending ? (
<span className="MessageBody__highlight"> {i18n('downloading')}</span>
) : null}
{pendingContent}
{onIncreaseTextLength ? (
<button
className="MessageBody__read-more"

9
ts/components/conversation/MessageBodyReadMore.tsx
View file

@ -11,11 +11,12 @@ export type Props = Pick<
MessageBodyPropsType,
| 'direction'
| 'text'
| 'textPending'
| 'textAttachment'
| 'disableLinks'
| 'i18n'
| 'bodyRanges'
| 'openConversation'
| 'kickOffBodyDownload'
> & {
id: string;
displayLimit?: number;
@ -39,8 +40,9 @@ export function MessageBodyReadMore({
id,
messageExpanded,
openConversation,
kickOffBodyDownload,
text,
textPending,
textAttachment,
}: Props): JSX.Element {
const maxLength = displayLimit || INITIAL_LENGTH;
@ -64,8 +66,9 @@ export function MessageBodyReadMore({
i18n={i18n}
onIncreaseTextLength={onIncreaseTextLength}
openConversation={openConversation}
kickOffBodyDownload={kickOffBodyDownload}
text={slicedText}
textPending={textPending}
textAttachment={textAttachment}
/>
);
}

128
ts/messageModifiers/AttachmentDownloads.ts
View file

@ -7,6 +7,7 @@ import { v4 as getGuid } from 'uuid';
import dataInterface from '../sql/Client';
import * as durations from '../util/durations';
import { clearTimeoutIfNecessary } from '../util/clearTimeoutIfNecessary';
import { strictAssert } from '../util/assert';
import { downloadAttachment } from '../util/downloadAttachment';
import * as Bytes from '../Bytes';
import type {
@ -16,11 +17,13 @@ import type {
import type { MessageModel } from '../models/messages';
import type { AttachmentType } from '../types/Attachment';
import * as Errors from '../types/errors';
import type { LoggerType } from '../types/Logging';
import * as log from '../logging/log';
const {
getMessageById,
getAttachmentDownloadJobById,
getNextAttachmentDownloadJobs,
removeAttachmentDownloadJob,
resetAttachmentDownloadPending,
@ -91,6 +94,32 @@ export async function addJob(
throw new Error('attachments_download/addJob: index must be a number');
}
if (attachment.downloadJobId) {
let existingJob = await getAttachmentDownloadJobById(
attachment.downloadJobId
);
if (existingJob) {
// Reset job attempts through user's explicit action
existingJob = { ...existingJob, attempts: 0 };
if (_activeAttachmentDownloadJobs[existingJob.id]) {
logger.info(
`attachment_downloads/addJob: ${existingJob.id} already running`
);
} else {
logger.info(
`attachment_downloads/addJob: restarting existing job ${existingJob.id}`
);
_activeAttachmentDownloadJobs[existingJob.id] = _runJob(existingJob);
}
return {
...attachment,
pending: true,
};
}
}
const id = getGuid();
const timestamp = Date.now();
const toSave: AttachmentDownloadJobType = {
@ -175,7 +204,7 @@ async function _maybeStartJob(): Promise<void> {
async function _runJob(job?: AttachmentDownloadJobType): Promise<void> {
if (!job) {
log.warn('_runJob: Job was missing!');
log.warn('attachment_downloads/_runJob: Job was missing!');
return;
}
@ -189,45 +218,60 @@ async function _runJob(job?: AttachmentDownloadJobType): Promise<void> {
);
}
logger.info(`attachment_downloads/_runJob for job id ${id}`);
const found =
window.MessageController.getById(messageId) ||
(await getMessageById(messageId));
if (!found) {
logger.error('_runJob: Source message not found, deleting job');
await _finishJob(null, id);
return;
}
message = window.MessageController.register(found.id, found);
logger.info(`attachment_downloads/_runJob(${id}): starting`);
const pending = true;
await setAttachmentDownloadJobPending(id, pending);
message = window.MessageController.getById(messageId);
if (!message) {
const messageAttributes = await getMessageById(messageId);
if (!messageAttributes) {
logger.error(
`attachment_downloads/_runJob(${id}): ` +
'Source message not found, deleting job'
);
await _finishJob(null, id);
return;
}
strictAssert(messageId === messageAttributes.id, 'message id mismatch');
message = window.MessageController.register(messageId, messageAttributes);
}
await _addAttachmentToMessage(
message,
{ ...attachment, pending: true },
{ type, index }
);
const downloaded = await downloadAttachment(attachment);
if (!downloaded) {
logger.warn(
`_runJob: Got 404 from server for CDN ${
`attachment_downloads/_runJob(${id}): Got 404 from server for CDN ${
attachment.cdnNumber
}, marking attachment ${
attachment.cdnId || attachment.cdnKey
} from message ${message.idForLogging()} as permanent error`
);
await _finishJob(message, id);
await _addAttachmentToMessage(
message,
_markAttachmentAsError(attachment),
_markAttachmentAsPermanentError(attachment),
{ type, index }
);
await _finishJob(message, id);
return;
}
const upgradedAttachment =
await window.Signal.Migrations.processNewAttachment(downloaded);
await _addAttachmentToMessage(message, upgradedAttachment, { type, index });
await _addAttachmentToMessage(message, omit(upgradedAttachment, 'error'), {
type,
index,
});
await _finishJob(message, id);
} catch (error) {
@ -236,25 +280,43 @@ async function _runJob(job?: AttachmentDownloadJobType): Promise<void> {
if (currentAttempt >= 3) {
logger.error(
`_runJob: ${currentAttempt} failed attempts, marking attachment ${id} from message ${logId} as permanent error:`,
error && error.stack ? error.stack : error
`attachment_downloads/runJob(${id}): ${currentAttempt} failed ` +
`attempts, marking attachment from message ${logId} as ` +
'error:',
Errors.toLogFormat(error)
);
await _finishJob(message, id);
await _addAttachmentToMessage(
message,
_markAttachmentAsError(attachment),
_markAttachmentAsTransientError(attachment),
{ type, index }
);
await _finishJob(message, id);
return;
}
logger.error(
`_runJob: Failed to download attachment type ${type} for message ${logId}, attempt ${currentAttempt}:`,
error && error.stack ? error.stack : error
`attachment_downloads/_runJob(${id}): Failed to download attachment ` +
`type ${type} for message ${logId}, attempt ${currentAttempt}:`,
Errors.toLogFormat(error)
);
// Remove `pending` flag from the attachment.
await _addAttachmentToMessage(
message,
{
...attachment,
downloadJobId: id,
},
{ type, index }
);
if (message) {
await saveMessage(message.attributes, {
ourUuid: window.textsecure.storage.user.getCheckedUuid().toString(),
});
}
const failedJob = {
...job,
pending: 0,
@ -289,13 +351,21 @@ function getActiveJobCount(): number {
return Object.keys(_activeAttachmentDownloadJobs).length;
}
function _markAttachmentAsError(attachment: AttachmentType): AttachmentType {
function _markAttachmentAsPermanentError(
attachment: AttachmentType
): AttachmentType {
return {
...omit(attachment, ['key', 'digest', 'id']),
error: true,
};
}
function _markAttachmentAsTransientError(
attachment: AttachmentType
): AttachmentType {
return { ...attachment, error: true };
}
async function _addAttachmentToMessage(
message: MessageModel | null | undefined,
attachment: AttachmentType,
@ -308,13 +378,21 @@ async function _addAttachmentToMessage(
const logPrefix = `${message.idForLogging()} (type: ${type}, index: ${index})`;
if (type === 'long-message') {
// Attachment wasn't downloaded yet.
if (!attachment.path) {
message.set({
bodyAttachment: attachment,
});
return;
}
try {
const { data } = await window.Signal.Migrations.loadAttachmentData(
attachment
);
message.set({
body: attachment.error ? message.get('body') : Bytes.toString(data),
bodyPending: false,
body: Bytes.toString(data),
bodyAttachment: undefined,
});
} finally {
if (attachment.path) {

2
ts/model-types.d.ts vendored
View file

@ -116,7 +116,7 @@ export type MessageReactionType = {
};
export type MessageAttributesType = {
bodyPending?: boolean;
bodyAttachment?: AttachmentType;
bodyRanges?: BodyRangesType;
callHistoryDetails?: CallHistoryDetailsFromDiskType;
changedId?: string;

5
ts/models/conversations.ts
View file

@ -3833,6 +3833,11 @@ export class ConversationModel extends window.Backbone
contentType,
width,
height,
blurHash: await window.imageToBlurHash(
new Blob([data], {
type: IMAGE_JPEG,
})
),
},
};

4
ts/sql/Client.ts
View file

@ -254,6 +254,7 @@ const dataInterface: ClientInterface = {
removeUnprocessed,
removeAllUnprocessed,
getAttachmentDownloadJobById,
getNextAttachmentDownloadJobs,
saveAttachmentDownloadJob,
resetAttachmentDownloadPending,
@ -1476,6 +1477,9 @@ async function removeAllUnprocessed() {
// Attachment downloads
async function getAttachmentDownloadJobById(id: string) {
return channels.getAttachmentDownloadJobById(id);
}
async function getNextAttachmentDownloadJobs(
limit?: number,
options?: { timestamp?: number }

3
ts/sql/Interface.ts
View file

@ -496,6 +496,9 @@ export type DataInterface = {
removeUnprocessed: (id: string | Array<string>) => Promise<void>;
removeAllUnprocessed: () => Promise<void>;
getAttachmentDownloadJobById: (
id: string
) => Promise<AttachmentDownloadJobType | undefined>;
getNextAttachmentDownloadJobs: (
limit?: number,
options?: { timestamp?: number }

6
ts/sql/Server.ts
View file

@ -251,6 +251,7 @@ const dataInterface: ServerInterface = {
removeUnprocessed,
removeAllUnprocessed,
getAttachmentDownloadJobById,
getNextAttachmentDownloadJobs,
saveAttachmentDownloadJob,
resetAttachmentDownloadPending,
@ -3299,6 +3300,11 @@ async function removeAllUnprocessed(): Promise<void> {
// Attachment Downloads
const ATTACHMENT_DOWNLOADS_TABLE = 'attachment_downloads';
async function getAttachmentDownloadJobById(
id: string
): Promise<AttachmentDownloadJobType | undefined> {
return getById(getInstance(), ATTACHMENT_DOWNLOADS_TABLE, id);
}
async function getNextAttachmentDownloadJobs(
limit?: number,
options: { timestamp?: number } = {}

23
ts/state/selectors/message.ts
View file

@ -39,7 +39,7 @@ import type { LinkPreviewType } from '../../types/message/LinkPreviews';
import { CallMode } from '../../types/Calling';
import { SignalService as Proto } from '../../protobuf';
import type { AttachmentType } from '../../types/Attachment';
import { isVoiceMessage } from '../../types/Attachment';
import { isVoiceMessage, canBeDownloaded } from '../../types/Attachment';
import { ReadStatus } from '../../messages/MessageReadStatus';
import type { CallingNotificationType } from '../../util/callingNotification';
@ -265,7 +265,7 @@ export const getAttachmentsForMessage = createSelectorCreator(memoizeByRoot)(
}
return attachments
.filter(attachment => !attachment.error)
.filter(attachment => !attachment.error || canBeDownloaded(attachment))
.map(attachment => getPropsForAttachment(attachment))
.filter(isNotNil);
}
@ -358,7 +358,7 @@ export const getPreviewsForMessage = createSelectorCreator(memoizeByRoot)(
...preview,
isStickerPack: isStickerPack(preview.url),
domain: getDomain(preview.url),
image: preview.image ? getPropsForAttachment(preview.image) : null,
image: preview.image ? getPropsForAttachment(preview.image) : undefined,
}));
}
);
@ -594,7 +594,6 @@ type ShallowPropsType = Pick<
| 'status'
| 'text'
| 'textDirection'
| 'textPending'
| 'timestamp'
>;
@ -682,7 +681,6 @@ const getShallowPropsForMessage = createSelectorCreator(memoizeByRoot, isEqual)(
status: getMessagePropStatus(message, ourConversationId),
text: message.body,
textDirection: getTextDirection(message.body),
textPending: message.bodyPending,
timestamp: message.sent_at,
};
},
@ -690,6 +688,14 @@ const getShallowPropsForMessage = createSelectorCreator(memoizeByRoot, isEqual)(
(_: unknown, props: ShallowPropsType) => props
);
function getTextAttachment(
message: MessageWithUIFieldsType
): AttachmentType | undefined {
return (
message.bodyAttachment && getPropsForAttachment(message.bodyAttachment)
);
}
function getTextDirection(body?: string): TextDirection {
if (!body) {
return TextDirection.None;
@ -727,6 +733,7 @@ export const getPropsForMessage: (
getReactionsForMessage,
getPropsForQuote,
getPropsForStoryReplyContext,
getTextAttachment,
getShallowPropsForMessage,
(
_,
@ -737,6 +744,7 @@ export const getPropsForMessage: (
reactions: PropsData['reactions'],
quote: PropsData['quote'],
storyReplyContext: PropsData['storyReplyContext'],
textAttachment: PropsData['textAttachment'],
shallowProps: ShallowPropsType
): Omit<PropsForMessage, 'renderingContext'> => {
return {
@ -747,6 +755,7 @@ export const getPropsForMessage: (
quote,
reactions,
storyReplyContext,
textAttachment,
...shallowProps,
};
}
@ -1468,9 +1477,9 @@ export function getPropsForEmbeddedContact(
export function getPropsForAttachment(
attachment: AttachmentType
): AttachmentType | null {
): AttachmentType | undefined {
if (!attachment) {
return null;
return undefined;
}
const { path, pending, size, screenshot, thumbnail } = attachment;

10
ts/types/Attachment.ts
View file

@ -73,6 +73,10 @@ export type AttachmentType = {
/** Legacy field. Used only for downloading old attachments */
id?: number;
/** Removed once we download the attachment */
digest?: string;
key?: string;
};
export enum TextAttachmentStyleType {
@ -1018,3 +1022,9 @@ export const defaultBlurHash = (theme: ThemeType = ThemeType.light): string => {
}
return 'L1Q]+w-;fQ-;~qfQfQfQfQfQfQfQ';
};
export const canBeDownloaded = (
attachment: Pick<AttachmentType, 'key' | 'digest'>
): boolean => {
return Boolean(attachment.key && attachment.digest);
};

17
ts/util/queueAttachmentDownloads.ts
View file

@ -22,7 +22,7 @@ import {
import dataInterface from '../sql/Client';
type ReturnType = {
bodyPending?: boolean;
bodyAttachment?: AttachmentType;
attachments: Array<AttachmentType>;
preview: PreviewMessageType;
contact: Array<EmbeddedContactType>;
@ -41,7 +41,7 @@ export async function queueAttachmentDownloads(
const idForLogging = getMessageIdForLogging(message);
let count = 0;
let bodyPending;
let bodyAttachment;
log.info(
`Queueing ${attachmentsToQueue.length} attachment downloads for message ${idForLogging}`
@ -64,8 +64,15 @@ export async function queueAttachmentDownloads(
if (longMessageAttachments.length > 0) {
count += 1;
bodyPending = true;
await AttachmentDownloads.addJob(longMessageAttachments[0], {
[bodyAttachment] = longMessageAttachments;
}
if (!bodyAttachment && message.bodyAttachment) {
count += 1;
bodyAttachment = message.bodyAttachment;
}
if (bodyAttachment) {
await AttachmentDownloads.addJob(bodyAttachment, {
messageId,
type: 'long-message',
index: 0,
@ -252,7 +259,7 @@ export async function queueAttachmentDownloads(
}
return {
bodyPending,
bodyAttachment,
attachments,
preview,
contact,