Browse Source

Fix server crash on bad file operation and other optimizations (#291)

* Fixed issue with generating thumbnail for video with 0 length cause undefined file and crash the server
* Added all file error handling operation
* Temporarily disabled WebSocket on the web because receiving a new upload event doesn't put the new file in the correct place. 
* Cosmetic fixed on the info panel
Alex 3 years ago
parent
commit
a45d6fdf57

+ 1 - 1
server/apps/immich/src/api-v1/asset/asset.controller.ts

@@ -116,7 +116,7 @@ export class AssetController {
   }
 
   @Get('/thumbnail/:assetId')
-  async getAssetThumbnail(@Param('assetId') assetId: string): Promise<StreamableFile> {
+  async getAssetThumbnail(@Param('assetId') assetId: string) {
     return await this.assetService.getAssetThumbnail(assetId);
   }
 

+ 86 - 65
server/apps/immich/src/api-v1/asset/asset.service.ts

@@ -11,12 +11,13 @@ import { IsNull, Not, Repository } from 'typeorm';
 import { AuthUserDto } from '../../decorators/auth-user.decorator';
 import { CreateAssetDto } from './dto/create-asset.dto';
 import { AssetEntity, AssetType } from '@app/database/entities/asset.entity';
-import { createReadStream, stat } from 'fs';
+import { constants, createReadStream, ReadStream, stat } from 'fs';
 import { ServeFileDto } from './dto/serve-file.dto';
 import { Response as Res } from 'express';
 import { promisify } from 'util';
 import { DeleteAssetDto } from './dto/delete-asset.dto';
 import { SearchAssetDto } from './dto/search-asset.dto';
+import fs from 'fs/promises';
 
 const fileInfo = promisify(stat);
 
@@ -123,7 +124,7 @@ export class AssetService {
 
   public async downloadFile(query: ServeFileDto, res: Res) {
     try {
-      let file = null;
+      let fileReadStream = null;
       const asset = await this.findOne(query.did, query.aid);
 
       if (query.isThumb === 'false' || !query.isThumb) {
@@ -132,76 +133,90 @@ export class AssetService {
           'Content-Type': asset.mimeType,
           'Content-Length': size,
         });
-        file = createReadStream(asset.originalPath);
+
+        await fs.access(asset.originalPath, constants.R_OK | constants.W_OK);
+        fileReadStream = createReadStream(asset.originalPath);
       } else {
         if (!asset.resizePath) {
-          throw new Error('resizePath not set');
+          throw new NotFoundException('resizePath not set');
         }
         const { size } = await fileInfo(asset.resizePath);
         res.set({
           'Content-Type': 'image/jpeg',
           'Content-Length': size,
         });
-        file = createReadStream(asset.resizePath);
+
+        await fs.access(asset.resizePath, constants.R_OK | constants.W_OK);
+        fileReadStream = createReadStream(asset.resizePath);
       }
 
-      return new StreamableFile(file);
+      return new StreamableFile(fileReadStream);
     } catch (e) {
-      Logger.error('Error download asset ', e);
+      Logger.error(`Error download asset`, 'downloadFile');
       throw new InternalServerErrorException(`Failed to download asset ${e}`, 'DownloadFile');
     }
   }
 
-  public async getAssetThumbnail(assetId: string): Promise<StreamableFile> {
-    try {
-      const asset = await this.assetRepository.findOne({ where: { id: assetId } });
-      if (!asset) {
-        throw new NotFoundException('Asset not found');
-      }
+  public async getAssetThumbnail(assetId: string) {
+    let fileReadStream: ReadStream;
+
+    const asset = await this.assetRepository.findOne({ where: { id: assetId } });
+
+    if (!asset) {
+      throw new NotFoundException('Asset not found');
+    }
 
+    try {
       if (asset.webpPath && asset.webpPath.length > 0) {
-        return new StreamableFile(createReadStream(asset.webpPath));
+        await fs.access(asset.webpPath, constants.R_OK | constants.W_OK);
+        fileReadStream = createReadStream(asset.webpPath);
       } else {
         if (!asset.resizePath) {
-          throw new Error('resizePath not set');
+          return new NotFoundException('resizePath not set');
         }
-        return new StreamableFile(createReadStream(asset.resizePath));
+
+        await fs.access(asset.resizePath, constants.R_OK | constants.W_OK);
+        fileReadStream = createReadStream(asset.resizePath);
       }
+
+      return new StreamableFile(fileReadStream);
     } catch (e) {
-      if (e instanceof NotFoundException) {
-        throw e;
-      }
-      Logger.error('Error serving asset thumbnail ', e);
-      throw new InternalServerErrorException('Failed to serve asset thumbnail', 'GetAssetThumbnail');
+      Logger.error(`Cannot create read stream for asset ${asset.id}`, 'getAssetThumbnail');
+      throw new InternalServerErrorException(
+        e,
+        `Cannot read thumbnail file for asset ${asset.id} - contact your administrator`,
+      );
     }
   }
 
   public async serveFile(authUser: AuthUserDto, query: ServeFileDto, res: Res, headers: any) {
-    let file = null;
+    let fileReadStream: ReadStream;
     const asset = await this.findOne(query.did, query.aid);
 
     if (!asset) {
-      // TODO: maybe this should be a NotFoundException?
-      throw new BadRequestException('Asset does not exist');
+      throw new NotFoundException('Asset does not exist');
     }
 
     // Handle Sending Images
     if (asset.type == AssetType.IMAGE || query.isThumb == 'true') {
-      /**
-       * Serve file viewer on the web
-       */
-      if (query.isWeb) {
-        res.set({
-          'Content-Type': 'image/jpeg',
-        });
-        if (!asset.resizePath) {
-          Logger.error('Error serving IMAGE asset for web', 'ServeFile');
-          throw new InternalServerErrorException(`Failed to serve image asset for web`, 'ServeFile');
+      try {
+        /**
+         * Serve file viewer on the web
+         */
+        if (query.isWeb) {
+          res.set({
+            'Content-Type': 'image/jpeg',
+          });
+          if (!asset.resizePath) {
+            Logger.error('Error serving IMAGE asset for web', 'ServeFile');
+            throw new InternalServerErrorException(`Failed to serve image asset for web`, 'ServeFile');
+          }
+          await fs.access(asset.resizePath, constants.R_OK | constants.W_OK);
+          fileReadStream = createReadStream(asset.resizePath);
+
+          return new StreamableFile(fileReadStream);
         }
-        return new StreamableFile(createReadStream(asset.resizePath));
-      }
 
-      try {
         /**
          * Serve thumbnail image for both web and mobile app
          */
@@ -209,34 +224,38 @@ export class AssetService {
           res.set({
             'Content-Type': asset.mimeType,
           });
-          file = createReadStream(asset.originalPath);
+
+          await fs.access(asset.originalPath, constants.R_OK | constants.W_OK);
+          fileReadStream = createReadStream(asset.originalPath);
         } else {
           if (asset.webpPath && asset.webpPath.length > 0) {
             res.set({
               'Content-Type': 'image/webp',
             });
 
-            file = createReadStream(asset.webpPath);
+            await fs.access(asset.webpPath, constants.R_OK | constants.W_OK);
+            fileReadStream = createReadStream(asset.webpPath);
           } else {
             res.set({
               'Content-Type': 'image/jpeg',
             });
+
             if (!asset.resizePath) {
               throw new Error('resizePath not set');
             }
-            file = createReadStream(asset.resizePath);
+
+            await fs.access(asset.resizePath, constants.R_OK | constants.W_OK);
+            fileReadStream = createReadStream(asset.resizePath);
           }
         }
 
-        file.on('error', (error) => {
-          Logger.log(`Cannot create read stream ${error}`);
-          return new BadRequestException('Cannot Create Read Stream');
-        });
-
-        return new StreamableFile(file);
+        return new StreamableFile(fileReadStream);
       } catch (e) {
-        Logger.error('Error serving IMAGE asset ', e);
-        throw new InternalServerErrorException(`Failed to serve image asset ${e}`, 'ServeFile');
+        Logger.error(`Cannot create read stream for asset ${asset.id}`, 'serveFile[IMAGE]');
+        throw new InternalServerErrorException(
+          e,
+          `Cannot read thumbnail file for asset ${asset.id} - contact your administrator`,
+        );
       }
     } else if (asset.type == AssetType.VIDEO) {
       try {
@@ -244,6 +263,8 @@ export class AssetService {
         let videoPath = asset.originalPath;
         let mimeType = asset.mimeType;
 
+        await fs.access(videoPath, constants.R_OK | constants.W_OK);
+
         if (query.isWeb && asset.mimeType == 'video/quicktime') {
           videoPath = asset.encodedVideoPath == '' ? asset.originalPath : asset.encodedVideoPath;
           mimeType = asset.encodedVideoPath == '' ? asset.mimeType : 'video/mp4';
@@ -297,7 +318,7 @@ export class AssetService {
           return new StreamableFile(createReadStream(videoPath));
         }
       } catch (e) {
-        Logger.error('Error serving VIDEO asset ', e);
+        Logger.error(`Error serving VIDEO asset id ${asset.id}`, 'serveFile[VIDEO]');
         throw new InternalServerErrorException(`Failed to serve video asset ${e}`, 'ServeFile');
       }
     }
@@ -334,11 +355,11 @@ export class AssetService {
     // TODO: should use query builder
     const rows = await this.assetRepository.query(
       `
-      select distinct si.tags, si.objects, e.orientation, e."lensModel", e.make, e.model , a.type, e.city, e.state, e.country
-      from assets a
-      left join exif e on a.id = e."assetId"
-      left join smart_info si on a.id = si."assetId"
-      where a."userId" = $1;
+      SELECT DISTINCT si.tags, si.objects, e.orientation, e."lensModel", e.make, e.model , a.type, e.city, e.state, e.country
+      FROM assets a
+      LEFT JOIN exif e ON a.id = e."assetId"
+      LEFT JOIN smart_info si ON a.id = si."assetId"
+      WHERE a."userId" = $1;
       `,
       [authUser.id],
     );
@@ -394,12 +415,12 @@ export class AssetService {
   async getCuratedLocation(authUser: AuthUserDto) {
     return await this.assetRepository.query(
       `
-        select distinct on (e.city) a.id, e.city, a."resizePath", a."deviceAssetId", a."deviceId"
-        from assets a
-        left join exif e on a.id = e."assetId"
-        where a."userId" = $1
-        and e.city is not null
-        and a.type = 'IMAGE';
+        SELECT DISTINCT ON (e.city) a.id, e.city, a."resizePath", a."deviceAssetId", a."deviceId"
+        FROM assets a
+        LEFT JOIN exif e ON a.id = e."assetId"
+        WHERE a."userId" = $1
+        AND e.city IS NOT NULL
+        AND a.type = 'IMAGE';
       `,
       [authUser.id],
     );
@@ -408,11 +429,11 @@ export class AssetService {
   async getCuratedObject(authUser: AuthUserDto) {
     return await this.assetRepository.query(
       `
-        select distinct on (unnest(si.objects)) a.id, unnest(si.objects) as "object", a."resizePath", a."deviceAssetId", a."deviceId"
-        from assets a
-        left join smart_info si on a.id = si."assetId"
-        where a."userId" = $1
-        and si.objects is not null
+        SELECT DISTINCT ON (unnest(si.objects)) a.id, unnest(si.objects) as "object", a."resizePath", a."deviceAssetId", a."deviceId"
+        FROM assets a
+        LEFT JOIN smart_info si ON a.id = si."assetId"
+        WHERE a."userId" = $1
+        AND si.objects IS NOT NULL
       `,
       [authUser.id],
     );

+ 1 - 2
server/apps/immich/src/api-v1/user/user.service.ts

@@ -154,7 +154,6 @@ export class UserService {
       }
 
       if (!user.profileImagePath) {
-        // throw new BadRequestException('User does not have a profile image');
         res.status(404).send('User does not have a profile image');
         return;
       }
@@ -165,7 +164,7 @@ export class UserService {
       const fileStream = createReadStream(user.profileImagePath);
       return new StreamableFile(fileStream);
     } catch (e) {
-      console.log('error getting user profile');
+      res.status(404).send('User does not have a profile image');
     }
   }
 }

+ 1 - 1
server/apps/microservices/src/processors/thumbnail.processor.ts

@@ -60,7 +60,7 @@ export class ThumbnailGeneratorProcessor {
 
     if (asset.type == AssetType.VIDEO) {
       ffmpeg(asset.originalPath)
-        .outputOptions(['-ss 00:00:01.000', '-frames:v 1'])
+        .outputOptions(['-ss 00:00:00.000', '-frames:v 1'])
         .output(jpegThumbnailPath)
         .on('start', () => {
           Logger.log('Start Generating Video Thumbnail', 'generateJPEGThumbnail');

+ 10 - 1
web/src/lib/components/shared/upload-panel.svelte

@@ -5,6 +5,8 @@
 	import CloudUploadOutline from 'svelte-material-icons/CloudUploadOutline.svelte';
 	import WindowMinimize from 'svelte-material-icons/WindowMinimize.svelte';
 	import type { UploadAsset } from '$lib/models/upload-asset';
+	import { getAssetsInfo } from '$lib/stores/assets';
+	import { session } from '$app/stores';
 
 	let showDetail = true;
 
@@ -73,8 +75,15 @@
 	}
 
 	let isUploading = false;
+	uploadAssetsStore.isUploading.subscribe((value) => {
+		isUploading = value;
 
-	uploadAssetsStore.isUploading.subscribe((value) => (isUploading = value));
+		if (isUploading == false) {
+			if ($session.user) {
+				getAssetsInfo($session.user.accessToken);
+			}
+		}
+	});
 </script>
 
 {#if isUploading}

+ 1 - 2
web/src/lib/stores/assets.ts

@@ -2,8 +2,8 @@ import { writable, derived } from 'svelte/store';
 import { getRequest } from '$lib/api';
 import type { ImmichAsset } from '$lib/models/immich-asset';
 import lodash from 'lodash-es';
+import _ from 'lodash';
 import moment from 'moment';
-
 export const assets = writable<ImmichAsset[]>([]);
 
 export const assetsGroupByDate = derived(assets, ($assets) => {
@@ -14,7 +14,6 @@ export const assetsGroupByDate = derived(assets, ($assets) => {
 			.sortBy((group) => $assets.indexOf(group[0]))
 			.value();
 	} catch (e) {
-		console.log('error deriving state assets', e);
 		return [];
 	}
 });

+ 10 - 3
web/src/lib/stores/websocket.ts

@@ -1,12 +1,16 @@
 import { Socket, io } from 'socket.io-client';
+import { writable } from 'svelte/store';
 import { serverEndpoint } from '../constants';
 import type { ImmichAsset } from '../models/immich-asset';
 import { assets } from './assets';
 
+let websocket: Socket;
+
 export const openWebsocketConnection = (accessToken: string) => {
 	const websocketEndpoint = serverEndpoint.replace('/api', '');
+
 	try {
-		const websocket = io(websocketEndpoint, {
+		websocket = io(websocketEndpoint, {
 			path: '/api/socket.io',
 			transports: ['polling'],
 			reconnection: true,
@@ -26,11 +30,14 @@ export const openWebsocketConnection = (accessToken: string) => {
 const listenToEvent = (socket: Socket) => {
 	socket.on('on_upload_success', (data) => {
 		const newUploadedAsset: ImmichAsset = JSON.parse(data);
-
-		assets.update((assets) => [...assets, newUploadedAsset]);
+		// assets.update((assets) => [...assets, newUploadedAsset]);
 	});
 
 	socket.on('error', (e) => {
 		console.log('Websocket Error', e);
 	});
 };
+
+export const closeWebsocketConnection = () => {
+	websocket?.close();
+};

+ 1 - 1
web/src/lib/utils/file-uploader.ts

@@ -84,7 +84,7 @@ export async function fileUploader(asset: File, accessToken: string) {
 		request.upload.onload = () => {
 			setTimeout(() => {
 				uploadAssetsStore.removeUploadAsset(deviceAssetId);
-			}, 2500);
+			}, 1000);
 		};
 
 		// listen for `error` event

+ 6 - 2
web/src/routes/photos/index.svelte

@@ -31,7 +31,7 @@
 
 	import ImageOutline from 'svelte-material-icons/ImageOutline.svelte';
 	import { AppSideBarSelection } from '$lib/models/admin-sidebar-selection';
-	import { onMount } from 'svelte';
+	import { onDestroy, onMount } from 'svelte';
 	import { fly } from 'svelte/transition';
 	import { session } from '$app/stores';
 	import { assetsGroupByDate, flattenAssetGroupByDate } from '$lib/stores/assets';
@@ -42,7 +42,7 @@
 	import DownloadPanel from '../../lib/components/asset-viewer/download-panel.svelte';
 	import StatusBox from '../../lib/components/shared/status-box.svelte';
 	import { fileUploader } from '../../lib/utils/file-uploader';
-	import { openWebsocketConnection } from '../../lib/stores/websocket';
+	import { openWebsocketConnection, closeWebsocketConnection } from '../../lib/stores/websocket';
 
 	export let user: ImmichUser;
 	let selectedAction: AppSideBarSelection;
@@ -71,6 +71,10 @@
 		}
 	});
 
+	onDestroy(() => {
+		closeWebsocketConnection();
+	});
+
 	const thumbnailMouseEventHandler = (event: CustomEvent) => {
 		const { selectedGroupIndex }: { selectedGroupIndex: number } = event.detail;