fix: Summarizing write errors instead of printing dedicated error

.vscode/launch.json

@@ -13,14 +13,14 @@
             ],
             "preLaunchTask": "Build App",
             "program": "${workspaceFolder}/app/src/main.ts",
-            "args": ["sync"],
+            "args": ["token"],
             "outFiles": [
                 "${workspaceFolder}/app/bin/**/*.js",
             ],
             "env": {
                 "NODE_NO_WARNINGS": "1"
             },
-            "envFile": "${workspaceFolder}/.vscode/private.env"
+            "envFile": "${workspaceFolder}/.vscode/test.env"
         }, {
             "name": "Run API Tests",
             "type": "node",
@@ -66,7 +66,7 @@
                 "--runInBand",
                 "--config", "jest.config.json",
                 //"--detectOpenHandles",
-                "test/unit/resources.network-manager.test.ts"
+                "test/unit/photos-library.test.ts"
             ],
             "env": {
                 "NODE_NO_WARNINGS": "1"

app/src/app/error/codes/library.ts

@@ -31,10 +31,6 @@ export const EXTRANEOUS_FILE: ErrorStruct = buildErrorStruct(
     name, prefix, `EXTRANEOUS_FILE`, `Extraneous file found while processing a folder`,
 );
 
-export const INVALID_ASSET: ErrorStruct = buildErrorStruct(
-    name, prefix, `INVALID_ASSET`, `Unable to verify asset`,
-);
-
 export const NO_PARENT: ErrorStruct = buildErrorStruct(
     name, prefix, `NO_PARENT`, `Unable to find parent of album`,
 );

app/src/app/event/cli.ts

@@ -14,6 +14,11 @@ export class CLIInterface {
      */
     progressBar: SingleBar;
 
+    /**
+     * Keeps track of write errors during sync to provide a summary at the end
+     */
+    writeErrors: number = 0;
+
     /**
      * Creates a new CLI interface based on the provided components
      * @param options - Parsed CLI Options
@@ -140,13 +145,21 @@ export class CLIInterface {
             .on(iCPSEventSyncEngine.WRITE_ASSETS, (toBeDeletedCount: number, toBeAddedCount: number, toBeKept: number) => {
                 this.print(chalk.cyan(`Syncing assets, by keeping ${toBeKept} and removing ${toBeDeletedCount} local assets, as well as adding ${toBeAddedCount} remote assets...`));
                 this.progressBar.start(toBeAddedCount, 0);
+                this.writeErrors = 0;
             })
             .on(iCPSEventSyncEngine.WRITE_ASSET_COMPLETED, _recordName => {
                 this.progressBar.increment();
             })
+            .on(iCPSEventSyncEngine.WRITE_ASSET_ERROR, _recordName => {
+                this.writeErrors++;
+                this.progressBar.increment();
+            })
             .on(iCPSEventSyncEngine.WRITE_ASSETS_COMPLETED, () => {
                 this.progressBar.stop();
                 this.print(chalk.greenBright(`Successfully synced assets!`));
+                if (this.writeErrors > 0) {
+                    this.print(`Detected ${this.writeErrors} errors while adding assets, please check the logs for more details.`);
+                }
             })
             .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...`));

app/src/app/event/metrics-exporter.ts

@@ -38,6 +38,7 @@ const FIELDS = {
     ASSETS_ARCHIVED: `assets_archived`,
     REMOTE_ASSETS_DELETED: `remote_assets_deleted`,
     ASSET_WRITTEN: `asset_written`,
+    ASSET_ERROR: `asset_error`,
     ALBUMS_TO_BE_ADDED: `albums_to_be_added`,
     ALBUMS_TO_BE_KEPT: `albums_to_be_kept`,
     ALBUMS_TO_BE_DELETED: `albums_to_be_deleted`,
@@ -378,6 +379,11 @@ export class MetricsExporter {
                         .addField(FIELDS.ASSET_WRITTEN, recordName),
                     );
                 })
+                .on(iCPSEventSyncEngine.WRITE_ASSET_ERROR, (recordName: string) => {
+                    this.logDataPoint(new iCPSInfluxLineProtocolPoint()
+                        .addField(FIELDS.ASSET_ERROR, recordName),
+                    );
+                })
                 .on(iCPSEventSyncEngine.WRITE_ASSETS_COMPLETED, () => {
                     this.logDataPoint(new iCPSInfluxLineProtocolPoint()
                         .logStatus(FIELDS.STATUS.values.WRITE_ASSETS_COMPLETED));

app/src/lib/icloud/icloud-photos/icloud-photos.ts

@@ -1,4 +1,4 @@
-import {AxiosRequestConfig, AxiosResponse} from 'axios';
+import {AxiosRequestConfig} from 'axios';
 import * as QueryBuilder from './query-builder.js';
 import {AlbumAssets, AlbumType} from '../../photos-library/model/album.js';
 import {Asset} from '../../photos-library/model/asset.js';
@@ -9,7 +9,7 @@ import {Resources} from '../../resources/main.js';
 import {ENDPOINTS} from '../../resources/network-types.js';
 import {SyncEngineHelper} from '../../sync-engine/helper.js';
 import {iCPSEventError, iCPSEventPhotos} from '../../resources/events-types.js';
-import {Readable} from 'stream';
+import fs from 'fs/promises';
 
 /**
  * To perform an operation, a record change tag is required. Hardcoding it for now
@@ -529,7 +529,9 @@ export class iCloudPhotos {
         if (ignoredAssets.length > 0) {
             Resources.logger(this).info(`Ignoring ${ignoredAssets.length} assets for ${parentId === undefined ? `All photos` : parentId}:`);
             const erroredAssets = ignoredAssets.filter(err => err.code !== ICLOUD_PHOTOS_ERR.UNWANTED_RECORD_TYPE.code); // Filtering 'expected' errors
-            Resources.logger(this).debug(`${erroredAssets.length} unexpected errors: ${erroredAssets.map(err => err.code).join(`, `)}`);
+            if (erroredAssets.length > 0) {
+                Resources.logger(this).warn(`${erroredAssets.length} unexpected errors for ${parentId === undefined ? `All photos` : parentId}: ${erroredAssets.map(err => err.code).join(`, `)}`);
+            }
         }
 
         // There should be one CPLMaster and one CPLAsset per record, however the iCloud response is sometimes not adhering to this.
@@ -549,10 +551,13 @@ export class iCloudPhotos {
     /**
      * Downloads an asset using the 'stream' method
      * @param asset - The asset to be downloaded
-     * @returns A promise, that -once resolved-, contains the Axios response
+     * @returns A promise, that resolves, once the asset has been written to disk
      */
-    async downloadAsset(asset: Asset): Promise<AxiosResponse<Readable, any>> {
-        return Resources.network().getDataStream(asset.downloadURL);
+    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();
     }
 
     /**

app/src/lib/photos-library/photos-library.ts

@@ -4,14 +4,11 @@ import {Album, AlbumType} from './model/album.js';
 import fs from 'fs';
 import {Asset} from './model/asset.js';
 import {PLibraryEntities} from './model/photos-entity.js';
-import {AxiosResponse} from 'axios';
-import {pEvent} from 'p-event';
 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 {Readable} from 'stream';
 
 type PathTuple = [namePath: string, uuidPath: string]
 
@@ -257,33 +254,6 @@ export class PhotosLibrary {
         return AlbumType.ALBUM;
     }
 
-    /**
-     * Writes the contents of the Axios response (as stream) to the filesystem, based on the associated Asset
-     * @param asset - The asset associated to the request
-     * @param response - The response -as a stream- containing the data of the asset
-     * @returns A promise, that resolves once this asset was written to disk and verified
-     */
-    async writeAsset(asset: Asset, response: AxiosResponse<Readable, any>): Promise<void> {
-        Resources.logger(this).debug(`Writing asset ${asset.getDisplayName()}`);
-        const location = asset.getAssetFilePath();
-        const writeStream = fs.createWriteStream(location, {flags: `w`});
-        response.data.pipe(writeStream);
-        await pEvent(writeStream, `finish`, {rejectionEvents: [`error`]});
-        try {
-            await fs.promises.utimes(location, new Date(asset.modified), new Date(asset.modified)); // Setting modified date on file
-            await this.verifyAsset(asset);
-            Resources.logger(this).debug(`Asset ${asset.getDisplayName()} successfully downloaded`);
-        } catch (err) {
-            Resources.emit(iCPSEventError.HANDLER_EVENT,
-                new iCPSError(LIBRARY_ERR.INVALID_ASSET)
-                    .addMessage(asset.getDisplayName())
-                    .addContext(`asset`, asset)
-                    .addCause(err)
-                    .setWarning(),
-            );
-        }
-    }
-
     /**
      * Deletes the specified asset from disk
      * @param asset - The asset that needs to be removed

app/src/lib/resources/events-types.ts

@@ -153,6 +153,10 @@ export enum iCPSEventSyncEngine {
      * Emitted when the write process has completed writing an asset - provides the asset name as argument
      */
     WRITE_ASSET_COMPLETED = `write-asset-completed`,
+    /**
+     * Emitted when the write process has experienced an error writing an asset - provides the asset name as argument
+     */
+    WRITE_ASSET_ERROR = `write-asset-error`,
     /**
      * Emitted when the write process has completed writing all assets
      */

app/src/lib/resources/network-manager.ts

@@ -1,16 +1,17 @@
 import axios, {AxiosInstance, AxiosRequestConfig, AxiosResponse, InternalAxiosRequestConfig} from "axios";
 import fs from "fs/promises";
+import {createWriteStream} from "fs";
 import * as PACKAGE from "../package.js";
 import {HEADER_KEYS, SigninResponse, COOKIE_KEYS, TrustResponse, SetupResponse, ENDPOINTS, PhotosSetupResponse, USER_AGENT, CLIENT_ID, CLIENT_INFO} from "./network-types.js";
 import {Cookie} from "tough-cookie";
 import {iCPSError} from "../../app/error/error.js";
 import {RESOURCES_ERR} from "../../app/error/error-codes.js";
 import {AxiosHarTracker} from "axios-har-tracker";
 import {FILE_ENCODING} from "./resource-types.js";
-import {Readable} from "stream";
 import PQueue from "p-queue";
 import {Resources} from "./main.js";
 import {iCPSAppOptions} from "../../app/factory.js";
+import {pEvent} from "p-event";
 
 export class Header {
     key: string;
@@ -274,14 +275,14 @@ export class NetworkManager {
      * Pending jobs will be cancelled and running jobs will be awaited
      * @param queue - The queue to settle
      */
-    async settleQueue(queue: PQueue) {
-        if (queue.size > 0) {
+    async settleQueue(queue: PQueue, clearQueuedJobs: boolean = true) {
+        if (clearQueuedJobs && queue.size > 0) {
             Resources.logger(this).info(`Clearing queue with ${queue.size} queued job(s)...`);
             queue.clear();
         }
 
         if (queue.pending > 0) {
-            Resources.logger(this).info(`${queue.pending} pending job(s), waiting for them to settle...`);
+            Resources.logger(this).info(`${queue.pending} pending job(s) (${queue.size} queued jobs), waiting for them to settle...`);
             await queue.onIdle();
         }
 
@@ -437,20 +438,21 @@ export class NetworkManager {
     }
 
     /**
-     * Performs a GET request to acquire a asset's data stream
-     * Uses concurrency limiting to ensure that the available bandwidth is used most efficiently
-     * @param url - The location of the asset
-     * @returns A promise, that resolves once the request has been completed.
+     * Downloads the provided url's content and writes it to the provided location
+     * @param url - The url to download
+     * @param location - The location to write the file to (existing files will be overwritten)
      */
-    // async getDataStream<R = AxiosResponse<Readable>, D = any>(url: string, config?: AxiosRequestConfig<D>): Promise<R> {
-    async getDataStream(url: string): Promise<AxiosResponse<Readable>> {
+    async downloadData(url: string, location: string): Promise<void> {
         Resources.logger(this).debug(`Adding ${url} to download queue`);
-        return this._streamingCCYLimiter.add(async () => {
+        await this._streamingCCYLimiter.add(async () => {
             Resources.logger(this).debug(`Starting download of ${url}`);
             const response = await this._streamingAxios.get(url);
+            Resources.logger(this).debug(`Starting to write ${url} to ${location}`);
+            const writeStream = createWriteStream(location, {flags: `w`});
+            response.data.pipe(writeStream);
+            await pEvent(writeStream, `finish`, {rejectionEvents: [`error`]});
             Resources.logger(this).debug(`Finished download of ${url}`);
-            return response;
-        }) as Promise<AxiosResponse<Readable>>;
+        });
     }
 
     /**

app/src/lib/sync-engine/sync-engine.ts

@@ -42,15 +42,13 @@ export class SyncEngine {
     async sync(): Promise<[Asset[], Album[]]> {
         Resources.logger(this).info(`Starting sync`);
         Resources.emit(iCPSEventSyncEngine.START);
-        let retryCount = 0;
-        while (Resources.manager().maxRetries > retryCount) {
-            retryCount++;
+        let retryCount = 1;
+        while (Resources.manager().maxRetries >= retryCount) {
             Resources.logger(this).info(`Performing sync, try #${retryCount}`);
 
-            const [remoteAssets, remoteAlbums, localAssets, localAlbums] = await this.fetchAndLoadState();
-            const [assetQueue, albumQueue] = await this.diffState(remoteAssets, remoteAlbums, localAssets, localAlbums);
-
             try {
+                const [remoteAssets, remoteAlbums, localAssets, localAlbums] = await this.fetchAndLoadState();
+                const [assetQueue, albumQueue] = await this.diffState(remoteAssets, remoteAlbums, localAssets, localAlbums);
                 await this.writeState(assetQueue, albumQueue);
                 Resources.logger(this).info(`Completed sync!`);
                 Resources.emit(iCPSEventSyncEngine.DONE);
@@ -63,6 +61,10 @@ export class SyncEngine {
                 }
 
                 await Resources.network().settleCCYLimiter();
+                Resources.logger(this).debug(`Refreshing iCloud cookies...`);
+                await this.icloud.setupAccount();
+
+                retryCount++;
                 Resources.emit(iCPSEventSyncEngine.RETRY, retryCount);
             }
         }
@@ -156,10 +158,13 @@ export class SyncEngine {
      */
     async addAsset(asset: Asset) {
         Resources.logger(this).debug(`Adding asset ${asset.getDisplayName()}`);
-
-        const response = await this.icloud.photos.downloadAsset(asset);
-
-        await this.photosLibrary.writeAsset(asset, response);
+        try {
+            await this.icloud.photos.downloadAsset(asset);
+        } catch (err) {
+            Resources.logger(this).info(`Error while downloading asset ${asset.getDisplayName()}: ${err.message}`);
+            Resources.emit(iCPSEventSyncEngine.WRITE_ASSET_ERROR, asset.getDisplayName());
+            return;
+        }
 
         Resources.emit(iCPSEventSyncEngine.WRITE_ASSET_COMPLETED, asset.getDisplayName());
     }

app/test/_helpers/_general.ts

@@ -57,6 +57,7 @@ export function prepareResourceForApiTests(): Resources.Types.Instances {
     instances.manager._readResourceFile = jest.fn<typeof instances.manager._readResourceFile>()
         .mockReturnValue({
             libraryVersion: 1,
+            trustToken: process.env.TEST_TRUST_TOKEN!,
         });
 
     return instances;