Skip to content

Commit

Permalink
fix: Separate streaming axios instance to handle asset downloads
Browse files Browse the repository at this point in the history
  • Loading branch information
Frank Steiler committed Aug 4, 2023
1 parent 0fda9ee commit afed636
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 25 deletions.
2 changes: 1 addition & 1 deletion .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"env": {
"NODE_NO_WARNINGS": "1"
},
"envFile": "${workspaceFolder}/.vscode/test.env"
"envFile": "${workspaceFolder}/.vscode/private.env"
}, {
"name": "Run API Tests",
"type": "node",
Expand Down
6 changes: 2 additions & 4 deletions app/src/app/event/error-handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {FILE_ENCODING} from '../../lib/resource-manager/resources.js';
import * as zlib from 'zlib';
import {Readable} from 'stream';
import os from 'os';
import {pEvent} from 'p-event';

const reportDenyList = [
ERR_SIGINT.code,
Expand Down Expand Up @@ -263,10 +264,7 @@ export class ErrorHandler {

const brotliStream = zlib.createBrotliCompress();
data.pipe(brotliStream).pipe(output);
await new Promise<void>((resolve, reject) => {
output.on(`finish`, resolve);
output.on(`error`, reject);
});
await pEvent(output, `finish`, {rejectionEvents: [`error`]});
} finally {
await targetFd?.close();
}
Expand Down
13 changes: 3 additions & 10 deletions app/src/lib/icloud/icloud-photos/icloud-photos.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {ResourceManager} from '../../resource-manager/resource-manager.js';
import {ENDPOINTS} from '../../resource-manager/network.js';
import {SyncEngineHelper} from '../../sync-engine/helper.js';
import {iCPSEventError, iCPSEventPhotos} from '../../resource-manager/events.js';
import {Readable} from 'stream';

/**
* To perform an operation, a record change tag is required. Hardcoding it for now
Expand Down Expand Up @@ -553,17 +554,9 @@ export class iCloudPhotos {
* @param asset - The asset to be downloaded
* @returns A promise, that -once resolved-, contains the Axios response
*/
async downloadAsset(asset: Asset): Promise<AxiosResponse<any, any>> {
async downloadAsset(asset: Asset): Promise<AxiosResponse<Readable, any>> {
ResourceManager.logger(this).debug(`Starting download of asset ${asset.getDisplayName()}`);

const config: AxiosRequestConfig = {
responseType: `stream`,
};

return ResourceManager.network.get(
asset.downloadURL,
config,
);
return ResourceManager.network.getDataStream(asset.downloadURL);
}

/**
Expand Down
7 changes: 4 additions & 3 deletions app/src/lib/photos-library/photos-library.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {LIBRARY_ERR} from '../../app/error/error-codes.js';
import {Zones} from '../icloud/icloud-photos/query-builder.js';
import {ResourceManager} from '../resource-manager/resource-manager.js';
import {iCPSEventError} from '../resource-manager/events.js';
import {Readable} from 'stream';

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

Expand Down Expand Up @@ -253,12 +254,12 @@ export class PhotosLibrary {
* @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<any, any>): Promise<void> {
async writeAsset(asset: Asset, response: AxiosResponse<Readable, any>): Promise<void> {
ResourceManager.logger(this).debug(`Writing asset ${asset.getDisplayName()}`);
const location = asset.getAssetFilePath();
const writeStream = fs.createWriteStream(location);
const writeStream = fs.createWriteStream(location, {flags: `w`});
response.data.pipe(writeStream);
await pEvent(writeStream, `close`);
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);
Expand Down
22 changes: 22 additions & 0 deletions app/src/lib/resource-manager/network-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {iCPSError} from "../../app/error/error.js";
import {RES_MANAGER_ERR} from "../../app/error/error-codes.js";
import {AxiosHarTracker} from "axios-har-tracker";
import {FILE_ENCODING} from "./resources.js";
import {Readable} from "stream";

class Header {
key: string;
Expand Down Expand Up @@ -106,6 +107,12 @@ export class NetworkManager {
*/
_axios: AxiosInstance;

/**
* A separate axios instance to handle stream based downloads of assets
* This allows us to bypass har files for those big files - additionally something is not handling the stream correctly
*/
_streamingAxios: AxiosInstance;

/**
* Collection of header values and cookies that are applied based on the request
*/
Expand All @@ -121,6 +128,11 @@ export class NetworkManager {
headers: HEADER.DEFAULT,
});

this._streamingAxios = axios.create({
headers: HEADER.DEFAULT,
responseType: `stream`,
});

if (enableNetworkCapture) {
this._harTracker = new AxiosHarTracker(this._axios as any);
}
Expand Down Expand Up @@ -323,6 +335,16 @@ export class NetworkManager {
return this._axios.get(url, config);
}

/**
* Performs a GET request to acquire a asset's data stream
* @param url - The location of the asset
* @returns A promise, that resolves once the request has been completed.
*/
// async getDataStream<R = AxiosResponse<Readable>, D = any>(url: string, config?: AxiosRequestConfig<D>): Promise<R> {
async getDataStream(url: string): Promise<AxiosResponse<Readable>> {
return this._streamingAxios.get(url);
}

/**
* Perform a PUT request using the local axios instance and configuration
* @param url - The url to request
Expand Down
10 changes: 3 additions & 7 deletions app/src/lib/sync-engine/sync-engine.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,13 +189,9 @@ export class SyncEngine {
async addAsset(asset: Asset) {
ResourceManager.logger(this).info(`Adding asset ${asset.getDisplayName()}`);

try {
await this.photosLibrary.verifyAsset(asset);
ResourceManager.logger(this).debug(`Asset ${asset.getDisplayName()} already downloaded`);
} catch (err) {
const data = await this.icloud.photos.downloadAsset(asset);
await this.photosLibrary.writeAsset(asset, data);
}
const response = await this.icloud.photos.downloadAsset(asset);

await this.photosLibrary.writeAsset(asset, response);

ResourceManager.emit(iCPSEventSyncEngine.WRITE_ASSET_COMPLETED, asset.getDisplayName());
}
Expand Down
6 changes: 6 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit afed636

Please sign in to comment.