Refactor server errors handler

11 files changed, 169 insertions, 45 deletions

diff --git a/server/controllers/api/videos/index.ts b/server/controllers/api/videos/index.ts
index db23e5630..7671f099e 100644
--- a/server/controllers/api/videos/index.ts
+++ b/server/controllers/api/videos/index.ts
@@ -2,6 +2,7 @@ import * as express from 'express'
 import toInt from 'validator/lib/toInt'
 import { doJSONRequest } from '@server/helpers/requests'
 import { LiveManager } from '@server/lib/live-manager'
+import { docMiddleware } from '@server/middlewares/doc'
 import { getServerActor } from '@server/models/application/application'
 import { MVideoAccountLight } from '@server/types/models'
 import { VideosCommonQuery } from '../../../../shared'
@@ -83,6 +84,7 @@ videosRouter.get('/:id/metadata/:videoFileId',
   asyncMiddleware(getVideoFileMetadata)
 )
 videosRouter.get('/:id',
+  docMiddleware('https://docs.joinpeertube.org/api-rest-reference.html#operation/getVideo'),
   optionalAuthenticate,
   asyncMiddleware(videosCustomGetValidator('only-video-with-rights')),
   asyncMiddleware(checkVideoFollowConstraints),
@@ -94,6 +96,7 @@ videosRouter.post('/:id/views',
 )
 
 videosRouter.delete('/:id',
+  docMiddleware('https://docs.joinpeertube.org/api-rest-reference.html#operation/delVideo'),
   authenticate,
   asyncMiddleware(videosRemoveValidator),
   asyncRetryTransactionMiddleware(removeVideo)
diff --git a/server/controllers/api/videos/update.ts b/server/controllers/api/videos/update.ts
index 2450abd0e..09e584d30 100644
--- a/server/controllers/api/videos/update.ts
+++ b/server/controllers/api/videos/update.ts
@@ -20,6 +20,7 @@ import { autoBlacklistVideoIfNeeded } from '../../../lib/video-blacklist'
 import { asyncMiddleware, asyncRetryTransactionMiddleware, authenticate, videosUpdateValidator } from '../../../middlewares'
 import { ScheduleVideoUpdateModel } from '../../../models/video/schedule-video-update'
 import { VideoModel } from '../../../models/video/video'
+import { docMiddleware } from '@server/middlewares/doc'
 
 const lTags = loggerTagsFactory('api', 'video')
 const auditLogger = auditLoggerFactory('videos')
@@ -35,6 +36,7 @@ const reqVideoFileUpdate = createReqFiles(
 )
 
 updateRouter.put('/:id',
+  docMiddleware('https://docs.joinpeertube.org/api-rest-reference.html#operation/putVideo'),
   authenticate,
   reqVideoFileUpdate,
   asyncMiddleware(videosUpdateValidator),
diff --git a/server/controllers/api/videos/upload.ts b/server/controllers/api/videos/upload.ts
index c33d7fcb9..93a68f759 100644
--- a/server/controllers/api/videos/upload.ts
+++ b/server/controllers/api/videos/upload.ts
@@ -6,6 +6,7 @@ import { createTorrentAndSetInfoHash } from '@server/helpers/webtorrent'
 import { getLocalVideoActivityPubUrl } from '@server/lib/activitypub/url'
 import { addOptimizeOrMergeAudioJob, buildLocalVideoFromReq, buildVideoThumbnailsFromReq, setVideoTags } from '@server/lib/video'
 import { generateVideoFilename, getVideoFilePath } from '@server/lib/video-paths'
+import { docMiddleware } from '@server/middlewares/doc'
 import { MVideo, MVideoFile, MVideoFullLight } from '@server/types/models'
 import { uploadx } from '@uploadx/core'
 import { VideoCreate, VideoState } from '../../../../shared'
@@ -60,6 +61,7 @@ const reqVideoFileAddResumable = createReqFiles(
 )
 
 uploadRouter.post('/upload',
+  docMiddleware('https://docs.joinpeertube.org/api-rest-reference.html#operation/uploadLegacy'),
   authenticate,
   reqVideoFileAdd,
   asyncMiddleware(videosAddLegacyValidator),
@@ -67,6 +69,7 @@ uploadRouter.post('/upload',
 )
 
 uploadRouter.post('/upload-resumable',
+  docMiddleware('https://docs.joinpeertube.org/api-rest-reference.html#operation/uploadResumableInit'),
   authenticate,
   reqVideoFileAddResumable,
   asyncMiddleware(videosAddResumableInitValidator),
@@ -79,6 +82,7 @@ uploadRouter.delete('/upload-resumable',
 )
 
 uploadRouter.put('/upload-resumable',
+  docMiddleware('https://docs.joinpeertube.org/api-rest-reference.html#operation/uploadResumable'),
   authenticate,
   uploadxMiddleware, // uploadx doesn't use call next() before the file upload completes
   asyncMiddleware(videosAddResumableValidator),
diff --git a/server/helpers/express-utils.ts b/server/helpers/express-utils.ts
index 10a860787..010c6961a 100644
--- a/server/helpers/express-utils.ts
+++ b/server/helpers/express-utils.ts
@@ -8,7 +8,6 @@ import { isArray } from './custom-validators/misc'
 import { logger } from './logger'
 import { deleteFileAndCatch, generateRandomString } from './utils'
 import { getExtFromMimetype } from './video'
-import { ProblemDocument, ProblemDocumentExtension } from 'http-problem-details'
 
 function buildNSFWFilter (res?: express.Response, paramNSFW?: string) {
   if (paramNSFW === 'true') return true
@@ -126,34 +125,6 @@ function getCountVideos (req: express.Request) {
   return req.query.skipCount !== true
 }
 
-// helpers added in server.ts and used in subsequent controllers used
-const apiResponseHelpers = (req, res: express.Response, next = null) => {
-  res.fail = (options) => {
-    const { data, status = HttpStatusCode.BAD_REQUEST_400, message, title, type, docs = res.docs, instance } = options
-
-    const extension = new ProblemDocumentExtension({
-      ...data,
-      docs,
-      // fields for <= 3.2 compatibility, deprecated
-      error: message,
-      code: type
-    })
-
-    res.status(status)
-    res.setHeader('Content-Type', 'application/problem+json')
-    res.json(new ProblemDocument({
-      status,
-      title,
-      instance,
-      // fields intended to replace 'error' and 'code' respectively
-      detail: message,
-      type: type && 'https://docs.joinpeertube.org/api-rest-reference.html#section/Errors/' + type
-    }, extension))
-  }
-
-  if (next) next()
-}
-
 // ---------------------------------------------------------------------------
 
 export {
@@ -163,6 +134,5 @@ export {
   badRequest,
   createReqFiles,
   cleanUpReqFiles,
-  getCountVideos,
-  apiResponseHelpers
+  getCountVideos
 }
diff --git a/server/middlewares/doc.ts b/server/middlewares/doc.ts
new file mode 100644
index 000000000..aa852cd77
--- /dev/null
+++ b/server/middlewares/doc.ts
@@ -0,0 +1,13 @@
+import * as express from 'express'
+
+function docMiddleware (docUrl: string) {
+  return (req: express.Request, res: express.Response, next: express.NextFunction) => {
+    res.locals.docUrl = docUrl
+
+    if (next) return next()
+  }
+}
+
+export {
+  docMiddleware
+}
diff --git a/server/middlewares/error.ts b/server/middlewares/error.ts
new file mode 100644
index 000000000..e3eb1c8f5
--- /dev/null
+++ b/server/middlewares/error.ts
@@ -0,0 +1,39 @@
+import * as express from 'express'
+import { ProblemDocument, ProblemDocumentExtension } from 'http-problem-details'
+import { HttpStatusCode } from '@shared/core-utils'
+
+function apiFailMiddleware (req: express.Request, res: express.Response, next: express.NextFunction) {
+  res.fail = options => {
+    const { status = HttpStatusCode.BAD_REQUEST_400, message, title, type, data, instance } = options
+
+    const extension = new ProblemDocumentExtension({
+      ...data,
+
+      docs: res.locals.docUrl,
+      code: type,
+
+      // For <= 3.2 compatibility
+      error: message
+    })
+
+    res.status(status)
+    res.setHeader('Content-Type', 'application/problem+json')
+    res.json(new ProblemDocument({
+      status,
+      title,
+      instance,
+
+      detail: message,
+
+      type: type
+        ? `https://docs.joinpeertube.org/api-rest-reference.html#section/Errors/${type}`
+        : undefined
+    }, extension))
+  }
+
+  if (next) next()
+}
+
+export {
+  apiFailMiddleware
+}
diff --git a/server/middlewares/index.ts b/server/middlewares/index.ts
index 3e280e16f..413653dac 100644
--- a/server/middlewares/index.ts
+++ b/server/middlewares/index.ts
@@ -7,4 +7,6 @@ export * from './servers'
 export * from './sort'
 export * from './user-right'
 export * from './dnt'
+export * from './error'
+export * from './doc'
 export * from './csp'
diff --git a/server/middlewares/validators/videos/videos.ts b/server/middlewares/validators/videos/videos.ts
index 64e09234e..b7a9bcbe3 100644
--- a/server/middlewares/validators/videos/videos.ts
+++ b/server/middlewares/validators/videos/videos.ts
@@ -73,7 +73,6 @@ const videosAddLegacyValidator = getCommonVideoEditAttributes().concat([
     .custom(isIdValid).withMessage('Should have correct video channel id'),
 
   async (req: express.Request, res: express.Response, next: express.NextFunction) => {
-    res.docs = "https://docs.joinpeertube.org/api-rest-reference.html#operation/uploadLegacy"
     logger.debug('Checking videosAdd parameters', { parameters: req.body, files: req.files })
 
     if (areValidationErrors(req, res)) return cleanUpReqFiles(req)
@@ -108,7 +107,6 @@ const videosAddLegacyValidator = getCommonVideoEditAttributes().concat([
  */
 const videosAddResumableValidator = [
   async (req: express.Request, res: express.Response, next: express.NextFunction) => {
-    res.docs = "https://docs.joinpeertube.org/api-rest-reference.html#operation/uploadResumable"
     const user = res.locals.oauth.token.User
 
     const body: express.CustomUploadXFile<express.UploadXFileMetadata> = req.body
@@ -170,7 +168,6 @@ const videosAddResumableInitValidator = getCommonVideoEditAttributes().concat([
     .withMessage('Should specify the file mimetype'),
 
   async (req: express.Request, res: express.Response, next: express.NextFunction) => {
-    res.docs = "https://docs.joinpeertube.org/api-rest-reference.html#operation/uploadResumableInit"
     const videoFileMetadata = {
       mimetype: req.headers['x-upload-content-type'] as string,
       size: +req.headers['x-upload-content-length'],
@@ -214,7 +211,6 @@ const videosUpdateValidator = getCommonVideoEditAttributes().concat([
     .custom(isIdValid).withMessage('Should have correct video channel id'),
 
   async (req: express.Request, res: express.Response, next: express.NextFunction) => {
-    res.docs = 'https://docs.joinpeertube.org/api-rest-reference.html#operation/putVideo'
     logger.debug('Checking videosUpdate parameters', { parameters: req.body })
 
     if (areValidationErrors(req, res)) return cleanUpReqFiles(req)
@@ -268,7 +264,6 @@ const videosCustomGetValidator = (
     param('id').custom(isIdOrUUIDValid).not().isEmpty().withMessage('Should have a valid id'),
 
     async (req: express.Request, res: express.Response, next: express.NextFunction) => {
-      res.docs = 'https://docs.joinpeertube.org/api-rest-reference.html#operation/getVideo'
       logger.debug('Checking videosGet parameters', { parameters: req.params })
 
       if (areValidationErrors(req, res)) return
@@ -334,7 +329,6 @@ const videosRemoveValidator = [
   param('id').custom(isIdOrUUIDValid).not().isEmpty().withMessage('Should have a valid id'),
 
   async (req: express.Request, res: express.Response, next: express.NextFunction) => {
-    res.docs = "https://docs.joinpeertube.org/api-rest-reference.html#operation/delVideo"
     logger.debug('Checking videosRemove parameters', { parameters: req.params })
 
     if (areValidationErrors(req, res)) return
diff --git a/server/tests/api/check-params/videos.ts b/server/tests/api/check-params/videos.ts
index c970c4a15..a6eecb13a 100644
--- a/server/tests/api/check-params/videos.ts
+++ b/server/tests/api/check-params/videos.ts
@@ -4,6 +4,8 @@ import 'mocha'
 import * as chai from 'chai'
 import { omit } from 'lodash'
 import { join } from 'path'
+import { randomInt } from '@shared/core-utils'
+import { PeerTubeProblemDocument } from '@shared/models'
 import { HttpStatusCode } from '../../../../shared/core-utils/miscs/http-error-codes'
 import {
   checkUploadVideoParam,
@@ -30,7 +32,6 @@ import {
   checkBadStartPagination
 } from '../../../../shared/extra-utils/requests/check-api-params'
 import { VideoPrivacy } from '../../../../shared/models/videos/video-privacy.enum'
-import { randomInt } from '@shared/core-utils'
 
 const expect = chai.expect
 
@@ -411,6 +412,31 @@ describe('Test videos API validator', function () {
         await checkUploadVideoParam(server.url, server.accessToken, { ...fields, ...attaches }, HttpStatusCode.BAD_REQUEST_400, mode)
       })
 
+      it('Should report the appropriate error', async function () {
+        const fields = immutableAssign(baseCorrectParams, { language: 'a'.repeat(15) })
+        const attaches = baseCorrectAttaches
+
+        const attributes = { ...fields, ...attaches }
+        const res = await checkUploadVideoParam(server.url, server.accessToken, attributes, HttpStatusCode.BAD_REQUEST_400, mode)
+
+        const error = res.body as PeerTubeProblemDocument
+
+        if (mode === 'legacy') {
+          expect(error.docs).to.equal('https://docs.joinpeertube.org/api-rest-reference.html#operation/uploadLegacy')
+        } else {
+          expect(error.docs).to.equal('https://docs.joinpeertube.org/api-rest-reference.html#operation/uploadResumableInit')
+        }
+
+        expect(error.type).to.equal('about:blank')
+        expect(error.title).to.equal('Bad Request')
+
+        expect(error.detail).to.equal('Incorrect request parameters: language')
+        expect(error.error).to.equal('Incorrect request parameters: language')
+
+        expect(error.status).to.equal(HttpStatusCode.BAD_REQUEST_400)
+        expect(error['invalid-params'].language).to.exist
+      })
+
       it('Should succeed with the correct parameters', async function () {
         this.timeout(10000)
 
@@ -645,6 +671,24 @@ describe('Test videos API validator', function () {
 
     it('Should fail with a video of another server')
 
+    it('Shoud report the appropriate error', async function () {
+      const fields = immutableAssign(baseCorrectParams, { licence: 125 })
+
+      const res = await makePutBodyRequest({ url: server.url, path: path + videoId, token: server.accessToken, fields })
+      const error = res.body as PeerTubeProblemDocument
+
+      expect(error.docs).to.equal('https://docs.joinpeertube.org/api-rest-reference.html#operation/putVideo')
+
+      expect(error.type).to.equal('about:blank')
+      expect(error.title).to.equal('Bad Request')
+
+      expect(error.detail).to.equal('Incorrect request parameters: licence')
+      expect(error.error).to.equal('Incorrect request parameters: licence')
+
+      expect(error.status).to.equal(HttpStatusCode.BAD_REQUEST_400)
+      expect(error['invalid-params'].licence).to.exist
+    })
+
     it('Should succeed with the correct parameters', async function () {
       const fields = baseCorrectParams
 
@@ -678,6 +722,22 @@ describe('Test videos API validator', function () {
       await getVideo(server.url, '4da6fde3-88f7-4d16-b119-108df5630b06', HttpStatusCode.NOT_FOUND_404)
     })
 
+    it('Shoud report the appropriate error', async function () {
+      const res = await getVideo(server.url, 'hi', HttpStatusCode.BAD_REQUEST_400)
+      const error = res.body as PeerTubeProblemDocument
+
+      expect(error.docs).to.equal('https://docs.joinpeertube.org/api-rest-reference.html#operation/getVideo')
+
+      expect(error.type).to.equal('about:blank')
+      expect(error.title).to.equal('Bad Request')
+
+      expect(error.detail).to.equal('Incorrect request parameters: id')
+      expect(error.error).to.equal('Incorrect request parameters: id')
+
+      expect(error.status).to.equal(HttpStatusCode.BAD_REQUEST_400)
+      expect(error['invalid-params'].id).to.exist
+    })
+
     it('Should succeed with the correct parameters', async function () {
       await getVideo(server.url, videoId)
     })
@@ -755,6 +815,22 @@ describe('Test videos API validator', function () {
 
     it('Should fail with a video of another server')
 
+    it('Shoud report the appropriate error', async function () {
+      const res = await removeVideo(server.url, server.accessToken, 'hello', HttpStatusCode.BAD_REQUEST_400)
+      const error = res.body as PeerTubeProblemDocument
+
+      expect(error.docs).to.equal('https://docs.joinpeertube.org/api-rest-reference.html#operation/delVideo')
+
+      expect(error.type).to.equal('about:blank')
+      expect(error.title).to.equal('Bad Request')
+
+      expect(error.detail).to.equal('Incorrect request parameters: id')
+      expect(error.error).to.equal('Incorrect request parameters: id')
+
+      expect(error.status).to.equal(HttpStatusCode.BAD_REQUEST_400)
+      expect(error['invalid-params'].id).to.exist
+    })
+
     it('Should succeed with the correct parameters', async function () {
       await removeVideo(server.url, server.accessToken, videoId)
     })
diff --git a/server/tests/api/server/follow-constraints.ts b/server/tests/api/server/follow-constraints.ts
index 8a91fbba3..3f2f71f46 100644
--- a/server/tests/api/server/follow-constraints.ts
+++ b/server/tests/api/server/follow-constraints.ts
@@ -18,6 +18,7 @@ import { unfollow } from '../../../../shared/extra-utils/server/follows'
 import { userLogin } from '../../../../shared/extra-utils/users/login'
 import { createUser } from '../../../../shared/extra-utils/users/users'
 import { HttpStatusCode } from '../../../../shared/core-utils/miscs/http-error-codes'
+import { PeerTubeProblemDocument, ServerErrorCode } from '@shared/models'
 
 const expect = chai.expect
 
@@ -153,7 +154,20 @@ describe('Test follow constraints', function () {
       })
 
       it('Should not get the remote video', async function () {
-        await getVideo(servers[0].url, video2UUID, HttpStatusCode.FORBIDDEN_403)
+        const res = await getVideo(servers[0].url, video2UUID, HttpStatusCode.FORBIDDEN_403)
+
+        const error = res.body as PeerTubeProblemDocument
+
+        const doc = 'https://docs.joinpeertube.org/api-rest-reference.html#section/Errors/does_not_respect_follow_constraints'
+        expect(error.type).to.equal(doc)
+        expect(error.code).to.equal(ServerErrorCode.DOES_NOT_RESPECT_FOLLOW_CONSTRAINTS)
+
+        expect(error.detail).to.equal('Cannot get this video regarding follow constraints')
+        expect(error.error).to.equal(error.detail)
+
+        expect(error.status).to.equal(HttpStatusCode.FORBIDDEN_403)
+
+        expect(error.originUrl).to.contains(servers[1].url)
       })
 
       it('Should list local account videos', async function () {
diff --git a/server/typings/express/index.d.ts b/server/typings/express/index.d.ts
index f58436ce1..cbbf40a78 100644
--- a/server/typings/express/index.d.ts
+++ b/server/typings/express/index.d.ts
@@ -1,3 +1,4 @@
+
 import { RegisterServerAuthExternalOptions } from '@server/types'
 import {
   MAbuseMessage,
@@ -20,9 +21,8 @@ import { MVideoImportDefault } from '@server/types/models/video/video-import'
 import { MVideoPlaylistElement, MVideoPlaylistElementVideoUrlPlaylistPrivacy } from '@server/types/models/video/video-playlist-element'
 import { MAccountVideoRateAccountVideo } from '@server/types/models/video/video-rate'
 import { HttpMethod } from '@shared/core-utils/miscs/http-methods'
-import { VideoCreate } from '@shared/models'
+import { PeerTubeProblemDocumentData, ServerErrorCode, VideoCreate } from '@shared/models'
 import { File as UploadXFile, Metadata } from '@uploadx/core'
-import { ProblemDocumentOptions } from 'http-problem-details/dist/ProblemDocument'
 import { RegisteredPlugin } from '../../lib/plugins/plugin-manager'
 import {
   MAccountDefault,
@@ -41,6 +41,7 @@ import {
   MVideoThumbnail,
   MVideoWithRights
 } from '../../types/models'
+
 declare module 'express' {
   export interface Request {
     query: any
@@ -86,14 +87,20 @@ declare module 'express' {
 
   // Extends Response with added functions and potential variables passed by middlewares
   interface Response {
-    docs?: string
     fail: (options: {
-      data?: Record<string, Object>
-      docs?: string
       message: string
-    } & ProblemDocumentOptions) => void
+
+      title?: string
+      status?: number
+      type?: ServerErrorCode
+      instance?: string
+
+      data?: PeerTubeProblemDocumentData
+    }) => void
 
     locals: {
+      docUrl?: string
+
       videoAll?: MVideoFullLight
       onlyImmutableVideo?: MVideoImmutable
       onlyVideo?: MVideoThumbnail