From 3455c2656e257ae3d9b4169af58b6889d9904148 Mon Sep 17 00:00:00 2001
From: Chocobozzz <me@florianbigard.com>
Date: Tue, 16 Nov 2021 11:17:52 +0100
Subject: [PATCH] Test and log request retries

---
 server/helpers/requests.ts                  | 18 +++++------
 server/lib/video-views.ts                   |  3 +-
 server/tests/helpers/request.ts             | 15 ++++++++++
 shared/extra-utils/mock-servers/mock-429.ts | 33 +++++++++++++++++++++
 4 files changed, 59 insertions(+), 10 deletions(-)
 create mode 100644 shared/extra-utils/mock-servers/mock-429.ts

diff --git a/server/helpers/requests.ts b/server/helpers/requests.ts
index 6e80995ad..fc77ebd35 100644
--- a/server/helpers/requests.ts
+++ b/server/helpers/requests.ts
@@ -1,5 +1,5 @@
 import { createWriteStream, remove } from 'fs-extra'
-import got, { CancelableRequest, Options as GotOptions, RequestError, Response } from 'got'
+import got, { CancelableRequest, NormalizedOptions, Options as GotOptions, RequestError, Response } from 'got'
 import { HttpProxyAgent, HttpsProxyAgent } from 'hpagent'
 import { join } from 'path'
 import { CONFIG } from '../initializers/config'
@@ -16,6 +16,7 @@ const httpSignature = require('@peertube/http-signature')
 export interface PeerTubeRequestError extends Error {
   statusCode?: number
   responseBody?: any
+  responseHeaders?: any
 }
 
 type PeerTubeRequestOptions = {
@@ -99,6 +100,12 @@ const peertubeGot = got.extend({
           }, httpSignatureOptions)
         }
       }
+    ],
+
+    beforeRetry: [
+      (_options: NormalizedOptions, error: RequestError, retryCount: number) => {
+        logger.debug('Retrying request to %s.', error.request.requestUrl, { retryCount, error: buildRequestError(error), ...lTags() })
+      }
     ]
   }
 })
@@ -107,7 +114,6 @@ function doRequest (url: string, options: PeerTubeRequestOptions = {}) {
   const gotOptions = buildGotOptions(options)
 
   return peertubeGot(url, gotOptions)
-    .on('retry', logRetryFactory(url))
     .catch(err => { throw buildRequestError(err) })
 }
 
@@ -115,7 +121,6 @@ function doJSONRequest <T> (url: string, options: PeerTubeRequestOptions = {}) {
   const gotOptions = buildGotOptions(options)
 
   return peertubeGot<T>(url, { ...gotOptions, responseType: 'json' })
-    .on('retry', logRetryFactory(url))
     .catch(err => { throw buildRequestError(err) })
 }
 
@@ -246,14 +251,9 @@ function buildRequestError (error: RequestError) {
 
   if (error.response) {
     newError.responseBody = error.response.body
+    newError.responseHeaders = error.response.headers
     newError.statusCode = error.response.statusCode
   }
 
   return newError
 }
-
-function logRetryFactory (url: string) {
-  return (retryCount: number, error: RequestError) => {
-    logger.debug('Retrying request to %s.', url, { retryCount, error, ...lTags() })
-  }
-}
diff --git a/server/lib/video-views.ts b/server/lib/video-views.ts
index 220b509c2..c024eb93c 100644
--- a/server/lib/video-views.ts
+++ b/server/lib/video-views.ts
@@ -1,3 +1,4 @@
+import { isTestInstance } from '@server/helpers/core-utils'
 import { logger, loggerTagsFactory } from '@server/helpers/logger'
 import { VIEW_LIFETIME } from '@server/initializers/constants'
 import { VideoModel } from '@server/models/video/video'
@@ -98,7 +99,7 @@ export class VideoViews {
   }
 
   private async cleanViewers () {
-    logger.info('Cleaning video viewers.', lTags())
+    if (!isTestInstance()) logger.info('Cleaning video viewers.', lTags())
 
     for (const videoId of this.viewersPerVideo.keys()) {
       const notBefore = new Date().getTime()
diff --git a/server/tests/helpers/request.ts b/server/tests/helpers/request.ts
index c9a2eb831..6edbf2a76 100644
--- a/server/tests/helpers/request.ts
+++ b/server/tests/helpers/request.ts
@@ -4,6 +4,7 @@ import 'mocha'
 import { expect } from 'chai'
 import { pathExists, remove } from 'fs-extra'
 import { join } from 'path'
+import { Mock429 } from '@shared/extra-utils/mock-servers/mock-429'
 import { FIXTURE_URLS, root, wait } from '../../../shared/extra-utils'
 import { doRequest, doRequestAndSaveToFile } from '../../helpers/requests'
 
@@ -34,6 +35,20 @@ describe('Request helpers', function () {
     throw new Error('No error thrown by do request and save to file')
   })
 
+  it('Should correctly retry on 429 error', async function () {
+    this.timeout(25000)
+
+    const mock = new Mock429()
+    const port = await mock.initialize()
+
+    const before = new Date().getTime()
+    await doRequest('http://localhost:' + port)
+
+    expect(new Date().getTime() - before).to.be.greaterThan(2000)
+
+    await mock.terminate()
+  })
+
   it('Should succeed if the file is below the limit', async function () {
     await doRequest(FIXTURE_URLS.file4K, { bodyKBLimit: 5 })
     await doRequestAndSaveToFile(FIXTURE_URLS.file4K, destPath2, { bodyKBLimit: 5 })
diff --git a/shared/extra-utils/mock-servers/mock-429.ts b/shared/extra-utils/mock-servers/mock-429.ts
new file mode 100644
index 000000000..9e0d1281a
--- /dev/null
+++ b/shared/extra-utils/mock-servers/mock-429.ts
@@ -0,0 +1,33 @@
+import express from 'express'
+import { Server } from 'http'
+import { getPort, randomListen, terminateServer } from './utils'
+
+export class Mock429 {
+  private server: Server
+  private responseSent = false
+
+  async initialize () {
+    const app = express()
+
+    app.get('/', (req: express.Request, res: express.Response, next: express.NextFunction) => {
+
+      if (!this.responseSent) {
+        this.responseSent = true
+
+        // Retry after 5 seconds
+        res.header('retry-after', '2')
+        return res.sendStatus(429)
+      }
+
+      return res.sendStatus(200)
+    })
+
+    this.server = await randomListen(app)
+
+    return getPort(this.server)
+  }
+
+  terminate () {
+    return terminateServer(this.server)
+  }
+}