diff options
-rw-r--r-- | server/helpers/requests.ts | 18 | ||||
-rw-r--r-- | server/lib/video-views.ts | 3 | ||||
-rw-r--r-- | server/tests/helpers/request.ts | 15 | ||||
-rw-r--r-- | shared/extra-utils/mock-servers/mock-429.ts | 33 |
4 files changed, 59 insertions, 10 deletions
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 @@ | |||
1 | import { createWriteStream, remove } from 'fs-extra' | 1 | import { createWriteStream, remove } from 'fs-extra' |
2 | import got, { CancelableRequest, Options as GotOptions, RequestError, Response } from 'got' | 2 | import got, { CancelableRequest, NormalizedOptions, Options as GotOptions, RequestError, Response } from 'got' |
3 | import { HttpProxyAgent, HttpsProxyAgent } from 'hpagent' | 3 | import { HttpProxyAgent, HttpsProxyAgent } from 'hpagent' |
4 | import { join } from 'path' | 4 | import { join } from 'path' |
5 | import { CONFIG } from '../initializers/config' | 5 | import { CONFIG } from '../initializers/config' |
@@ -16,6 +16,7 @@ const httpSignature = require('@peertube/http-signature') | |||
16 | export interface PeerTubeRequestError extends Error { | 16 | export interface PeerTubeRequestError extends Error { |
17 | statusCode?: number | 17 | statusCode?: number |
18 | responseBody?: any | 18 | responseBody?: any |
19 | responseHeaders?: any | ||
19 | } | 20 | } |
20 | 21 | ||
21 | type PeerTubeRequestOptions = { | 22 | type PeerTubeRequestOptions = { |
@@ -99,6 +100,12 @@ const peertubeGot = got.extend({ | |||
99 | }, httpSignatureOptions) | 100 | }, httpSignatureOptions) |
100 | } | 101 | } |
101 | } | 102 | } |
103 | ], | ||
104 | |||
105 | beforeRetry: [ | ||
106 | (_options: NormalizedOptions, error: RequestError, retryCount: number) => { | ||
107 | logger.debug('Retrying request to %s.', error.request.requestUrl, { retryCount, error: buildRequestError(error), ...lTags() }) | ||
108 | } | ||
102 | ] | 109 | ] |
103 | } | 110 | } |
104 | }) | 111 | }) |
@@ -107,7 +114,6 @@ function doRequest (url: string, options: PeerTubeRequestOptions = {}) { | |||
107 | const gotOptions = buildGotOptions(options) | 114 | const gotOptions = buildGotOptions(options) |
108 | 115 | ||
109 | return peertubeGot(url, gotOptions) | 116 | return peertubeGot(url, gotOptions) |
110 | .on('retry', logRetryFactory(url)) | ||
111 | .catch(err => { throw buildRequestError(err) }) | 117 | .catch(err => { throw buildRequestError(err) }) |
112 | } | 118 | } |
113 | 119 | ||
@@ -115,7 +121,6 @@ function doJSONRequest <T> (url: string, options: PeerTubeRequestOptions = {}) { | |||
115 | const gotOptions = buildGotOptions(options) | 121 | const gotOptions = buildGotOptions(options) |
116 | 122 | ||
117 | return peertubeGot<T>(url, { ...gotOptions, responseType: 'json' }) | 123 | return peertubeGot<T>(url, { ...gotOptions, responseType: 'json' }) |
118 | .on('retry', logRetryFactory(url)) | ||
119 | .catch(err => { throw buildRequestError(err) }) | 124 | .catch(err => { throw buildRequestError(err) }) |
120 | } | 125 | } |
121 | 126 | ||
@@ -246,14 +251,9 @@ function buildRequestError (error: RequestError) { | |||
246 | 251 | ||
247 | if (error.response) { | 252 | if (error.response) { |
248 | newError.responseBody = error.response.body | 253 | newError.responseBody = error.response.body |
254 | newError.responseHeaders = error.response.headers | ||
249 | newError.statusCode = error.response.statusCode | 255 | newError.statusCode = error.response.statusCode |
250 | } | 256 | } |
251 | 257 | ||
252 | return newError | 258 | return newError |
253 | } | 259 | } |
254 | |||
255 | function logRetryFactory (url: string) { | ||
256 | return (retryCount: number, error: RequestError) => { | ||
257 | logger.debug('Retrying request to %s.', url, { retryCount, error, ...lTags() }) | ||
258 | } | ||
259 | } | ||
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 @@ | |||
1 | import { isTestInstance } from '@server/helpers/core-utils' | ||
1 | import { logger, loggerTagsFactory } from '@server/helpers/logger' | 2 | import { logger, loggerTagsFactory } from '@server/helpers/logger' |
2 | import { VIEW_LIFETIME } from '@server/initializers/constants' | 3 | import { VIEW_LIFETIME } from '@server/initializers/constants' |
3 | import { VideoModel } from '@server/models/video/video' | 4 | import { VideoModel } from '@server/models/video/video' |
@@ -98,7 +99,7 @@ export class VideoViews { | |||
98 | } | 99 | } |
99 | 100 | ||
100 | private async cleanViewers () { | 101 | private async cleanViewers () { |
101 | logger.info('Cleaning video viewers.', lTags()) | 102 | if (!isTestInstance()) logger.info('Cleaning video viewers.', lTags()) |
102 | 103 | ||
103 | for (const videoId of this.viewersPerVideo.keys()) { | 104 | for (const videoId of this.viewersPerVideo.keys()) { |
104 | const notBefore = new Date().getTime() | 105 | 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' | |||
4 | import { expect } from 'chai' | 4 | import { expect } from 'chai' |
5 | import { pathExists, remove } from 'fs-extra' | 5 | import { pathExists, remove } from 'fs-extra' |
6 | import { join } from 'path' | 6 | import { join } from 'path' |
7 | import { Mock429 } from '@shared/extra-utils/mock-servers/mock-429' | ||
7 | import { FIXTURE_URLS, root, wait } from '../../../shared/extra-utils' | 8 | import { FIXTURE_URLS, root, wait } from '../../../shared/extra-utils' |
8 | import { doRequest, doRequestAndSaveToFile } from '../../helpers/requests' | 9 | import { doRequest, doRequestAndSaveToFile } from '../../helpers/requests' |
9 | 10 | ||
@@ -34,6 +35,20 @@ describe('Request helpers', function () { | |||
34 | throw new Error('No error thrown by do request and save to file') | 35 | throw new Error('No error thrown by do request and save to file') |
35 | }) | 36 | }) |
36 | 37 | ||
38 | it('Should correctly retry on 429 error', async function () { | ||
39 | this.timeout(25000) | ||
40 | |||
41 | const mock = new Mock429() | ||
42 | const port = await mock.initialize() | ||
43 | |||
44 | const before = new Date().getTime() | ||
45 | await doRequest('http://localhost:' + port) | ||
46 | |||
47 | expect(new Date().getTime() - before).to.be.greaterThan(2000) | ||
48 | |||
49 | await mock.terminate() | ||
50 | }) | ||
51 | |||
37 | it('Should succeed if the file is below the limit', async function () { | 52 | it('Should succeed if the file is below the limit', async function () { |
38 | await doRequest(FIXTURE_URLS.file4K, { bodyKBLimit: 5 }) | 53 | await doRequest(FIXTURE_URLS.file4K, { bodyKBLimit: 5 }) |
39 | await doRequestAndSaveToFile(FIXTURE_URLS.file4K, destPath2, { bodyKBLimit: 5 }) | 54 | 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 @@ | |||
1 | import express from 'express' | ||
2 | import { Server } from 'http' | ||
3 | import { getPort, randomListen, terminateServer } from './utils' | ||
4 | |||
5 | export class Mock429 { | ||
6 | private server: Server | ||
7 | private responseSent = false | ||
8 | |||
9 | async initialize () { | ||
10 | const app = express() | ||
11 | |||
12 | app.get('/', (req: express.Request, res: express.Response, next: express.NextFunction) => { | ||
13 | |||
14 | if (!this.responseSent) { | ||
15 | this.responseSent = true | ||
16 | |||
17 | // Retry after 5 seconds | ||
18 | res.header('retry-after', '2') | ||
19 | return res.sendStatus(429) | ||
20 | } | ||
21 | |||
22 | return res.sendStatus(200) | ||
23 | }) | ||
24 | |||
25 | this.server = await randomListen(app) | ||
26 | |||
27 | return getPort(this.server) | ||
28 | } | ||
29 | |||
30 | terminate () { | ||
31 | return terminateServer(this.server) | ||
32 | } | ||
33 | } | ||