From f2eb23cd87cf32b8fe545178143b5f49e06a58da Mon Sep 17 00:00:00 2001 From: Rigel Kent Date: Tue, 8 Dec 2020 21:16:10 +0100 Subject: emit more specific status codes on video upload (#3423) - reduce http status codes list to potentially useful codes - convert more codes to typed ones - factorize html generator for error responses --- .../contact-admin-modal.component.ts | 3 ++- client/src/app/+accounts/accounts.component.ts | 6 +++++- .../my-video-channel-create.component.ts | 3 ++- .../+video-channels/video-channels.component.ts | 6 +++++- .../video-add-components/video-upload.component.ts | 22 ++++++++++++-------- .../+videos/+video-watch/video-watch.component.ts | 24 +++++++++++++++++++--- client/src/app/core/auth/auth.service.ts | 3 ++- client/src/app/core/rest/rest-extractor.service.ts | 9 ++++---- 8 files changed, 56 insertions(+), 20 deletions(-) (limited to 'client/src/app') diff --git a/client/src/app/+about/about-instance/contact-admin-modal.component.ts b/client/src/app/+about/about-instance/contact-admin-modal.component.ts index 11e442f6b..ac2a6c980 100644 --- a/client/src/app/+about/about-instance/contact-admin-modal.component.ts +++ b/client/src/app/+about/about-instance/contact-admin-modal.component.ts @@ -10,6 +10,7 @@ import { FormReactive, FormValidatorService } from '@app/shared/shared-forms' import { InstanceService } from '@app/shared/shared-instance' import { NgbModal } from '@ng-bootstrap/ng-bootstrap' import { NgbModalRef } from '@ng-bootstrap/ng-bootstrap/modal/modal-ref' +import { HttpStatusCode } from '@shared/core-utils/miscs/http-error-codes' import { ServerConfig } from '@shared/models' @Component({ @@ -78,7 +79,7 @@ export class ContactAdminModalComponent extends FormReactive implements OnInit { }, err => { - this.error = err.status === 403 + this.error = err.status === HttpStatusCode.FORBIDDEN_403 ? $localize`You already sent this form recently` : err.message } diff --git a/client/src/app/+accounts/accounts.component.ts b/client/src/app/+accounts/accounts.component.ts index dbc7c8887..4820eaf32 100644 --- a/client/src/app/+accounts/accounts.component.ts +++ b/client/src/app/+accounts/accounts.component.ts @@ -6,6 +6,7 @@ import { AuthService, Notifier, RedirectService, RestExtractor, ScreenService, U import { Account, AccountService, DropdownAction, ListOverflowItem, VideoChannel, VideoChannelService } from '@app/shared/shared-main' import { AccountReportComponent } from '@app/shared/shared-moderation' import { User, UserRight } from '@shared/models' +import { HttpStatusCode } from '@shared/core-utils/miscs/http-error-codes' @Component({ templateUrl: './accounts.component.html', @@ -47,7 +48,10 @@ export class AccountsComponent implements OnInit, OnDestroy { switchMap(accountId => this.accountService.getAccount(accountId)), tap(account => this.onAccount(account)), switchMap(account => this.videoChannelService.listAccountVideoChannels(account)), - catchError(err => this.restExtractor.redirectTo404IfNotFound(err, [ 400, 404 ])) + catchError(err => this.restExtractor.redirectTo404IfNotFound(err, [ + HttpStatusCode.BAD_REQUEST_400, + HttpStatusCode.NOT_FOUND_404 + ])) ) .subscribe( videoChannels => this.videoChannels = videoChannels.data, diff --git a/client/src/app/+my-library/+my-video-channels/my-video-channel-create.component.ts b/client/src/app/+my-library/+my-video-channels/my-video-channel-create.component.ts index 1d0cbf246..a625493de 100644 --- a/client/src/app/+my-library/+my-video-channels/my-video-channel-create.component.ts +++ b/client/src/app/+my-library/+my-video-channels/my-video-channel-create.component.ts @@ -10,6 +10,7 @@ import { import { FormValidatorService } from '@app/shared/shared-forms' import { VideoChannelService } from '@app/shared/shared-main' import { VideoChannelCreate } from '@shared/models' +import { HttpStatusCode } from '@shared/core-utils/miscs/http-error-codes' import { MyVideoChannelEdit } from './my-video-channel-edit' @Component({ @@ -58,7 +59,7 @@ export class MyVideoChannelCreateComponent extends MyVideoChannelEdit implements }, err => { - if (err.status === 409) { + if (err.status === HttpStatusCode.CONFLICT_409) { this.error = $localize`This name already exists on this instance.` return } diff --git a/client/src/app/+video-channels/video-channels.component.ts b/client/src/app/+video-channels/video-channels.component.ts index ea8bda1cf..d2fd265c4 100644 --- a/client/src/app/+video-channels/video-channels.component.ts +++ b/client/src/app/+video-channels/video-channels.component.ts @@ -6,6 +6,7 @@ import { ActivatedRoute } from '@angular/router' import { AuthService, Notifier, RestExtractor, ScreenService } from '@app/core' import { ListOverflowItem, VideoChannel, VideoChannelService } from '@app/shared/shared-main' import { SubscribeButtonComponent } from '@app/shared/shared-user-subscription' +import { HttpStatusCode } from '@shared/core-utils/miscs/http-error-codes' @Component({ templateUrl: './video-channels.component.html', @@ -37,7 +38,10 @@ export class VideoChannelsComponent implements OnInit, OnDestroy { map(params => params[ 'videoChannelName' ]), distinctUntilChanged(), switchMap(videoChannelName => this.videoChannelService.getVideoChannel(videoChannelName)), - catchError(err => this.restExtractor.redirectTo404IfNotFound(err, [ 400, 404 ])) + catchError(err => this.restExtractor.redirectTo404IfNotFound(err, [ + HttpStatusCode.BAD_REQUEST_400, + HttpStatusCode.NOT_FOUND_404 + ])) ) .subscribe(videoChannel => { this.videoChannel = videoChannel 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 bee3679f7..cafb030b9 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 @@ -9,6 +9,7 @@ import { BytesPipe, VideoCaptionService, VideoEdit, VideoService } from '@app/sh import { LoadingBarService } from '@ngx-loading-bar/core' import { VideoPrivacy } from '@shared/models' import { VideoSend } from './video-send' +import { HttpStatusCode } from '@shared/core-utils/miscs/http-error-codes' @Component({ selector: 'my-video-upload', @@ -129,17 +130,17 @@ export class VideoUploadComponent extends VideoSend implements OnInit, OnDestroy cancelUpload () { if (this.videoUploadObservable !== null) { this.videoUploadObservable.unsubscribe() + } - this.isUploadingVideo = false - this.videoUploadPercents = 0 - this.videoUploadObservable = null + this.isUploadingVideo = false + this.videoUploadPercents = 0 + this.videoUploadObservable = null - this.firstStepError.emit() - this.enableRetryAfterError = false - this.error = '' + this.firstStepError.emit() + this.enableRetryAfterError = false + this.error = '' - this.notifier.info($localize`Upload cancelled`) - } + this.notifier.info($localize`Upload cancelled`) } uploadFirstStep (clickedOnButton = false) { @@ -229,6 +230,11 @@ export class VideoUploadComponent extends VideoSend implements OnInit, OnDestroy notifier: this.notifier, sticky: false }) + + if (err.status === HttpStatusCode.PAYLOAD_TOO_LARGE_413 || + err.status === HttpStatusCode.UNSUPPORTED_MEDIA_TYPE_415) { + this.cancelUpload() + } } ) } diff --git a/client/src/app/+videos/+video-watch/video-watch.component.ts b/client/src/app/+videos/+video-watch/video-watch.component.ts index b15de2a79..33de901c0 100644 --- a/client/src/app/+videos/+video-watch/video-watch.component.ts +++ b/client/src/app/+videos/+video-watch/video-watch.component.ts @@ -39,6 +39,7 @@ import { isWebRTCDisabled, timeToInt } from '../../../assets/player/utils' import { environment } from '../../../environments/environment' import { VideoSupportComponent } from './modal/video-support.component' import { VideoWatchPlaylistComponent } from './video-watch-playlist.component' +import { HttpStatusCode } from '@shared/core-utils/miscs/http-error-codes' type URLOptions = CustomizationOptions & { playerMode: PlayerMode } @@ -412,13 +413,25 @@ export class VideoWatchComponent implements OnInit, OnDestroy { $localize`This video is not available on this instance. Do you want to be redirected on the origin instance: ${originUrl}?`, $localize`Redirection` ).then(res => { - if (res === false) return this.restExtractor.redirectTo404IfNotFound(err, [ 400, 401, 403, 404 ]) + if (res === false) { + return this.restExtractor.redirectTo404IfNotFound(err, [ + HttpStatusCode.BAD_REQUEST_400, + HttpStatusCode.UNAUTHORIZED_401, + HttpStatusCode.FORBIDDEN_403, + HttpStatusCode.NOT_FOUND_404 + ]) + } return window.location.href = originUrl }) } - return this.restExtractor.redirectTo404IfNotFound(err, [ 400, 401, 403, 404 ]) + return this.restExtractor.redirectTo404IfNotFound(err, [ + HttpStatusCode.BAD_REQUEST_400, + HttpStatusCode.UNAUTHORIZED_401, + HttpStatusCode.FORBIDDEN_403, + HttpStatusCode.NOT_FOUND_404 + ]) }) ) .subscribe(([ video, captionsResult ]) => { @@ -450,7 +463,12 @@ export class VideoWatchComponent implements OnInit, OnDestroy { this.playlistService.getVideoPlaylist(playlistId) .pipe( // If 401, the video is private or blocked so redirect to 404 - catchError(err => this.restExtractor.redirectTo404IfNotFound(err, [ 400, 401, 403, 404 ])) + catchError(err => this.restExtractor.redirectTo404IfNotFound(err, [ + HttpStatusCode.BAD_REQUEST_400, + HttpStatusCode.UNAUTHORIZED_401, + HttpStatusCode.FORBIDDEN_403, + HttpStatusCode.NOT_FOUND_404 + ])) ) .subscribe(playlist => { this.playlist = playlist diff --git a/client/src/app/core/auth/auth.service.ts b/client/src/app/core/auth/auth.service.ts index fd6062d3f..cdf13186b 100644 --- a/client/src/app/core/auth/auth.service.ts +++ b/client/src/app/core/auth/auth.service.ts @@ -11,6 +11,7 @@ import { environment } from '../../../environments/environment' import { RestExtractor } from '../rest/rest-extractor.service' import { AuthStatus } from './auth-status.model' import { AuthUser } from './auth-user.model' +import { HttpStatusCode } from '@shared/core-utils/miscs/http-error-codes' interface UserLoginWithUsername extends UserLogin { access_token: string @@ -94,7 +95,7 @@ export class AuthService { error => { let errorMessage = error.message - if (error.status === 403) { + if (error.status === HttpStatusCode.FORBIDDEN_403) { errorMessage = $localize`Cannot retrieve OAuth Client credentials: ${error.text}. Ensure you have correctly configured PeerTube (config/ directory), in particular the "webserver" section.` } diff --git a/client/src/app/core/rest/rest-extractor.service.ts b/client/src/app/core/rest/rest-extractor.service.ts index 4b8c1e155..84d9ed074 100644 --- a/client/src/app/core/rest/rest-extractor.service.ts +++ b/client/src/app/core/rest/rest-extractor.service.ts @@ -3,6 +3,7 @@ import { Injectable } from '@angular/core' import { Router } from '@angular/router' import { dateToHuman } from '@app/helpers' import { ResultList } from '@shared/models' +import { HttpStatusCode } from '@shared/core-utils/miscs/http-error-codes' @Injectable() export class RestExtractor { @@ -57,9 +58,9 @@ export class RestExtractor { errorMessage = errorsArray.join('. ') } else if (err.error && err.error.error) { errorMessage = err.error.error - } else if (err.status === 413) { + } else if (err.status === HttpStatusCode.PAYLOAD_TOO_LARGE_413) { errorMessage = $localize`Media is too large for the server. Please contact you administrator if you want to increase the limit size.` - } else if (err.status === 429) { + } else if (err.status === HttpStatusCode.TOO_MANY_REQUESTS_429) { const secondsLeft = err.headers.get('retry-after') if (secondsLeft) { const minutesLeft = Math.floor(parseInt(secondsLeft, 10) / 60) @@ -67,7 +68,7 @@ export class RestExtractor { } else { errorMessage = $localize`Too many attempts, please try again later.` } - } else if (err.status === 500) { + } else if (err.status === HttpStatusCode.INTERNAL_SERVER_ERROR_500) { errorMessage = $localize`Server error. Please retry later.` } @@ -92,7 +93,7 @@ export class RestExtractor { return observableThrowError(errorObj) } - redirectTo404IfNotFound (obj: { status: number }, status = [ 404 ]) { + redirectTo404IfNotFound (obj: { status: number }, status = [ HttpStatusCode.NOT_FOUND_404 ]) { if (obj && obj.status && status.indexOf(obj.status) !== -1) { // Do not use redirectService to avoid circular dependencies this.router.navigate([ '/404' ], { skipLocationChange: true }) -- cgit v1.2.3