Skip to content

Commit

Permalink
fix: Summarizing link errors instead of printing dedicated error
Browse files Browse the repository at this point in the history
  • Loading branch information
@steilerDev
steilerDev committed Aug 24, 2023
1 parent cb5fff5 commit a938c7c
Show file tree
Hide file tree
Showing 9 changed files with 120 additions and 25 deletions.
21 changes: 19 additions & 2 deletions app/src/app/event/cli.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {SingleBar} from 'cli-progress';
import {Resources} from '../../lib/resources/main.js';
import {iCPSEventApp, iCPSEventArchiveEngine, iCPSEventCloud, iCPSEventError, iCPSEventLog, iCPSEventMFA, iCPSEventPhotos, iCPSEventSyncEngine} from '../../lib/resources/events-types.js';
import {MFAMethod} from '../../lib/icloud/mfa/mfa-method.js';
import { iCPSError } from '../error/error.js';

Check warning on line 7 in app/src/app/event/cli.ts

View workflow job for this annotation

GitHub Actions / ci / app-build 

'iCPSError' is defined but never used. Allowed unused vars must match /^_/u

/**
* This class handles the input/output to the command line
Expand All @@ -19,6 +20,11 @@ export class CLIInterface {
*/
writeErrors: number = 0;

/**
* Keeps track of album link errors during sync to provide a summary at the end
*/
linkErrors: number = 0;

/**
* Creates a new CLI interface based on the provided components
* @param options - Parsed CLI Options
Expand Down Expand Up @@ -156,16 +162,27 @@ export class CLIInterface {
})
.on(iCPSEventSyncEngine.WRITE_ASSETS_COMPLETED, () => {
this.progressBar.stop();
this.print(chalk.greenBright(`Successfully synced assets!`));

this.print(chalk.greenBright(`Asset sync completed!`));
if (this.writeErrors > 0) {
this.print(`Detected ${this.writeErrors} errors while adding assets, please check the logs for more details.`);
return;
}
this.print(chalk.greenBright(`Successfully synced assets without errors!`));
})
.on(iCPSEventSyncEngine.WRITE_ALBUMS, (toBeDeletedCount: number, toBeAddedCount: number, toBeKept: number) => {
this.print(chalk.cyan(`Syncing albums, by keeping ${toBeKept} and removing ${toBeDeletedCount} local albums, as well as adding ${toBeAddedCount} remote albums...`));
this.linkErrors = 0;
})
.on(iCPSEventSyncEngine.LINK_ERROR, (_assetUUID: string, _albumName: string) => {
this.linkErrors++;
})
.on(iCPSEventSyncEngine.WRITE_ALBUMS_COMPLETED, () => {
this.print(chalk.greenBright(`Successfully synced albums!`));
this.print(chalk.greenBright(`Album sync completed!`));
if (this.linkErrors > 0) {
this.print(`Detected ${this.linkErrors} errors while linking album assets, please check the logs for more details.`);
}
this.print(chalk.greenBright(`Successfully synced albums without errors!`));
})
.on(iCPSEventSyncEngine.WRITE_COMPLETED, () => {
this.print(chalk.green(`Successfully wrote diff to disk!`));
Expand Down
6 changes: 6 additions & 0 deletions app/src/app/event/metrics-exporter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ const FIELDS = {
ALBUMS_TO_BE_ADDED: `albums_to_be_added`,
ALBUMS_TO_BE_KEPT: `albums_to_be_kept`,
ALBUMS_TO_BE_DELETED: `albums_to_be_deleted`,
LINK_ERROR: `link_error`,
ERROR: `errors`,
WARNING: `warnings`,
STATUS_TIME: `status_time`,
Expand Down Expand Up @@ -396,6 +397,11 @@ export class MetricsExporter {
.addField(FIELDS.ALBUMS_TO_BE_KEPT, toBeKept),
);
})
.on(iCPSEventSyncEngine.LINK_ERROR, (_assetUUID: string, _albumName: string) => {
this.logDataPoint(new iCPSInfluxLineProtocolPoint()
.addField(FIELDS.LINK_ERROR, `${_assetUUID} -> ${_albumName}`)
);
})
.on(iCPSEventSyncEngine.WRITE_ALBUMS_COMPLETED, () => {
this.logDataPoint(new iCPSInfluxLineProtocolPoint()
.logStatus(FIELDS.STATUS.values.WRITE_ALBUMS_COMPLETED));
Expand Down
3 changes: 1 addition & 2 deletions app/src/lib/icloud/icloud-photos/icloud-photos.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -549,15 +549,14 @@ export class iCloudPhotos {
}

/**
* Downloads an asset using the 'stream' method
* Downloads an asset using the 'stream' method and applies relevant metadata to the file
* @param asset - The asset to be downloaded
* @returns A promise, that resolves, once the asset has been written to disk
*/
async downloadAsset(asset: Asset): Promise<void> {
const location = asset.getAssetFilePath();
await Resources.network().downloadData(asset.downloadURL, location);
await fs.utimes(location, new Date(asset.modified), new Date(asset.modified)); // Setting modified date on file
await asset.verify();
}

/**
Expand Down
14 changes: 7 additions & 7 deletions app/src/lib/photos-library/photos-library.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {iCPSError} from '../../app/error/error.js';
import {LIBRARY_ERR} from '../../app/error/error-codes.js';
import {Zones} from '../icloud/icloud-photos/query-builder.js';
import {Resources} from '../resources/main.js';
import {iCPSEventError} from '../resources/events-types.js';
import {iCPSEventError, iCPSEventSyncEngine} from '../resources/events-types.js';

type PathTuple = [namePath: string, uuidPath: string]

Expand Down Expand Up @@ -416,12 +416,12 @@ export class PhotosLibrary {
fs.symlinkSync(relativeAssetPath, linkedAsset);
fs.lutimesSync(linkedAsset, assetTime, assetTime);
} catch (err) {
Resources.emit(iCPSEventError.HANDLER_EVENT,
new iCPSError(LIBRARY_ERR.LINK)
.setWarning()
.addMessage(`${relativeAssetPath} to ${linkedAsset}`)
.addCause(err),
);
const linkErr = new iCPSError(LIBRARY_ERR.LINK)
.setWarning()
.addMessage(`${relativeAssetPath} to ${linkedAsset}`)
.addCause(err);
Resources.emit(iCPSEventSyncEngine.LINK_ERROR, assetUUID, album.getDisplayName());
Resources.logger(this).info(`Unable to link ${relativeAssetPath} to ${linkedAsset}: ${linkErr.getDescription()}`);
}
});
}
Expand Down
6 changes: 5 additions & 1 deletion app/src/lib/resources/events-types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,7 @@ export enum iCPSEventSyncEngine {
*/
WRITE_ASSET_COMPLETED = `write-asset-completed`,
/**
* Emitted when the write process has experienced an error writing an asset - provides the asset name as argument
* Emitted when the write process has experienced an error while verifying a written asset - provides the asset name as argument
*/
WRITE_ASSET_ERROR = `write-asset-error`,
/**
Expand All @@ -165,6 +165,10 @@ export enum iCPSEventSyncEngine {
* Emitted when the write process has started writing albums - provides the number of albums to be deleted, added and kept as arguments
*/
WRITE_ALBUMS = `write-albums`,
/**
* Emitted when an asset cannot be linked to an album - provides the asset UUID and album name as arguments
*/
LINK_ERROR = `link-error`,
/**
* Emitted when the write process has completed writing all albums
*/
Expand Down
1 change: 0 additions & 1 deletion app/src/lib/resources/network-manager.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -443,7 +443,6 @@ export class NetworkManager {
* @param location - The location to write the file to (existing files will be overwritten)
*/
async downloadData(url: string, location: string): Promise<void> {
Resources.logger(this).debug(`Adding ${url} to download queue`);
await this._streamingCCYLimiter.add(async () => {
Resources.logger(this).debug(`Starting download of ${url}`);
const response = await this._streamingAxios.get(url);
Expand Down
8 changes: 5 additions & 3 deletions app/src/lib/sync-engine/sync-engine.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,14 +157,16 @@ export class SyncEngine {
* @returns A promise that resolves, once the file has been successfully written to disk
*/
async addAsset(asset: Asset) {
Resources.logger(this).debug(`Adding asset ${asset.getDisplayName()}`);
await this.icloud.photos.downloadAsset(asset);

try {
await this.icloud.photos.downloadAsset(asset);
await asset.verify()
} catch (err) {
Resources.logger(this).info(`Error while downloading asset ${asset.getDisplayName()}: ${err.message}`);
Resources.logger(this).info(`Unable to verify asset ${asset.getDisplayName()} at ${asset.getAssetFilePath()}: ${err.message}`);
Resources.emit(iCPSEventSyncEngine.WRITE_ASSET_ERROR, asset.getDisplayName());
return;
}


Resources.emit(iCPSEventSyncEngine.WRITE_ASSET_COMPLETED, asset.getDisplayName());
}
Expand Down
13 changes: 7 additions & 6 deletions app/test/unit/photos-library.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {FileType} from '../../src/lib/photos-library/model/file-type';
import * as Config from '../_helpers/_config';
import {MockedEventManager, MockedResourceManager, prepareResources} from '../_helpers/_general';
import {Zones} from '../../src/lib/icloud/icloud-photos/query-builder';
import { iCPSEventSyncEngine } from '../../src/lib/resources/events-types';

const primaryAssetDir = path.join(Config.defaultConfig.dataDir, PRIMARY_ASSET_DIR);
const sharedAssetDir = path.join(Config.defaultConfig.dataDir, SHARED_ASSET_DIR);
Expand Down Expand Up @@ -1190,7 +1191,7 @@ describe(`Write state`, () => {
folder.assets = albumAssets;
const library = new PhotosLibrary();

const handlerEvent = mockedEventManager.spyOnHandlerEvent();
const linkErrorEvent = mockedEventManager.spyOnEvent(iCPSEventSyncEngine.LINK_ERROR)

library.writeAlbum(folder);

Expand All @@ -1204,8 +1205,8 @@ describe(`Write state`, () => {
const albumAsset1Path = path.join(Config.defaultConfig.dataDir, `.${albumUUID}`, albumAsset1PrettyFilename);
expect(fs.existsSync(albumAsset1Path)).toBeFalsy();

expect(handlerEvent).toHaveBeenCalledWith(new Error(`Unable to link assets`));
expect(handlerEvent).toHaveBeenCalledTimes(1);
expect(linkErrorEvent).toHaveBeenCalledWith(albumAsset1Filename, albumName);
expect(linkErrorEvent).toHaveBeenCalledTimes(1);

const albumAsset2Path = path.join(Config.defaultConfig.dataDir, `.${albumUUID}`, albumAsset2PrettyFilename);
const albumAsset2Stat = fs.lstatSync(albumAsset2Path);
Expand Down Expand Up @@ -1352,7 +1353,7 @@ describe(`Write state`, () => {
folder.assets = albumAssets;
const library = new PhotosLibrary();

const handlerEvent = mockedEventManager.spyOnHandlerEvent();
const linkErrorEvent = mockedEventManager.spyOnEvent(iCPSEventSyncEngine.LINK_ERROR)

library.writeAlbum(folder);

Expand All @@ -1370,8 +1371,8 @@ describe(`Write state`, () => {
const albumAsset1Target = fs.readlinkSync(albumAsset1Path);
expect(albumAsset1Target).toEqual(path.join(`..`, PRIMARY_ASSET_DIR, albumAsset1Filename));

expect(handlerEvent).toHaveBeenCalledWith(new Error(`Unable to link assets`));
expect(handlerEvent).toHaveBeenCalledTimes(1);
expect(linkErrorEvent).toHaveBeenCalledWith(albumAsset2Filename, albumName);
expect(linkErrorEvent).toHaveBeenCalledTimes(1);
});
});

Expand Down
73 changes: 70 additions & 3 deletions app/test/unit/sync-engine.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -252,15 +252,17 @@ describe(`Coordination`, () => {

describe(`Handle processing queue`, () => {
describe(`Handle asset queue`, () => {
let writeAssetCompleteEvent: jest.Mock<UnknownFunction>;
let writeAssetCompleteEvent: jest.Mock<UnknownFunction>
let writeAssetErrorEvent: jest.Mock<UnknownFunction>;

beforeEach(() => {
syncEngine.photosLibrary.deleteAsset = jest.fn<typeof syncEngine.photosLibrary.deleteAsset>()
.mockResolvedValue();
syncEngine.icloud.photos.downloadAsset = jest.fn<typeof syncEngine.icloud.photos.downloadAsset>()
.mockResolvedValue({} as any);
.mockResolvedValue();

writeAssetCompleteEvent = mockedEventManager.spyOnEvent(iCPSEventSyncEngine.WRITE_ASSET_COMPLETED);
writeAssetCompleteEvent = mockedEventManager.spyOnEvent(iCPSEventSyncEngine.WRITE_ASSET_COMPLETED);
writeAssetErrorEvent = mockedEventManager.spyOnEvent(iCPSEventSyncEngine.WRITE_ASSET_ERROR);
});

test(`Empty processing queue`, async () => {
Expand Down Expand Up @@ -289,8 +291,11 @@ describe(`Handle processing queue`, () => {

test(`Only adding`, async () => {
const asset1 = new Asset(`somechecksum1`, 42, FileType.fromExtension(`png`), 42, getRandomZone(), AssetType.EDIT, `test1`, `somekey`, `somechecksum1`, `https://icloud.com`, `somerecordname1`, false);
asset1.verify = jest.fn<typeof asset1.verify>()
const asset2 = new Asset(`somechecksum2`, 42, FileType.fromExtension(`png`), 42, getRandomZone(), AssetType.EDIT, `test2`, `somekey`, `somechecksum2`, `https://icloud.com`, `somerecordname2`, false);
asset2.verify = jest.fn<typeof asset2.verify>()
const asset3 = new Asset(`somechecksum3`, 42, FileType.fromExtension(`png`), 42, getRandomZone(), AssetType.ORIG, `test3`, `somekey`, `somechecksum3`, `https://icloud.com`, `somerecordname3`, false);
asset3.verify = jest.fn<typeof asset3.verify>()
const toBeAdded = [asset1, asset2, asset3];

await syncEngine.writeAssets([[], toBeAdded, []]);
Expand All @@ -305,13 +310,75 @@ describe(`Handle processing queue`, () => {
expect(writeAssetCompleteEvent).toHaveBeenNthCalledWith(2, `somechecksum2`);
expect(writeAssetCompleteEvent).toHaveBeenNthCalledWith(3, `somechecksum3`);

expect(writeAssetErrorEvent).not.toHaveBeenCalled();

expect(syncEngine.photosLibrary.deleteAsset).not.toHaveBeenCalled();
});

test(`Only adding with verification error`, async () => {
const asset1 = new Asset(`somechecksum1`, 42, FileType.fromExtension(`png`), 42, getRandomZone(), AssetType.EDIT, `test1`, `somekey`, `somechecksum1`, `https://icloud.com`, `somerecordname1`, false);
asset1.verify = jest.fn<typeof asset1.verify>()
const asset2 = new Asset(`somechecksum2`, 42, FileType.fromExtension(`png`), 42, getRandomZone(), AssetType.EDIT, `test2`, `somekey`, `somechecksum2`, `https://icloud.com`, `somerecordname2`, false);
asset2.verify = jest.fn<typeof asset2.verify>()
const asset3 = new Asset(`somechecksum3`, 42, FileType.fromExtension(`png`), 42, getRandomZone(), AssetType.ORIG, `test3`, `somekey`, `somechecksum3`, `https://icloud.com`, `somerecordname3`, false);
asset3.verify = jest.fn<typeof asset3.verify>()
.mockRejectedValue(new Error(`verification error`))

const toBeAdded = [asset1, asset2, asset3];

await syncEngine.writeAssets([[], toBeAdded, []]);

expect(syncEngine.icloud.photos.downloadAsset).toHaveBeenCalledTimes(3);
expect(syncEngine.icloud.photos.downloadAsset).toHaveBeenNthCalledWith(1, asset1);
expect(syncEngine.icloud.photos.downloadAsset).toHaveBeenNthCalledWith(2, asset2);
expect(syncEngine.icloud.photos.downloadAsset).toHaveBeenNthCalledWith(3, asset3);

expect(writeAssetErrorEvent).toHaveBeenCalledTimes(1);

expect(writeAssetCompleteEvent).toHaveBeenCalledTimes(2);
expect(writeAssetCompleteEvent).toHaveBeenNthCalledWith(1, `somechecksum1`);
expect(writeAssetCompleteEvent).toHaveBeenNthCalledWith(2, `somechecksum2`);

expect(syncEngine.photosLibrary.deleteAsset).not.toHaveBeenCalled();
});

test(`Only adding with download error`, async () => {
const asset1 = new Asset(`somechecksum1`, 42, FileType.fromExtension(`png`), 42, getRandomZone(), AssetType.EDIT, `test1`, `somekey`, `somechecksum1`, `https://icloud.com`, `somerecordname1`, false);
asset1.verify = jest.fn<typeof asset1.verify>()
const asset2 = new Asset(`somechecksum2`, 42, FileType.fromExtension(`png`), 42, getRandomZone(), AssetType.EDIT, `test2`, `somekey`, `somechecksum2`, `https://icloud.com`, `somerecordname2`, false);
asset2.verify = jest.fn<typeof asset2.verify>()
const asset3 = new Asset(`somechecksum3`, 42, FileType.fromExtension(`png`), 42, getRandomZone(), AssetType.ORIG, `test3`, `somekey`, `somechecksum3`, `https://icloud.com`, `somerecordname3`, false);
asset3.verify = jest.fn<typeof asset3.verify>()

syncEngine.icloud.photos.downloadAsset = jest.fn<typeof syncEngine.icloud.photos.downloadAsset>()
.mockResolvedValueOnce()
.mockResolvedValueOnce()
.mockRejectedValueOnce(new Error());


const toBeAdded = [asset1, asset2, asset3];

await expect(syncEngine.writeAssets([[], toBeAdded, []])).rejects.toThrowError();

expect(syncEngine.icloud.photos.downloadAsset).toHaveBeenCalledTimes(3);
expect(syncEngine.icloud.photos.downloadAsset).toHaveBeenNthCalledWith(1, asset1);
expect(syncEngine.icloud.photos.downloadAsset).toHaveBeenNthCalledWith(2, asset2);
expect(syncEngine.icloud.photos.downloadAsset).toHaveBeenNthCalledWith(3, asset3);

expect(writeAssetCompleteEvent).toHaveBeenCalledTimes(2);
expect(writeAssetCompleteEvent).toHaveBeenNthCalledWith(1, `somechecksum1`);
expect(writeAssetCompleteEvent).toHaveBeenNthCalledWith(2, `somechecksum2`);

expect(syncEngine.photosLibrary.deleteAsset).not.toHaveBeenCalled();
});

test(`Adding & deleting`, async () => {
const asset1 = new Asset(`somechecksum1`, 42, FileType.fromExtension(`png`), 42, getRandomZone(), AssetType.EDIT, `test1`, `somekey`, `somechecksum1`, `https://icloud.com`, `somerecordname1`, false);
asset1.verify = jest.fn<typeof asset1.verify>()
const asset2 = new Asset(`somechecksum2`, 42, FileType.fromExtension(`png`), 42, getRandomZone(), AssetType.EDIT, `test2`, `somekey`, `somechecksum2`, `https://icloud.com`, `somerecordname2`, false);
asset2.verify = jest.fn<typeof asset2.verify>()
const asset3 = new Asset(`somechecksum3`, 42, FileType.fromExtension(`png`), 42, getRandomZone(), AssetType.ORIG, `test3`, `somekey`, `somechecksum3`, `https://icloud.com`, `somerecordname3`, false);
asset3.verify = jest.fn<typeof asset3.verify>()
const asset4 = new Asset(`somechecksum4`, 42, FileType.fromExtension(`png`), 42, getRandomZone(), AssetType.EDIT, `test4`, `somekey`, `somechecksum4`, `https://icloud.com`, `somerecordname4`, false);
const asset5 = new Asset(`somechecksum5`, 42, FileType.fromExtension(`png`), 42, getRandomZone(), AssetType.EDIT, `test5`, `somekey`, `somechecksum5`, `https://icloud.com`, `somerecordname5`, false);
const asset6 = new Asset(`somechecksum6`, 42, FileType.fromExtension(`png`), 42, getRandomZone(), AssetType.ORIG, `test6`, `somekey`, `somechecksum6`, `https://icloud.com`, `somerecordname6`, false);
Expand Down

0 comments on commit a938c7c

Please sign in to comment.