From 10d22b7ac105464f6bde90248dffb94ed3b03f97 Mon Sep 17 00:00:00 2001 From: Szymon Marczak <36894700+szmarczak@users.noreply.github.com> Date: Fri, 3 Aug 2018 06:27:58 +0200 Subject: [PATCH] Increase coverage (#542) --- source/normalize-arguments.js | 6 ++-- source/request-as-event-emitter.js | 18 +++++----- test/error.js | 11 ++++++ test/retry.js | 57 ++++++++++++++++++++++++++++-- test/stream.js | 38 ++++++++++++++++++-- test/timeout.js | 4 +-- test/unix-socket.js | 4 +++ 7 files changed, 120 insertions(+), 18 deletions(-) diff --git a/source/normalize-arguments.js b/source/normalize-arguments.js index 36d4e5080..bf5ba52c3 100644 --- a/source/normalize-arguments.js +++ b/source/normalize-arguments.js @@ -169,10 +169,10 @@ module.exports = (url, options, defaults) => { if (Reflect.has(error, 'headers') && Reflect.has(error.headers, 'retry-after') && retryAfterStatusCodes.has(error.statusCode)) { let after = Number(error.headers['retry-after']); - if (is.number(after)) { - after *= 1000; + if (is.nan(after)) { + after = Date.parse(error.headers['retry-after']) - Date.now(); } else { - after = Math.max(Date.parse(error.headers['retry-after']) - Date.now(), 0); + after *= 1000; } if (after > options.gotRetry.maxRetryAfter) { diff --git a/source/request-as-event-emitter.js b/source/request-as-event-emitter.js index b0c32996d..ace016dd7 100644 --- a/source/request-as-event-emitter.js +++ b/source/request-as-event-emitter.js @@ -15,7 +15,7 @@ const {GotError, CacheError, UnsupportedProtocolError, MaxRedirectsError, Reques const getMethodRedirectCodes = new Set([300, 301, 302, 303, 304, 305, 307, 308]); const allMethodRedirectCodes = new Set([300, 303, 307, 308]); -module.exports = (options = {}) => { +module.exports = options => { const emitter = new EventEmitter(); const requestUrl = options.href || (new URLGlobal(options.path, urlLib.format(options))).toString(); const redirects = []; @@ -138,17 +138,17 @@ module.exports = (options = {}) => { }; emitter.on('retry', (error, cb) => { - const backoff = options.gotRetry.retries(++retryTries, error); + let backoff; + try { + backoff = options.gotRetry.retries(++retryTries, error); + } catch (error) { + emitter.emit('error', error); + return; + } if (backoff) { retryCount++; - setTimeout(options => { - try { - get(options); - } catch (error2) { - emitter.emit('error', error2); - } - }, backoff, options); + setTimeout(get, backoff, options); cb(true); return; } diff --git a/test/error.js b/test/error.js index f9326e9b2..b1b8239a1 100644 --- a/test/error.js +++ b/test/error.js @@ -27,6 +27,11 @@ test.before('setup', async () => { response.end('body'); }); + s.on('/no-status-message', (request, response) => { + response.writeHead(400, ''); + response.end('body'); + }); + s.on('/body', async (request, response) => { const body = await getStream(request); response.end(body); @@ -102,6 +107,12 @@ test('custom status message', async t => { t.is(error.statusMessage, 'Something Exploded'); }); +test('no status message is overriden by the default one', async t => { + const error = await t.throws(got(`${s.url}/no-status-message`)); + t.is(error.statusCode, 400); + t.is(error.statusMessage, http.STATUS_CODES[400]); +}); + test.serial('http.request error', async t => { const stub = sinon.stub(http, 'request').callsFake(() => { throw new TypeError('The header content contains invalid characters'); diff --git a/test/retry.js b/test/retry.js index 72807d2ec..511e04c64 100644 --- a/test/retry.js +++ b/test/retry.js @@ -8,6 +8,7 @@ let trys = 0; let knocks = 0; let fifth = 0; let lastTried413access = Date.now(); +let lastTried413TimestampAccess; const retryAfterOn413 = 2; const socketTimeout = 200; @@ -54,6 +55,16 @@ test.before('setup', async () => { response.end(); }); + s.on('/413withTimestamp', (request, response) => { + const date = (new Date(Date.now() + (retryAfterOn413 * 1000))).toUTCString(); + + response.writeHead(413, { + 'Retry-After': date + }); + response.end(lastTried413TimestampAccess); + lastTried413TimestampAccess = date; + }); + s.on('/413withoutRetryAfter', (request, response) => { response.statusCode = 413; response.end(); @@ -149,6 +160,15 @@ test('respect 413 Retry-After', async t => { t.true(Number(body) >= retryAfterOn413 * 1000); }); +test('respect 413 Retry-After with RFC-1123 timestamp', async t => { + const {statusCode, body} = await got(`${s.url}/413withTimestamp`, { + throwHttpErrors: false, + retry: 1 + }); + t.is(statusCode, 413); + t.true(Date.now() >= Date.parse(body)); +}); + test('doesn\'t retry on 413 with empty statusCodes and methods', async t => { const {statusCode, retryCount} = await got(`${s.url}/413`, { throwHttpErrors: false, @@ -193,8 +213,10 @@ test('retries on 503 without Retry-After header', async t => { test('doesn\'t retry on streams', async t => { const stream = got.stream(s.url, { timeout: 1, - retries: () => { - t.fail('Retries on streams'); + retry: { + retries: () => { + t.fail('Retries on streams'); + } } }); await t.throws(pEvent(stream, 'response')); @@ -207,3 +229,34 @@ test('doesn\'t retry if Retry-After header is greater than maxRetryAfter', async }); t.is(retryCount, 0); }); + +test('doesn\'t retry when set to false', async t => { + const {statusCode, retryCount} = await got(`${s.url}/413`, { + throwHttpErrors: false, + retry: false + }); + t.is(statusCode, 413); + t.is(retryCount, 0); +}); + +test('works when defaults.options.retry is not an object', async t => { + const instance = got.extend({ + retry: 2 + }); + + const {retryCount} = await instance(`${s.url}/413`, { + throwHttpErrors: false + }); + t.is(retryCount, 0); +}); + +test('retry function can throw', async t => { + const error = 'Simple error'; + await t.throws(got(`${s.url}/413`, { + retry: { + retries: () => { + throw new Error(error); + } + } + }), error); +}); diff --git a/test/stream.js b/test/stream.js index 12f734128..6e38f921d 100644 --- a/test/stream.js +++ b/test/stream.js @@ -2,6 +2,7 @@ import test from 'ava'; import toReadableStream from 'to-readable-stream'; import getStream from 'get-stream'; import pEvent from 'p-event'; +import delay from 'delay'; import is from '@sindresorhus/is'; import got from '../source'; import {createServer} from './helpers/server'; @@ -13,9 +14,10 @@ test.before('setup', async () => { s.on('/', (request, response) => { response.writeHead(200, { - unicorn: 'rainbow' + unicorn: 'rainbow', + 'content-encoding': 'gzip' }); - response.end('ok'); + response.end(Buffer.from('H4sIAAAAAAAA/8vPBgBH3dx5AgAAAA==', 'base64')); // 'ok' }); s.on('/post', (request, response) => { @@ -127,6 +129,38 @@ test('proxying headers works', async t => { const {headers} = await got(server.url); t.is(headers.unicorn, 'rainbow'); + t.is(headers['content-encoding'], undefined); await server.close(); }); + +test('skips proxying headers after server has sent them already', async t => { + const server = await createServer(); + + server.on('/', (request, response) => { + response.writeHead(200); + got.stream(s.url).pipe(response); + }); + + await server.listen(server.port); + + const {headers} = await got(server.url); + t.is(headers.unicorn, undefined); + + await server.close(); +}); + +test('throws when trying to proxy through a closed stream', async t => { + const server = await createServer(); + + server.on('/', async (request, response) => { + const stream = got.stream(s.url); + await delay(1000); + t.throws(() => stream.pipe(response)); + response.end(); + }); + + await server.listen(server.port); + await got(server.url); + await server.close(); +}); diff --git a/test/timeout.js b/test/timeout.js index 667556bdc..67ba3ae1d 100644 --- a/test/timeout.js +++ b/test/timeout.js @@ -24,8 +24,8 @@ const slowDataStream = () => { return slowStream; }; -const requestDelay = 250; -const requestTimeout = requestDelay - 10; +const requestDelay = 500; +const requestTimeout = requestDelay - 20; const errorMatcher = { instanceOf: got.TimeoutError, diff --git a/test/unix-socket.js b/test/unix-socket.js index 769b1b1c2..b3bba76cb 100644 --- a/test/unix-socket.js +++ b/test/unix-socket.js @@ -41,4 +41,8 @@ if (process.platform !== 'win32') { const url = format('unix:%s:%s', socketPath, '/foo:bar'); t.is((await got(url)).body, 'ok'); }); + + test('throws on invalid URL', async t => { + await t.throws(got('unix:')); + }); }