Repository: sindresorhus/got
Pull request #542 (2018-08-02 15:41:12)

Increase coverage

szmarczak: I'll leave this part:

https://github.com/sindresorhus/got/blob/6ba9e68c7f496a69641a8aac51be0781f1ae0eac/source/timed-out.js#L30-L32

this makes us sure `timed-out` isn't called twice or more, although it's obvious we don't do it.
szmarczak: We also need tests for `electron` support.
szmarczak: I just wonder if this piece of code is needed:

https://github.com/sindresorhus/got/blob/6ba9e68c7f496a69641a8aac51be0781f1ae0eac/source/get-body-size.js#L31-L33

because I tried passing a `Buffer` as body and this doesn't even get called.
sindresorhus: > because I tried passing a Buffer as body and this doesn't even get called.

You have to pass a stream. The buffer testing is testing a property on the stream.
szmarczak: @sindresorhus But it's converted to a stream here:

https://github.com/sindresorhus/got/blob/6ba9e68c7f496a69641a8aac51be0781f1ae0eac/source/normalize-arguments.js#L111-L114

Patch:
diff --git a/readme.md b/readme.md
index fa38aef..1a7df57 100644
--- a/readme.md
+++ b/readme.md
@@ -137,7 +137,7 @@ Returns a `Stream` instead of a `Promise`. This is equivalent to calling `got.st
 
 Type: `string` `Buffer` `stream.Readable` [`form-data` instance](https://github.com/form-data/form-data)
 
-*This is mutually exclusive with stream mode.*
+*If you provide this option, `got.stream()` will be read-only.*
 
 Body that will be sent with a `POST` request.
 
@@ -157,7 +157,7 @@ Default: `'utf8'`
 Type: `boolean`<br>
 Default: `false`
 
-*This is mutually exclusive with stream mode.*
+*If you provide this option, `got.stream()` will be read-only.*
 
 If set to `true` and `Content-Type` header is not set, it will be set to `application/x-www-form-urlencoded`.
 
@@ -168,7 +168,7 @@ If set to `true` and `Content-Type` header is not set, it will be set to `applic
 Type: `boolean`<br>
 Default: `false`
 
-*This is mutually exclusive with stream mode.*
+*If you use `got.stream()`, this option will be ignored.*
 
 If set to `true` and `Content-Type` header is not set, it will be set to `application/json`.
 
diff --git a/source/as-stream.js b/source/as-stream.js
index e878b41..38143bb 100644
--- a/source/as-stream.js
+++ b/source/as-stream.js
@@ -14,10 +14,6 @@ module.exports = options => {
 
 	options.gotRetry.retries = () => 0;
 
-	if (options.json) {
-		throw new Error('Got can not be used as a stream when the `json` option is used');
-	}
-
 	if (options.body) {
 		proxy.write = () => {
 			throw new Error('Got\'s stream is not writable when the `body` option is used');
@@ -76,7 +72,8 @@ module.exports = options => {
 			for (const [key, value] of Object.entries(response.headers)) {
 				// Got gives *uncompressed* data. Overriding `content-encoding` header would result in an error.
 				// It's not possible to decompress uncompressed data, is it?
-				if (key.toLowerCase() !== 'content-encoding') {
+				const allowed = options.decompress ? key !== 'content-encoding' : true;
+				if (allowed) {
 					destination.setHeader(key, value);
 				}
 			}
diff --git a/source/normalize-arguments.js b/source/normalize-arguments.js
index d224366..bf5ba52 100644
--- a/source/normalize-arguments.js
+++ b/source/normalize-arguments.js
@@ -49,6 +49,10 @@ module.exports = (url, options, defaults) => {
 		...options
 	};
 
+	if (options.stream && options.json) {
+		options.json = false;
+	}
+
 	if (options.decompress && is.undefined(options.headers['accept-encoding'])) {
 		options.headers['accept-encoding'] = 'gzip, deflate';
 	}
@@ -165,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 b0c3299..ace016d 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;
 		}


Test patch:
diff --git a/test/arguments.js b/test/arguments.js
index 21a4b93..77d2623 100644
--- a/test/arguments.js
+++ b/test/arguments.js
@@ -109,9 +109,9 @@ test('should return streams when using stream option', async t => {
 	t.is(data.toString(), 'ok');
 });
 
-test('should not allow stream and JSON option at the same time', async t => {
-	const error = await t.throws(got(`${s.url}/stream`, {stream: true, json: true}));
-	t.is(error.message, 'Got can not be used as a stream when the `json` option is used');
+test('should ignore JSON option when using stream option', async t => {
+	const data = await pEvent(got(`${s.url}/stream`, {stream: true, json: true}), 'data');
+	t.is(data.toString(), 'ok');
 });
 
 test('throws TypeError when `url` is passed as an option', async t => {
diff --git a/test/error.js b/test/error.js
index f9326e9..b1b8239 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 72807d2..511e04c 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 5e1defa..6e38f92 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) => {
@@ -41,10 +43,8 @@ test.after('cleanup', async () => {
 	await s.close();
 });
 
-test('option.json can not be used', t => {
-	t.throws(() => {
-		got.stream(s.url, {json: true});
-	}, 'Got can not be used as a stream when the `json` option is used');
+test('options.json is ignored', t => {
+	t.notThrows(() => got.stream(s.url, {json: true}));
 });
 
 test('returns readable stream', async t => {
@@ -129,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 667556b..67ba3ae 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 769b1b1..b3bba76 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:'));
+	});
 }