From 3866ea02d4a5c8e4c69a5d8633a883e3733414b9 Mon Sep 17 00:00:00 2001 From: Rigel Kent Date: Tue, 1 Jun 2021 16:07:58 +0200 Subject: [PATCH] correct error codes and backward compat --- server/controllers/api/videos/import.ts | 2 +- server/helpers/express-utils.ts | 18 +++++++------- .../validators/activitypub/activity.ts | 6 ++--- .../validators/videos/video-live.ts | 4 ++-- .../middlewares/validators/videos/videos.ts | 2 +- server/tests/api/users/users.ts | 24 ++++++++++++------- .../models/server/server-error-code.enum.ts | 8 +++---- 7 files changed, 35 insertions(+), 29 deletions(-) diff --git a/server/controllers/api/videos/import.ts b/server/controllers/api/videos/import.ts index 6ee109a8f..42ca59975 100644 --- a/server/controllers/api/videos/import.ts +++ b/server/controllers/api/videos/import.ts @@ -335,7 +335,7 @@ async function processTorrentOrAbortRequest (req: express.Request, res: express. cleanUpReqFiles(req) res.fail({ - type: ServerErrorCode.INCORRECT_FILES_IN_TORRENT.toString(), + type: ServerErrorCode.INCORRECT_FILES_IN_TORRENT, message: 'Torrents with only 1 file are supported.' }) return undefined diff --git a/server/helpers/express-utils.ts b/server/helpers/express-utils.ts index bca59a83c..10a860787 100644 --- a/server/helpers/express-utils.ts +++ b/server/helpers/express-utils.ts @@ -129,15 +129,14 @@ function getCountVideos (req: express.Request) { // 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, message, title, type, docs, instance } = { - data: null, - ...options, - status: options.status || HttpStatusCode.BAD_REQUEST_400 - } + const { data, status = HttpStatusCode.BAD_REQUEST_400, message, title, type, docs = res.docs, instance } = options const extension = new ProblemDocumentExtension({ ...data, - docs: docs || res.docs + docs, + // fields for <= 3.2 compatibility, deprecated + error: message, + code: type }) res.status(status) @@ -146,12 +145,13 @@ const apiResponseHelpers = (req, res: express.Response, next = null) => { status, title, instance, - type: type && '' + type, - detail: message + // 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 !== null) next() + if (next) next() } // --------------------------------------------------------------------------- diff --git a/server/middlewares/validators/activitypub/activity.ts b/server/middlewares/validators/activitypub/activity.ts index 59355e855..cc6acd4b1 100644 --- a/server/middlewares/validators/activitypub/activity.ts +++ b/server/middlewares/validators/activitypub/activity.ts @@ -9,16 +9,14 @@ async function activityPubValidator (req: express.Request, res: express.Response if (!isRootActivityValid(req.body)) { logger.warn('Incorrect activity parameters.', { activity: req.body }) - return res.status(HttpStatusCode.BAD_REQUEST_400) - .end() + return res.fail({ message: 'Incorrect activity' }) } const serverActor = await getServerActor() const remoteActor = res.locals.signature.actor if (serverActor.id === remoteActor.id || remoteActor.serverId === null) { logger.error('Receiving request in INBOX by ourselves!', req.body) - return res.status(HttpStatusCode.CONFLICT_409) - .end() + return res.status(HttpStatusCode.CONFLICT_409).end() } return next() diff --git a/server/middlewares/validators/videos/video-live.ts b/server/middlewares/validators/videos/video-live.ts index 9544fa4f5..0fb864098 100644 --- a/server/middlewares/validators/videos/video-live.ts +++ b/server/middlewares/validators/videos/video-live.ts @@ -104,7 +104,7 @@ const videoLiveAddValidator = getCommonVideoEditAttributes().concat([ return res.fail({ status: HttpStatusCode.FORBIDDEN_403, message: 'Cannot create this live because the max instance lives limit is reached.', - type: ServerErrorCode.MAX_INSTANCE_LIVES_LIMIT_REACHED.toString() + type: ServerErrorCode.MAX_INSTANCE_LIVES_LIMIT_REACHED }) } } @@ -117,7 +117,7 @@ const videoLiveAddValidator = getCommonVideoEditAttributes().concat([ return res.fail({ status: HttpStatusCode.FORBIDDEN_403, - type: ServerErrorCode.MAX_USER_LIVES_LIMIT_REACHED.toString(), + type: ServerErrorCode.MAX_USER_LIVES_LIMIT_REACHED, message: 'Cannot create this live because the max user lives limit is reached.' }) } diff --git a/server/middlewares/validators/videos/videos.ts b/server/middlewares/validators/videos/videos.ts index dfd472400..fd0e543f1 100644 --- a/server/middlewares/validators/videos/videos.ts +++ b/server/middlewares/validators/videos/videos.ts @@ -253,7 +253,7 @@ async function checkVideoFollowConstraints (req: express.Request, res: express.R return res.fail({ status: HttpStatusCode.FORBIDDEN_403, message: 'Cannot get this video regarding follow constraints.', - type: ServerErrorCode.DOES_NOT_RESPECT_FOLLOW_CONSTRAINTS.toString(), + type: ServerErrorCode.DOES_NOT_RESPECT_FOLLOW_CONSTRAINTS, data: { originUrl: video.url } diff --git a/server/tests/api/users/users.ts b/server/tests/api/users/users.ts index 363236b62..464c11d34 100644 --- a/server/tests/api/users/users.ts +++ b/server/tests/api/users/users.ts @@ -93,16 +93,20 @@ describe('Test users', function () { const client = { id: 'client', secret: server.client.secret } const res = await login(server.url, client, server.user, HttpStatusCode.BAD_REQUEST_400) - expect(res.body.type).to.equal('invalid_client') - expect(res.body.detail).to.contain('client is invalid') + expect(res.body.code).to.equal('invalid_client') + expect(res.body.error).to.contain('client is invalid') + expect(res.body.type.startsWith('https://')).to.be.true + expect(res.body.type).to.contain('invalid_client') }) it('Should not login with an invalid client secret', async function () { const client = { id: server.client.id, secret: 'coucou' } const res = await login(server.url, client, server.user, HttpStatusCode.BAD_REQUEST_400) - expect(res.body.type).to.equal('invalid_client') - expect(res.body.detail).to.contain('client is invalid') + expect(res.body.code).to.equal('invalid_client') + expect(res.body.error).to.contain('client is invalid') + expect(res.body.type.startsWith('https://')).to.be.true + expect(res.body.type).to.contain('invalid_client') }) }) @@ -112,16 +116,20 @@ describe('Test users', function () { const user = { username: 'captain crochet', password: server.user.password } const res = await login(server.url, server.client, user, HttpStatusCode.BAD_REQUEST_400) - expect(res.body.type).to.equal('invalid_grant') - expect(res.body.detail).to.contain('credentials are invalid') + expect(res.body.code).to.equal('invalid_grant') + expect(res.body.error).to.contain('credentials are invalid') + expect(res.body.type.startsWith('https://')).to.be.true + expect(res.body.type).to.contain('invalid_grant') }) it('Should not login with an invalid password', async function () { const user = { username: server.user.username, password: 'mew_three' } const res = await login(server.url, server.client, user, HttpStatusCode.BAD_REQUEST_400) - expect(res.body.type).to.equal('invalid_grant') - expect(res.body.detail).to.contain('credentials are invalid') + expect(res.body.code).to.equal('invalid_grant') + expect(res.body.error).to.contain('credentials are invalid') + expect(res.body.type.startsWith('https://')).to.be.true + expect(res.body.type).to.contain('invalid_grant') }) it('Should not be able to upload a video', async function () { diff --git a/shared/models/server/server-error-code.enum.ts b/shared/models/server/server-error-code.enum.ts index d17d958be..f9fe47830 100644 --- a/shared/models/server/server-error-code.enum.ts +++ b/shared/models/server/server-error-code.enum.ts @@ -1,6 +1,6 @@ export const enum ServerErrorCode { - DOES_NOT_RESPECT_FOLLOW_CONSTRAINTS = 1, - MAX_INSTANCE_LIVES_LIMIT_REACHED = 2, - MAX_USER_LIVES_LIMIT_REACHED = 3, - INCORRECT_FILES_IN_TORRENT = 4 + DOES_NOT_RESPECT_FOLLOW_CONSTRAINTS = 'DOES_NOT_RESPECT_FOLLOW_CONSTRAINTS', + MAX_INSTANCE_LIVES_LIMIT_REACHED = 'MAX_INSTANCE_LIVES_LIMIT_REACHED', + MAX_USER_LIVES_LIMIT_REACHED = 'MAX_USER_LIVES_LIMIT_REACHED', + INCORRECT_FILES_IN_TORRENT = 'INCORRECT_FILES_IN_TORRENT' }