fix: Adding timeout option for download requests

steilerDev · Aug 24, 2023 · 841c7e4 · 841c7e4
1 parent a938c7c
commit 841c7e4
Show file tree

Hide file tree

Showing 13 changed files with 116 additions and 571 deletions.
diff --git a/.vscode/launch.json b/.vscode/launch.json
@@ -66,7 +66,7 @@
                 "--runInBand",
                 "--config", "jest.config.json",
                 //"--detectOpenHandles",
-                "test/unit/photos-library.test.ts"
+                "test/unit/app.test.ts"
             ],
             "env": {
                 "NODE_NO_WARNINGS": "1"

diff --git a/app/src/app/error/codes/sync.ts b/app/src/app/error/codes/sync.ts
@@ -28,9 +28,9 @@ export const MAX_RETRY: ErrorStruct = buildErrorStruct(
 );
 
 export const NETWORK: ErrorStruct = buildErrorStruct(
-    name, prefix, `NETWORK`, `Network error during sync, trying to retry`,
+    name, prefix, `NETWORK`, `Network error during sync`,
 );
 
 export const UNKNOWN: ErrorStruct = buildErrorStruct(
-    name, prefix, `UNKNOWN`, `Unknown error during sync, trying to retry`,
+    name, prefix, `UNKNOWN`, `Unknown error during sync`,
 );
diff --git a/app/src/app/event/cli.ts b/app/src/app/event/cli.ts
@@ -4,7 +4,6 @@ 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';
 
 /**
  * This class handles the input/output to the command line
@@ -168,6 +167,7 @@ export class CLIInterface {
                     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) => {
@@ -182,6 +182,7 @@ export class CLIInterface {
                 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, () => {
@@ -194,7 +195,7 @@ export class CLIInterface {
             })
             .on(iCPSEventSyncEngine.RETRY, retryCount => {
                 this.progressBar.stop();
-                this.print(chalk.magenta(`Detected recoverable error, refreshing iCloud connection & retrying (#${retryCount})...`));
+                this.print(chalk.magenta(`Detected error during sync, refreshing iCloud connection & retrying (attempt #${retryCount})...`));
                 this.print(chalk.white(this.getHorizontalLine()));
             });
 

diff --git a/app/src/app/event/metrics-exporter.ts b/app/src/app/event/metrics-exporter.ts
@@ -399,7 +399,7 @@ export class MetricsExporter {
                 })
                 .on(iCPSEventSyncEngine.LINK_ERROR, (_assetUUID: string, _albumName: string) => {
                     this.logDataPoint(new iCPSInfluxLineProtocolPoint()
-                        .addField(FIELDS.LINK_ERROR, `${_assetUUID} -> ${_albumName}`) 
+                        .addField(FIELDS.LINK_ERROR, `${_assetUUID} -> ${_albumName}`),
                     );
                 })
                 .on(iCPSEventSyncEngine.WRITE_ALBUMS_COMPLETED, () => {

diff --git a/app/src/app/factory.ts b/app/src/app/factory.ts
@@ -91,6 +91,7 @@ export type iCPSAppOptions = {
     port: number,
     maxRetries: number,
     downloadThreads: number,
+    downloadTimeout: number,
     schedule: string,
     enableCrashReporting: boolean,
     enableNetworkCapture: boolean,
@@ -148,6 +149,10 @@ export function appFactory(argv: string[]): iCPSApp {
             .env(`DOWNLOAD_THREADS`)
             .default(5)
             .argParser(commanderParsePositiveIntOrInfinity))
+        .addOption(new Option(`--download-timeout <number>`, `Sets the request timeout (in minutes) for downloading assets, should be increased on slower connections and/or if there are large assets in the library ('0' will remove the timeout).`)
+            .env(`DOWNLOAD_TIMEOUT`)
+            .default(5)
+            .argParser(commanderParsePositiveInt))
         .addOption(new Option(`-S, --schedule <cron-string>`, `In case this app is executed in daemon mode, it will use this cron schedule to perform regular syncs.`)
             .env(`SCHEDULE`)
             .default(`0 2 * * *`)

diff --git a/app/src/lib/photos-library/photos-library.ts b/app/src/lib/photos-library/photos-library.ts
@@ -421,7 +421,7 @@ export class PhotosLibrary {
                     .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()}`);
+                Resources.logger(this).warn(`Unable to link ${relativeAssetPath} to ${linkedAsset}: ${linkErr.getDescription()}`);
             }
         });
     }

diff --git a/app/src/lib/resources/network-manager.ts b/app/src/lib/resources/network-manager.ts
@@ -211,6 +211,7 @@ export class NetworkManager {
 
         this._streamingAxios = axios.create({
             responseType: `stream`,
+            timeout: (1000 * 60 * resources.downloadTimeout),
         });
 
         if (resources.enableNetworkCapture) {

diff --git a/app/src/lib/sync-engine/sync-engine.ts b/app/src/lib/sync-engine/sync-engine.ts
@@ -43,6 +43,10 @@ export class SyncEngine {
         Resources.logger(this).info(`Starting sync`);
         Resources.emit(iCPSEventSyncEngine.START);
         let retryCount = 1;
+
+        // Keeping track of all previous errors in case we reache retry limit
+        const retryError = new iCPSError(SYNC_ERR.MAX_RETRY);
+
         while (Resources.manager().maxRetries >= retryCount) {
             Resources.logger(this).info(`Performing sync, try #${retryCount}`);
 
@@ -55,23 +59,30 @@ export class SyncEngine {
                 return [remoteAssets, remoteAlbums];
             } catch (err) {
                 if (err instanceof AxiosError) {
-                    Resources.emit(iCPSEventError.HANDLER_EVENT, new iCPSError(SYNC_ERR.NETWORK).addCause(err));
+                    Resources.emit(iCPSEventError.HANDLER_EVENT, new iCPSError(SYNC_ERR.NETWORK)
+                        .addCause(err)
+                        .setWarning(),
+                    );
                 } else {
-                    Resources.emit(iCPSEventError.HANDLER_EVENT, new iCPSError(SYNC_ERR.UNKNOWN).addCause(err));
+                    Resources.emit(iCPSEventError.HANDLER_EVENT, new iCPSError(SYNC_ERR.UNKNOWN)
+                        .addCause(err)
+                        .setWarning(),
+                    );
                 }
 
+                retryError.addContext(`error-try-${retryCount}`, err);
+                retryCount++;
+                Resources.emit(iCPSEventSyncEngine.RETRY, retryCount);
+
                 await Resources.network().settleCCYLimiter();
+
                 Resources.logger(this).debug(`Refreshing iCloud cookies...`);
                 await this.icloud.setupAccount();
-
-                retryCount++;
-                Resources.emit(iCPSEventSyncEngine.RETRY, retryCount);
             }
         }
 
         // We'll only reach this, if we exceeded retryCount
-        throw new iCPSError(SYNC_ERR.MAX_RETRY)
-            .addMessage(`${retryCount}`);
+        throw retryError.addMessage(`${retryCount}`);
     }
 
     /**
@@ -160,13 +171,12 @@ export class SyncEngine {
         await this.icloud.photos.downloadAsset(asset);
 
         try {
-            await asset.verify()
+            await asset.verify();
         } catch (err) {
-            Resources.logger(this).info(`Unable to verify asset ${asset.getDisplayName()} at ${asset.getAssetFilePath()}: ${err.message}`);
+            Resources.logger(this).warn(`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());
     }

diff --git a/app/test/_helpers/_config.ts b/app/test/_helpers/_config.ts
@@ -9,6 +9,7 @@ export const defaultConfig = {
     port: 80,
     maxRetries: 10,
     downloadThreads: 5,
+    downloadTimeout: 5,
     schedule: `0 2 * * *`,
     enableCrashReporting: false,
     failOnMfa: false,