Repository: sindresorhus/got
Pull request #411 (2017-11-10 10:03:24)

Do not throw when using a URL object with auth
Before version 7.1.0, the error was thrown only when using a URL [string](https://github.com/sindresorhus/got/pull/329/files#diff-168726dbe96b3ce427e7fedce31bb0bcL267). Now it is thrown even when using a URL object.


Patch:
diff --git a/index.js b/index.js
index 09ea52a..6775dd5 100644
--- a/index.js
+++ b/index.js
@@ -198,7 +198,13 @@ function requestAsEventEmitter(opts) {
 			});
 		});
 
-		cacheReq.on('error', err => ee.emit('error', new got.CacheError(err, opts)));
+		cacheReq.on('error', err => {
+			if (err instanceof CacheableRequest.RequestError) {
+				ee.emit('error', new got.RequestError(err, opts));
+			} else {
+				ee.emit('error', new got.CacheError(err, opts));
+			}
+		});
 
 		cacheReq.on('request', req => {
 			req.once('error', err => {
@@ -435,14 +441,13 @@ function normalizeArguments(url, opts) {
 	} else if (typeof url === 'string') {
 		url = url.replace(/^unix:/, 'http://$&');
 		url = urlParseLax(url);
+		if (url.auth) {
+			throw new Error('Basic authentication must be done with auth option');
+		}
 	} else if (isURL.lenient(url)) {
 		url = urlToOptions(url);
 	}
 
-	if (url.auth) {
-		throw new Error('Basic authentication must be done with auth option');
-	}
-
 	opts = Object.assign(
 		{
 			path: '',
@@ -458,10 +463,17 @@ function normalizeArguments(url, opts) {
 		opts
 	);
 
+	const headers = lowercaseKeys(opts.headers);
+	for (const key of Object.keys(headers)) {
+		if (headers[key] === null || headers[key] === undefined) {
+			delete headers[key];
+		}
+	}
+
 	opts.headers = Object.assign({
 		'user-agent': `${pkg.name}/${pkg.version} (https://github.com/sindresorhus/got)`,
 		'accept-encoding': 'gzip,deflate'
-	}, lowercaseKeys(opts.headers));
+	}, headers);
 
 	const query = opts.query;
 
diff --git a/package.json b/package.json
index 6119377..39aca30 100644
--- a/package.json
+++ b/package.json
@@ -50,7 +50,7 @@
 		"electron"
 	],
 	"dependencies": {
-		"cacheable-request": "^2.0.0",
+		"cacheable-request": "^2.1.0",
 		"decompress-response": "^3.2.0",
 		"duplexer3": "^0.1.4",
 		"get-stream": "^3.0.0",


Test patch:
diff --git a/test/arguments.js b/test/arguments.js
index 101baaa..cd3cb8f 100644
--- a/test/arguments.js
+++ b/test/arguments.js
@@ -53,11 +53,20 @@ test('overrides querystring from opts', async t => {
 	t.is((await got(`${s.url}/?test=doge`, {query: {test: 'wow'}})).body, '/?test=wow');
 });
 
-test('should throw with auth in url', async t => {
+test('should throw with auth in url string', async t => {
 	const err = await t.throws(got('https://test:45d3ps453@account.myservice.com/api/token'));
 	t.regex(err.message, /Basic authentication must be done with auth option/);
 });
 
+test('does not throw with auth in url object', async t => {
+	await t.notThrows(got({
+		auth: 'foo:bar',
+		hostname: s.host,
+		port: s.port,
+		path: '/test'
+	}));
+});
+
 test('should throw when body is set to object', async t => {
 	await t.throws(got(`${s.url}/`, {body: {}}), TypeError);
 });
@@ -67,13 +76,6 @@ test('WHATWG URL support', async t => {
 	await t.notThrows(got(wURL));
 });
 
-test('throws on WHATWG URL with auth', async t => {
-	const wURL = new URL(`${s.url}/test`);
-	wURL.username = 'alex';
-	wURL.password = 'secret';
-	await t.throws(got(wURL));
-});
-
 test.after('cleanup', async () => {
 	await s.close();
 });
diff --git a/test/cancel.js b/test/cancel.js
index caa2d15..8dd2e36 100644
--- a/test/cancel.js
+++ b/test/cancel.js
@@ -1,3 +1,4 @@
+import EventEmitter from 'events';
 import stream from 'stream';
 import test from 'ava';
 import getStream from 'get-stream';
@@ -9,8 +10,10 @@ const Readable = stream.Readable;
 
 async function createAbortServer() {
 	const s = await createServer();
-	const aborted = new Promise((resolve, reject) => {
+	const ee = new EventEmitter();
+	ee.aborted = new Promise((resolve, reject) => {
 		s.on('/abort', (req, res) => {
+			ee.emit('connection');
 			req.on('aborted', resolve);
 			res.on('finish', reject.bind(null, new Error('Request finished instead of aborting.')));
 
@@ -21,11 +24,9 @@ async function createAbortServer() {
 	});
 
 	await s.listen(s.port);
+	ee.url = `${s.url}/abort`;
 
-	return {
-		aborted,
-		url: `${s.url}/abort`
-	};
+	return ee;
 }
 
 test('cancel in-progress request', async t => {
@@ -37,11 +38,11 @@ test('cancel in-progress request', async t => {
 
 	const p = got(helper.url, {body});
 
-	// Wait for the stream to be established before canceling
-	setTimeout(() => {
+	// Wait for the connection to be established before canceling
+	helper.on('connection', () => {
 		p.cancel();
 		body.push(null);
-	}, 100);
+	});
 
 	await t.throws(p, PCancelable.CancelError);
 	await t.notThrows(helper.aborted, 'Request finished instead of aborting.');
@@ -56,11 +57,11 @@ test('cancel in-progress request with timeout', async t => {
 
 	const p = got(helper.url, {body, timeout: 10000});
 
-	// Wait for the stream to be established before canceling
-	setTimeout(() => {
+	// Wait for the connection to be established before canceling
+	helper.on('connection', () => {
 		p.cancel();
 		body.push(null);
-	}, 100);
+	});
 
 	await t.throws(p, PCancelable.CancelError);
 	await t.notThrows(helper.aborted, 'Request finished instead of aborting.');
diff --git a/test/error.js b/test/error.js
index f9a116a..ae05acc 100644
--- a/test/error.js
+++ b/test/error.js
@@ -73,7 +73,8 @@ test.serial('http.request error', async t => {
 		throw new TypeError('The header content contains invalid characters');
 	});
 	const err = await t.throws(got(s.url));
-	t.regex(err.message, /The header content contains invalid characters/);
+	t.true(err instanceof got.RequestError);
+	t.is(err.message, 'The header content contains invalid characters');
 	stub.restore();
 });
 
diff --git a/test/headers.js b/test/headers.js
index 2a0a0ae..07fa774 100644
--- a/test/headers.js
+++ b/test/headers.js
@@ -89,6 +89,24 @@ test('form-data automatic content-type', async t => {
 	t.is(headers['content-type'], `multipart/form-data; boundary=${form.getBoundary()}`);
 });
 
+test('remove null value headers', async t => {
+	const headers = (await got(s.url, {
+		headers: {
+			unicorns: null
+		}
+	})).body;
+	t.false(Object.prototype.hasOwnProperty.call(headers, 'unicorns'));
+});
+
+test('remove undefined value headers', async t => {
+	const headers = (await got(s.url, {
+		headers: {
+			unicorns: undefined
+		}
+	})).body;
+	t.false(Object.prototype.hasOwnProperty.call(headers, 'unicorns'));
+});
+
 test.after('cleanup', async () => {
 	await s.close();
 });