Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Mobile: Fixes #10313: Fix error on retry or ignore attachment too large error #10314

Merged
merged 3 commits into from Apr 15, 2024
Merged

Mobile: Fixes #10313: Fix error on retry or ignore attachment too large error #10314

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
95f0891
Mobile: Fixes #10313: Fix error on retry or ignore attachment too large
personalizedrefrigerator Apr 15, 2024
b9ae7ce
Improve commenting
personalizedrefrigerator Apr 15, 2024
2b09698
Also test .retryHandler
personalizedrefrigerator Apr 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
88 changes: 71 additions & 17 deletions packages/lib/services/ReportService.test.ts
View file
Open in desktop
@@ -1,22 +1,25 @@
import SyncTargetRegistry from '../SyncTargetRegistry';
import { _ } from '../locale';
import ReportService, { ReportSection } from './ReportService';
import { createNTestNotes, decryptionWorker, setupDatabaseAndSynchronizer, switchClient, synchronizerStart } from '../testing/test-utils';
import { createNTestNotes, decryptionWorker, setupDatabaseAndSynchronizer, supportDir, switchClient, syncTargetId, synchronizer, synchronizerStart } from '../testing/test-utils';
import Folder from '../models/Folder';
import BaseItem from '../models/BaseItem';
import DecryptionWorker from './DecryptionWorker';
import Note from '../models/Note';
import shim from '../shim';


const getSectionsWithTitle = (report: ReportSection[], title: string) => {
return report.filter(section => section.title === title);
const firstSectionWithTitle = (report: ReportSection[], title: string) => {
const sections = report.filter(section => section.title === title);
if (sections.length === 0) return null;
return sections[0];
};

const getCannotSyncSection = (report: ReportSection[]) => {
return getSectionsWithTitle(report, _('Items that cannot be synchronised'))[0];
return firstSectionWithTitle(report, _('Items that cannot be synchronised'));
};

const getIgnoredSection = (report: ReportSection[]) => {
return getSectionsWithTitle(report, _('Ignored items that cannot be synchronised'))[0];
return firstSectionWithTitle(report, _('Ignored items that cannot be synchronised'));
};

const sectionBodyToText = (section: ReportSection) => {
Expand All @@ -32,8 +35,8 @@ const sectionBodyToText = (section: ReportSection) => {
describe('ReportService', () => {
beforeEach(async () => {
await setupDatabaseAndSynchronizer(1);
await setupDatabaseAndSynchronizer(2);
await switchClient(1);
await synchronizerStart();
// For compatibility with code that calls DecryptionWorker.instance()
DecryptionWorker.instance_ = decryptionWorker();
});
Expand All @@ -43,15 +46,14 @@ describe('ReportService', () => {
const noteCount = 5;
const testNotes = await createNTestNotes(noteCount, folder);
await synchronizerStart();
const syncTargetId = SyncTargetRegistry.nameToId('memory');

const disabledReason = 'Test reason';
for (const testNote of testNotes) {
await BaseItem.saveSyncDisabled(syncTargetId, testNote, disabledReason);
await BaseItem.saveSyncDisabled(syncTargetId(), testNote, disabledReason);
}

const service = new ReportService();
let report = await service.status(syncTargetId);
let report = await service.status(syncTargetId());

// Items should all initially be listed as "cannot be synchronized", but should be ignorable.
const unsyncableSection = getCannotSyncSection(report);
Expand All @@ -65,16 +67,16 @@ describe('ReportService', () => {
expect(sectionBodyToText(unsyncableSection)).toContain(disabledReason);

// Ignore all
expect(await BaseItem.syncDisabledItemsCount(syncTargetId)).toBe(noteCount);
expect(await BaseItem.syncDisabledItemsCountIncludingIgnored(syncTargetId)).toBe(noteCount);
expect(await BaseItem.syncDisabledItemsCount(syncTargetId())).toBe(noteCount);
expect(await BaseItem.syncDisabledItemsCountIncludingIgnored(syncTargetId())).toBe(noteCount);
for (const item of ignorableItems) {
await item.ignoreHandler();
}
expect(await BaseItem.syncDisabledItemsCount(syncTargetId)).toBe(0);
expect(await BaseItem.syncDisabledItemsCountIncludingIgnored(syncTargetId)).toBe(noteCount);
expect(await BaseItem.syncDisabledItemsCount(syncTargetId())).toBe(0);
expect(await BaseItem.syncDisabledItemsCountIncludingIgnored(syncTargetId())).toBe(noteCount);

await synchronizerStart();
report = await service.status(syncTargetId);
report = await service.status(syncTargetId());

// Should now be in the ignored section
const ignoredSection = getIgnoredSection(report);
Expand All @@ -92,7 +94,7 @@ describe('ReportService', () => {
}
}
// Should have the correct number of ignored items
expect(await BaseItem.syncDisabledItemsCountIncludingIgnored(syncTargetId)).toBe(ignoredItemCount);
expect(await BaseItem.syncDisabledItemsCountIncludingIgnored(syncTargetId())).toBe(ignoredItemCount);
expect(ignoredItemCount).toBe(noteCount);

// Clicking "retry" should un-ignore
Expand All @@ -103,6 +105,58 @@ describe('ReportService', () => {
break;
}
}
expect(await BaseItem.syncDisabledItemsCountIncludingIgnored(syncTargetId)).toBe(noteCount - 1);
expect(await BaseItem.syncDisabledItemsCountIncludingIgnored(syncTargetId())).toBe(noteCount - 1);
});

it('should support ignoring sync errors for resources that failed to download', async () => {
const createAttachmentDownloadError = async () => {
await switchClient(2);

const note1 = await Note.save({ title: 'note' });
await shim.attachFileToNote(note1, `${supportDir}/photo.jpg`);
await synchronizerStart();

await switchClient(1);

const previousMax = synchronizer().maxResourceSize_;
synchronizer().maxResourceSize_ = 1;
await synchronizerStart();
synchronizer().maxResourceSize_ = previousMax;
};
await createAttachmentDownloadError();
Comment on lines +112 to +125
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, this logic is roughly copied from Synchronizer.resources.test.ts.


const service = new ReportService();
let report = await service.status(syncTargetId());

const unsyncableSection = getCannotSyncSection(report);
expect(sectionBodyToText(unsyncableSection)).toContain('could not be downloaded');

// Item for the download error should be ignorable
const ignorableItems = [];
for (const item of unsyncableSection.body) {
if (typeof item === 'object' && item.canIgnore) {
ignorableItems.push(item);
}
}
expect(ignorableItems).toHaveLength(1);

await ignorableItems[0].ignoreHandler();

// Should now be ignored.
report = await service.status(syncTargetId());
const ignoredItem = getIgnoredSection(report).body.find(item => typeof item === 'object' && item.canRetry === true);
expect(ignoredItem).not.toBeFalsy();

// Type narrowing
if (typeof ignoredItem === 'string') throw new Error('should be an object');

// Should be possible to retry
await ignoredItem.retryHandler();
await synchronizerStart();

// Should be fixed after retrying
report = await service.status(syncTargetId());
expect(getIgnoredSection(report)).toBeNull();
expect(getCannotSyncSection(report)).toBeNull();
});
});
6 changes: 4 additions & 2 deletions packages/lib/services/ReportService.ts
View file
Open in desktop
Expand Up @@ -194,16 +194,18 @@ export default class ReportService {
msg = _('Item "%s" could not be downloaded: %s', row.syncInfo.item_id, row.syncInfo.sync_disabled_reason);
}

// row.item may be undefined when location !== SYNC_ITEM_LOCATION_LOCAL
const item = { type_: row.syncInfo.item_type, id: row.syncInfo.item_id };
section.body.push({
text: msg,
canRetry: true,
canRetryType: CanRetryType.ItemSync,
retryHandler: async () => {
await BaseItem.saveSyncEnabled(row.item.type_, row.item.id);
await BaseItem.saveSyncEnabled(item.type_, item.id);
},
canIgnore: !row.warning_ignored,
ignoreHandler: async () => {
await BaseItem.ignoreItemSyncWarning(syncTarget, row.item);
await BaseItem.ignoreItemSyncWarning(syncTarget, item);
},
});
};
Expand Down