From d4132d3f56b392a2e4e632db59e6644e4851228e Mon Sep 17 00:00:00 2001 From: Rigel Kent Date: Mon, 23 Nov 2020 10:45:42 +0100 Subject: more explicit error messages for file uploads --- .../my-account-settings.component.ts | 8 ++- .../video-upload.component.html | 19 ++++++-- .../video-upload.component.scss | 12 +++-- .../video-add-components/video-upload.component.ts | 57 ++++++++++++++-------- .../src/app/core/notification/notifier.service.ts | 20 +++++--- client/src/app/helpers/utils.ts | 33 ++++++++++++- .../app/shared/shared-main/video/video.service.ts | 6 +-- client/src/sass/bootstrap.scss | 5 ++ client/src/sass/include/_mixins.scss | 4 ++ 9 files changed, 125 insertions(+), 39 deletions(-) (limited to 'client/src') diff --git a/client/src/app/+my-account/my-account-settings/my-account-settings.component.ts b/client/src/app/+my-account/my-account-settings/my-account-settings.component.ts index 7ea4610d4..d5d019b35 100644 --- a/client/src/app/+my-account/my-account-settings/my-account-settings.component.ts +++ b/client/src/app/+my-account/my-account-settings/my-account-settings.component.ts @@ -1,6 +1,8 @@ import { ViewportScroller } from '@angular/common' +import { HttpErrorResponse } from '@angular/common/http' import { AfterViewChecked, Component, OnInit } from '@angular/core' import { AuthService, Notifier, User, UserService } from '@app/core' +import { uploadErrorHandler } from '@app/helpers' @Component({ selector: 'my-account-settings', @@ -44,7 +46,11 @@ export class MyAccountSettingsComponent implements OnInit, AfterViewChecked { this.user.updateAccountAvatar(data.avatar) }, - err => this.notifier.error(err.message) + (err: HttpErrorResponse) => uploadErrorHandler({ + err, + name: $localize`avatar`, + notifier: this.notifier + }) ) } } diff --git a/client/src/app/+videos/+video-edit/video-add-components/video-upload.component.html b/client/src/app/+videos/+video-edit/video-add-components/video-upload.component.html index 677fa1197..88ee4e32a 100644 --- a/client/src/app/+videos/+video-edit/video-add-components/video-upload.component.html +++ b/client/src/app/+videos/+video-edit/video-add-components/video-upload.component.html @@ -44,17 +44,30 @@ +
-
+
Processing… {{ videoUploadPercents }}%
- +
-
+
+
+
+ {{ error }} +
+
+
+ + +
+
+ +
Sorry, but something went wrong
{{ error }}
diff --git a/client/src/app/+videos/+video-edit/video-add-components/video-upload.component.scss b/client/src/app/+videos/+video-edit/video-add-components/video-upload.component.scss index 9ebfa2f2f..9549257f6 100644 --- a/client/src/app/+videos/+video-edit/video-add-components/video-upload.component.scss +++ b/client/src/app/+videos/+video-edit/video-add-components/video-upload.component.scss @@ -16,27 +16,27 @@ } } +.upload-progress-retry, .upload-progress-cancel { display: flex; - margin-top: 25px; margin-bottom: 40px; .progress { @include progressbar; flex-grow: 1; height: 30px; - font-size: 15px; + font-size: 14px; background-color: rgba(11, 204, 41, 0.16); .progress-bar { background-color: $green; line-height: 30px; text-align: left; - font-weight: $font-bold; + font-weight: $font-semibold; span { color: pvar(--mainBackgroundColor); - margin-left: 18px; + margin-left: 13px; } } } @@ -47,4 +47,8 @@ margin-left: 10px; } + + .btn-group > input:not(:first-child) { + margin-left: 0; + } } diff --git a/client/src/app/+videos/+video-edit/video-add-components/video-upload.component.ts b/client/src/app/+videos/+video-edit/video-add-components/video-upload.component.ts index 258f5c7a0..fdd0a56e5 100644 --- a/client/src/app/+videos/+video-edit/video-add-components/video-upload.component.ts +++ b/client/src/app/+videos/+video-edit/video-add-components/video-upload.component.ts @@ -1,9 +1,9 @@ import { Subscription } from 'rxjs' -import { HttpEventType, HttpResponse } from '@angular/common/http' +import { HttpErrorResponse, HttpEventType, HttpResponse } from '@angular/common/http' import { Component, ElementRef, EventEmitter, OnDestroy, OnInit, Output, ViewChild } from '@angular/core' import { Router } from '@angular/router' import { AuthService, CanComponentDeactivate, Notifier, ServerService, UserService } from '@app/core' -import { scrollToTop } from '@app/helpers' +import { scrollToTop, uploadErrorHandler } from '@app/helpers' import { FormValidatorService } from '@app/shared/shared-forms' import { BytesPipe, VideoCaptionService, VideoEdit, VideoService } from '@app/shared/shared-main' import { LoadingBarService } from '@ngx-loading-bar/core' @@ -41,11 +41,13 @@ export class VideoUploadComponent extends VideoSend implements OnInit, OnDestroy id: 0, uuid: '' } + formData: FormData waitTranscodingEnabled = true previewfileUpload: File error: string + enableRetryAfterError: boolean protected readonly DEFAULT_VIDEO_PRIVACY = VideoPrivacy.PUBLIC @@ -118,6 +120,12 @@ export class VideoUploadComponent extends VideoSend implements OnInit, OnDestroy this.uploadFirstStep() } + retryUpload () { + this.enableRetryAfterError = false + this.error = '' + this.uploadVideo() + } + cancelUpload () { if (this.videoUploadObservable !== null) { this.videoUploadObservable.unsubscribe() @@ -127,6 +135,8 @@ export class VideoUploadComponent extends VideoSend implements OnInit, OnDestroy this.videoUploadObservable = null this.firstStepError.emit() + this.enableRetryAfterError = false + this.error = '' this.notifier.info($localize`Upload cancelled`) } @@ -163,20 +173,20 @@ export class VideoUploadComponent extends VideoSend implements OnInit, OnDestroy const downloadEnabled = true const channelId = this.firstStepChannelId.toString() - const formData = new FormData() - formData.append('name', name) + this.formData = new FormData() + this.formData.append('name', name) // Put the video "private" -> we are waiting the user validation of the second step - formData.append('privacy', VideoPrivacy.PRIVATE.toString()) - formData.append('nsfw', '' + nsfw) - formData.append('commentsEnabled', '' + commentsEnabled) - formData.append('downloadEnabled', '' + downloadEnabled) - formData.append('waitTranscoding', '' + waitTranscoding) - formData.append('channelId', '' + channelId) - formData.append('videofile', videofile) + this.formData.append('privacy', VideoPrivacy.PRIVATE.toString()) + this.formData.append('nsfw', '' + nsfw) + this.formData.append('commentsEnabled', '' + commentsEnabled) + this.formData.append('downloadEnabled', '' + downloadEnabled) + this.formData.append('waitTranscoding', '' + waitTranscoding) + this.formData.append('channelId', '' + channelId) + this.formData.append('videofile', videofile) if (this.previewfileUpload) { - formData.append('previewfile', this.previewfileUpload) - formData.append('thumbnailfile', this.previewfileUpload) + this.formData.append('previewfile', this.previewfileUpload) + this.formData.append('thumbnailfile', this.previewfileUpload) } this.isUploadingVideo = true @@ -190,7 +200,11 @@ export class VideoUploadComponent extends VideoSend implements OnInit, OnDestroy previewfile: this.previewfileUpload }) - this.videoUploadObservable = this.videoService.uploadVideo(formData).subscribe( + this.uploadVideo() + } + + uploadVideo () { + this.videoUploadObservable = this.videoService.uploadVideo(this.formData).subscribe( event => { if (event.type === HttpEventType.UploadProgress) { this.videoUploadPercents = Math.round(100 * event.loaded / event.total) @@ -203,13 +217,18 @@ export class VideoUploadComponent extends VideoSend implements OnInit, OnDestroy } }, - err => { - // Reset progress - this.isUploadingVideo = false + (err: HttpErrorResponse) => { + // Reset progress (but keep isUploadingVideo true) this.videoUploadPercents = 0 this.videoUploadObservable = null - this.firstStepError.emit() - this.notifier.error(err.message) + this.enableRetryAfterError = true + + this.error = uploadErrorHandler({ + err, + name: $localize`video`, + notifier: this.notifier, + sticky: false + }) } ) } diff --git a/client/src/app/core/notification/notifier.service.ts b/client/src/app/core/notification/notifier.service.ts index f736672bb..165bb0c76 100644 --- a/client/src/app/core/notification/notifier.service.ts +++ b/client/src/app/core/notification/notifier.service.ts @@ -7,31 +7,35 @@ export class Notifier { constructor (private messageService: MessageService) { } - info (text: string, title?: string, timeout?: number) { + info (text: string, title?: string, timeout?: number, sticky?: boolean) { if (!title) title = $localize`Info` - return this.notify('info', text, title, timeout) + console.info(`${title}: ${text}`) + return this.notify('info', text, title, timeout, sticky) } - error (text: string, title?: string, timeout?: number) { + error (text: string, title?: string, timeout?: number, sticky?: boolean) { if (!title) title = $localize`Error` - return this.notify('error', text, title, timeout) + console.error(`${title}: ${text}`) + return this.notify('error', text, title, timeout, sticky) } - success (text: string, title?: string, timeout?: number) { + success (text: string, title?: string, timeout?: number, sticky?: boolean) { if (!title) title = $localize`Success` - return this.notify('success', text, title, timeout) + console.log(`${title}: ${text}`) + return this.notify('success', text, title, timeout, sticky) } - private notify (severity: 'success' | 'info' | 'warn' | 'error', text: string, title: string, timeout?: number) { + private notify (severity: 'success' | 'info' | 'warn' | 'error', text: string, title: string, timeout?: number, sticky?: boolean) { this.messageService.add({ severity, summary: title, detail: text, closable: true, - life: timeout || this.TIMEOUT + life: timeout || this.TIMEOUT, + sticky }) } } diff --git a/client/src/app/helpers/utils.ts b/client/src/app/helpers/utils.ts index 9c805b4ca..f96f26fff 100644 --- a/client/src/app/helpers/utils.ts +++ b/client/src/app/helpers/utils.ts @@ -1,7 +1,10 @@ import { DatePipe } from '@angular/common' +import { HttpErrorResponse } from '@angular/common/http' +import { Notifier } from '@app/core' import { SelectChannelItem } from '@app/shared/shared-forms' import { environment } from '../../environments/environment' import { AuthService } from '../core/auth' +import { HttpStatusCode } from '@shared/core-utils/miscs/http-error-codes' // Thanks: https://stackoverflow.com/questions/901115/how-can-i-get-query-string-values-in-javascript function getParameterByName (name: string, url: string) { @@ -172,6 +175,33 @@ function isXPercentInViewport (el: HTMLElement, percentVisible: number) { ) } +function uploadErrorHandler (parameters: { + err: HttpErrorResponse + name: string + notifier: Notifier + sticky?: boolean +}) { + const { err, name, notifier, sticky } = { sticky: false, ...parameters } + const title = $localize`The upload failed` + let message = err.message + + if (err instanceof ErrorEvent) { // network error + message = $localize`The connection was interrupted` + notifier.error(message, title, null, sticky) + } else if (err.status === HttpStatusCode.REQUEST_TIMEOUT_408) { + message = $localize`Your ${name} file couldn't be transferred before the set timeout (usually 10min)` + notifier.error(message, title, null, sticky) + } else if (err.status === HttpStatusCode.PAYLOAD_TOO_LARGE_413) { + const maxFileSize = err.headers?.get('X-File-Maximum-Size') || '8G' + message = $localize`Your ${name} file was too large (max. size: ${maxFileSize})` + notifier.error(message, title, null, sticky) + } else { + notifier.error(err.message, title) + } + + return message +} + export { sortBy, durationToString, @@ -187,5 +217,6 @@ export { removeElementFromArray, scrollToTop, isInViewport, - isXPercentInViewport + isXPercentInViewport, + uploadErrorHandler } diff --git a/client/src/app/shared/shared-main/video/video.service.ts b/client/src/app/shared/shared-main/video/video.service.ts index 70be5d7d2..59860c5cb 100644 --- a/client/src/app/shared/shared-main/video/video.service.ts +++ b/client/src/app/shared/shared-main/video/video.service.ts @@ -1,6 +1,6 @@ -import { Observable } from 'rxjs' -import { catchError, map, switchMap } from 'rxjs/operators' -import { HttpClient, HttpParams, HttpRequest } from '@angular/common/http' +import { Observable, of, throwError } from 'rxjs' +import { catchError, map, mergeMap, switchMap } from 'rxjs/operators' +import { HttpClient, HttpErrorResponse, HttpParams, HttpRequest } from '@angular/common/http' import { Injectable } from '@angular/core' import { ComponentPaginationLight, RestExtractor, RestService, ServerService, UserService, AuthService } from '@app/core' import { objectToFormData } from '@app/helpers' diff --git a/client/src/sass/bootstrap.scss b/client/src/sass/bootstrap.scss index b90bffbfc..208c7f582 100644 --- a/client/src/sass/bootstrap.scss +++ b/client/src/sass/bootstrap.scss @@ -44,6 +44,11 @@ $icon-font-path: '~@neos21/bootstrap3-glyphicons/assets/fonts/'; z-index: inherit !important; } +.btn-group > .btn:not(:first-child) { + border-top-left-radius: 0 !important; + border-bottom-left-radius: 0 !important; +} + .dropdown-menu { z-index: z(dropdown) + 1 !important; diff --git a/client/src/sass/include/_mixins.scss b/client/src/sass/include/_mixins.scss index 1a94de5b2..fecae9fbc 100644 --- a/client/src/sass/include/_mixins.scss +++ b/client/src/sass/include/_mixins.scss @@ -732,6 +732,10 @@ &.secondary { background-color: pvar(--secondaryColor); } + + &.red { + background-color: lighten($color: #c54130, $amount: 10); + } } } -- cgit v1.2.3