Skip to content

Commit

Permalink
more explicit error messages for file uploads
Browse files Browse the repository at this point in the history
  • Loading branch information
rigelk committed Dec 2, 2020
1 parent 387d041 commit def0ac1
Show file tree
Hide file tree
Showing 14 changed files with 155 additions and 56 deletions.
Original file line number Diff line number Diff line change
@@ -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',
Expand Down Expand Up @@ -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
})
)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,30 @@
</div>
</div>

<!-- Upload progress/cancel/error/success header -->
<div *ngIf="isUploadingVideo && !error" class="upload-progress-cancel">
<div class="progress" i18n-title title="Total video quota">
<div class="progress" i18n-title title="Total video uploaded">
<div class="progress-bar" role="progressbar" [style]="{ width: videoUploadPercents + '%' }" [attr.aria-valuenow]="videoUploadPercents" aria-valuemin="0" [attr.aria-valuemax]="100">
<span *ngIf="videoUploadPercents === 100 && videoUploaded === false" i18n>Processing…</span>
<span *ngIf="videoUploadPercents !== 100 || videoUploaded">{{ videoUploadPercents }}%</span>
</div>
</div>
<input *ngIf="videoUploaded === false" type="button" value="Cancel" (click)="cancelUpload()" />
<input *ngIf="videoUploaded === false" type="button" i18n-value="Cancel ongoing upload of a video" value="Cancel" (click)="cancelUpload()" />
</div>

<div *ngIf="error" class="alert alert-danger">
<div *ngIf="error && enableRetryAfterError" class="upload-progress-retry">
<div class="progress">
<div class="progress-bar red" role="progressbar" [style]="{ width: '100%' }" [attr.aria-valuenow]="100" aria-valuemin="0" [attr.aria-valuemax]="100">
<span>{{ error }}</span>
</div>
</div>
<div class="btn-group" role="group">
<input type="button" class="btn" i18n-value="Retry failed upload of a video" value="Retry" (click)="retryUpload()" />
<input type="button" class="btn" i18n-value="Cancel ongoing upload of a video" value="Cancel" (click)="cancelUpload()" />
</div>
</div>

<div *ngIf="error && !enableRetryAfterError" class="alert alert-danger">
<div i18n>Sorry, but something went wrong</div>
{{ error }}
</div>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}
Expand All @@ -47,4 +47,8 @@

margin-left: 10px;
}

.btn-group > input:not(:first-child) {
margin-left: 0;
}
}
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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()
Expand All @@ -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`)
}
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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
})
}
)
}
Expand Down
20 changes: 12 additions & 8 deletions client/src/app/core/notification/notifier.service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
}
}
1 change: 0 additions & 1 deletion client/src/app/helpers/constants.ts

This file was deleted.

33 changes: 32 additions & 1 deletion client/src/app/helpers/utils.ts
Original file line number Diff line number Diff line change
@@ -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) {
Expand Down Expand Up @@ -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,
Expand All @@ -187,5 +217,6 @@ export {
removeElementFromArray,
scrollToTop,
isInViewport,
isXPercentInViewport
isXPercentInViewport,
uploadErrorHandler
}
6 changes: 3 additions & 3 deletions client/src/app/shared/shared-main/video/video.service.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down
5 changes: 5 additions & 0 deletions client/src/sass/bootstrap.scss
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
4 changes: 4 additions & 0 deletions client/src/sass/include/_mixins.scss
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,10 @@
&.secondary {
background-color: pvar(--secondaryColor);
}

&.red {
background-color: lighten($color: #c54130, $amount: 10);
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions server/controllers/api/videos/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -174,8 +174,8 @@ function listVideoPrivacies (req: express.Request, res: express.Response) {
}

async function addVideo (req: express.Request, res: express.Response) {
// Processing the video could be long
// Set timeout to 10 minutes
// Transferring the video could be long
// Set timeout to 10 minutes, as Express's default is 2 minutes
req.setTimeout(1000 * 60 * 10, () => {
logger.error('Upload video has timed out.')
return res.sendStatus(408)
Expand Down
1 change: 1 addition & 0 deletions shared/core-utils/miscs/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export * from './date'
export * from './miscs'
export * from './types'
export * from './http-error-codes'
16 changes: 16 additions & 0 deletions support/doc/api/openapi.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -972,6 +972,14 @@ paths:
application/json:
schema:
$ref: '#/components/schemas/Avatar'
'413':
description: image file too large
headers:
X-File-Maximum-Size:
schema:
type: string
format: Nginx size
description: Maximum file size for the avatar
requestBody:
content:
multipart/form-data:
Expand Down Expand Up @@ -1308,6 +1316,14 @@ paths:
description: user video quota is exceeded with this video
'408':
description: upload has timed out
'413':
description: video file too large
headers:
X-File-Maximum-Size:
schema:
type: string
format: Nginx size
description: Maximum file size for the video
'422':
description: invalid input file
requestBody:
Expand Down
Loading

0 comments on commit def0ac1

Please sign in to comment.