From d7f83948a1af0ef3bed61f83e87e826902c96f7d Mon Sep 17 00:00:00 2001
From: Chocobozzz <me@florianbigard.com>
Date: Fri, 3 Aug 2018 09:27:30 +0200
Subject: [PATCH] Correctly handle video import errors

---
 .../my-account-video-imports.component.html   | 19 +++++++++++++++++--
 .../my-account-video-imports.component.scss   |  8 ++++++++
 .../+video-edit/video-import.component.html   |  2 +-
 server/lib/job-queue/handlers/video-import.ts |  3 ++-
 server/lib/job-queue/job-queue.ts             |  5 ++++-
 server/models/video/video-import.ts           |  6 ++++--
 shared/models/videos/video-import.model.ts    |  2 ++
 7 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/client/src/app/+my-account/my-account-video-imports/my-account-video-imports.component.html b/client/src/app/+my-account/my-account-video-imports/my-account-video-imports.component.html
index 74ca33fa3..00b2d7cb0 100644
--- a/client/src/app/+my-account/my-account-video-imports/my-account-video-imports.component.html
+++ b/client/src/app/+my-account/my-account-video-imports/my-account-video-imports.component.html
@@ -1,9 +1,10 @@
 <p-table
   [value]="videoImports" [lazy]="true" [paginator]="true" [totalRecords]="totalRecords" [rows]="rowsPerPage"
-  [sortField]="sort.field" [sortOrder]="sort.order" (onLazyLoad)="loadLazy($event)"
+  [sortField]="sort.field" [sortOrder]="sort.order" (onLazyLoad)="loadLazy($event)" dataKey="id"
 >
   <ng-template pTemplate="header">
     <tr>
+      <th style="width: 40px;"></th>
       <th i18n>URL</th>
       <th i18n>Video</th>
       <th i18n style="width: 150px">State</th>
@@ -12,8 +13,14 @@
     </tr>
   </ng-template>
 
-  <ng-template pTemplate="body" let-videoImport>
+  <ng-template pTemplate="body" let-expanded="expanded" let-videoImport>
     <tr>
+      <td>
+        <span *ngIf="videoImport.error" class="expander" [pRowToggler]="videoImport">
+          <i [ngClass]="expanded ? 'glyphicon glyphicon-menu-down' : 'glyphicon glyphicon-menu-right'"></i>
+        </span>
+      </td>
+
       <td>
         <a [href]="videoImport.targetUrl" target="_blank" rel="noopener noreferrer">{{ videoImport.targetUrl }}</a>
       </td>
@@ -34,4 +41,12 @@
       </td>
     </tr>
   </ng-template>
+
+  <ng-template pTemplate="rowexpansion" let-videoImport>
+    <tr class="video-import-error" *ngIf="videoImport.error">
+      <td colspan="6">
+        <pre>{{ videoImport.error }}</pre>
+      </td>
+    </tr>
+  </ng-template>
 </p-table>
diff --git a/client/src/app/+my-account/my-account-video-imports/my-account-video-imports.component.scss b/client/src/app/+my-account/my-account-video-imports/my-account-video-imports.component.scss
index 5e6774739..bdd2f8270 100644
--- a/client/src/app/+my-account/my-account-video-imports/my-account-video-imports.component.scss
+++ b/client/src/app/+my-account/my-account-video-imports/my-account-video-imports.component.scss
@@ -1,2 +1,10 @@
 @import '_variables';
 @import '_mixins';
+
+pre {
+  font-size: 11px;
+}
+
+.video-import-error {
+  color: red;
+}
\ No newline at end of file
diff --git a/client/src/app/videos/+video-edit/video-import.component.html b/client/src/app/videos/+video-edit/video-import.component.html
index d59c6a23a..cfad5e289 100644
--- a/client/src/app/videos/+video-edit/video-import.component.html
+++ b/client/src/app/videos/+video-edit/video-import.component.html
@@ -6,7 +6,7 @@
       <label i18n for="targetUrl">URL</label>
       <my-help
         helpType="custom" i18n-customHtml
-        customHtml="You can import any URL <a href='https://rg3.github.io/youtube-dl/supportedsites.html'>supported by youtube-dl</a> or URL that points to a raw MP4 file. Failure to secure these rights could cause legal trouble to yourself and your instance."
+        customHtml="You can import any URL <a href='https://rg3.github.io/youtube-dl/supportedsites.html' target='_blank' rel='noopener noreferrer'>supported by youtube-dl</a> or URL that points to a raw MP4 file. Failure to secure these rights could cause legal trouble to yourself and your instance."
       ></my-help>
 
       <input type="text" id="targetUrl" [(ngModel)]="targetUrl" />
diff --git a/server/lib/job-queue/handlers/video-import.ts b/server/lib/job-queue/handlers/video-import.ts
index 5a7722153..4f2faab7d 100644
--- a/server/lib/job-queue/handlers/video-import.ts
+++ b/server/lib/job-queue/handlers/video-import.ts
@@ -35,7 +35,7 @@ async function processVideoImport (job: Bull.Job) {
 
     // Get information about this video
     const { videoFileResolution } = await getVideoFileResolution(tempVideoPath)
-    const fps = await getVideoFileFPS(tempVideoPath + 's')
+    const fps = await getVideoFileFPS(tempVideoPath)
     const stats = await statPromise(tempVideoPath)
     const duration = await getDurationFromVideoFile(tempVideoPath)
 
@@ -115,6 +115,7 @@ async function processVideoImport (job: Bull.Job) {
       logger.error('Cannot cleanup files after a video import error.', { err: errUnlink })
     }
 
+    videoImport.error = err.message
     videoImport.state = VideoImportState.FAILED
     await videoImport.save()
 
diff --git a/server/lib/job-queue/job-queue.ts b/server/lib/job-queue/job-queue.ts
index 2e14867f2..ffd948b5f 100644
--- a/server/lib/job-queue/job-queue.ts
+++ b/server/lib/job-queue/job-queue.ts
@@ -79,7 +79,10 @@ class JobQueue {
       const handler = handlers[handlerName]
 
       queue.process(JOB_CONCURRENCY[handlerName], handler)
-        .catch(err => logger.error('Cannot execute job queue %s.', handlerName, { err }))
+
+      queue.on('failed', (job, err) => {
+        logger.error('Cannot execute job %d in queue %s.', job.id, handlerName, { payload: job.data, err })
+      })
 
       queue.on('error', err => {
         logger.error('Error in job queue %s.', handlerName, { err })
diff --git a/server/models/video/video-import.ts b/server/models/video/video-import.ts
index 6b8a16b65..c2e55509c 100644
--- a/server/models/video/video-import.ts
+++ b/server/models/video/video-import.ts
@@ -26,7 +26,7 @@ import { TagModel } from './tag'
   include: [
     {
       model: () => VideoModel,
-      required: true,
+      required: false,
       include: [
         {
           model: () => VideoChannelModel,
@@ -112,7 +112,7 @@ export class VideoImportModel extends Model<VideoImportModel> {
       include: [
         {
           model: VideoModel,
-          required: true,
+          required: false,
           include: [
             {
               model: VideoChannelModel,
@@ -157,11 +157,13 @@ export class VideoImportModel extends Model<VideoImportModel> {
       : undefined
 
     return {
+      id: this.id,
       targetUrl: this.targetUrl,
       state: {
         id: this.state,
         label: VideoImportModel.getStateLabel(this.state)
       },
+      error: this.error,
       updatedAt: this.updatedAt.toISOString(),
       createdAt: this.createdAt.toISOString(),
       video
diff --git a/shared/models/videos/video-import.model.ts b/shared/models/videos/video-import.model.ts
index b23e6b245..c8dea0246 100644
--- a/shared/models/videos/video-import.model.ts
+++ b/shared/models/videos/video-import.model.ts
@@ -3,10 +3,12 @@ import { VideoConstant } from './video-constant.model'
 import { VideoImportState } from '../../index'
 
 export interface VideoImport {
+  id: number
   targetUrl: string
   createdAt: string
   updatedAt: string
   state: VideoConstant<VideoImportState>
+  error?: string
 
   video?: Video & { tags: string[] }
 }
-- 
2.41.0