Repository: sindresorhus/got
Pull request #493 (2018-06-22 18:39:38)

Improve the body error message when `{json: false}` and an object is passed
Currently if both the `form` and `json` options are both `false` and you pass a plain Object, the error tells you to pass a `stream.Readable, string, Buffer or plain Object` which I found confusing.

This change makes it so that you only get told to pass an plain Object if the `form` or `json` option is `true`.


Patch:
diff --git a/index.js b/index.js
index 248fdb8..3323eb1 100644
--- a/index.js
+++ b/index.js
@@ -7,6 +7,7 @@ const {PassThrough, Transform} = require('stream');
 const urlLib = require('url');
 const fs = require('fs');
 const querystring = require('querystring');
+const extend = require('extend');
 const CacheableRequest = require('cacheable-request');
 const duplexer3 = require('duplexer3');
 const toReadableStream = require('to-readable-stream');
@@ -28,6 +29,19 @@ const errors = require('./errors');
 const getMethodRedirectCodes = new Set([300, 301, 302, 303, 304, 305, 307, 308]);
 const allMethodRedirectCodes = new Set([300, 303, 307, 308]);
 
+const defaults = {
+	retries: 2,
+	cache: false,
+	decompress: true,
+	useElectronNet: false,
+	throwHttpErrors: true,
+	headers: {
+		'user-agent': `${pkg.name}/${pkg.version} (https://github.com/sindresorhus/got)`
+	}
+};
+
+const got = create(defaults);
+
 const isFormData = body => is.nodeStream(body) && is.function(body.getBoundary);
 
 const getBodySize = async opts => {
@@ -130,6 +144,13 @@ function requestAsEventEmitter(opts) {
 
 				redirectUrl = urlLib.resolve(urlLib.format(opts), bufferString);
 
+				try {
+					decodeURI(redirectUrl);
+				} catch (err) {
+					ee.emit('error', err);
+					return;
+				}
+
 				redirects.push(redirectUrl);
 
 				const redirectOpts = {
@@ -147,8 +168,8 @@ function requestAsEventEmitter(opts) {
 			setImmediate(() => {
 				try {
 					getResponse(res, opts, ee, redirects);
-				} catch (e) {
-					ee.emit('error', e);
+				} catch (err) {
+					ee.emit('error', err);
 				}
 			});
 		});
@@ -177,7 +198,13 @@ function requestAsEventEmitter(opts) {
 				const backoff = opts.retries(++retryCount, err);
 
 				if (backoff) {
-					setTimeout(get, backoff, opts);
+					setTimeout(opts => {
+						try {
+							get(opts);
+						} catch (e) {
+							ee.emit('error', e);
+						}
+					}, backoff, opts);
 					return;
 				}
 
@@ -203,7 +230,7 @@ function requestAsEventEmitter(opts) {
 							}
 
 							const lastUploaded = uploaded;
-							const headersSize = Buffer.byteLength(req._header);
+							const headersSize = req._header ? Buffer.byteLength(req._header) : 0;
 							uploaded = socket.bytesWritten - headersSize;
 
 							// Prevent the known issue of `bytesWritten` being larger than body size
@@ -511,33 +538,25 @@ function normalizeArguments(url, opts) {
 		url = urlToOptions(url);
 	}
 
-	const defaults = {
-		path: '',
-		retries: 2,
-		cache: false,
-		decompress: true,
-		useElectronNet: false,
-		throwHttpErrors: true
-	};
-
 	opts = {
-		...defaults,
+		path: '',
 		...url,
 		protocol: url.protocol || 'http:', // Override both null/undefined with default protocol
 		...opts
 	};
 
 	const headers = lowercaseKeys(opts.headers);
+	if (!Object.keys(headers).includes('user-agent')) {
+		headers['user-agent'] = `${pkg.name}/${pkg.version} (https://github.com/sindresorhus/got)`;
+	}
+
 	for (const [key, value] of Object.entries(headers)) {
 		if (is.nullOrUndefined(value)) {
 			delete headers[key];
 		}
 	}
 
-	opts.headers = {
-		'user-agent': `${pkg.name}/${pkg.version} (https://github.com/sindresorhus/got)`,
-		...headers
-	};
+	opts.headers = headers;
 
 	if (opts.decompress && is.undefined(opts.headers['accept-encoding'])) {
 		opts.headers['accept-encoding'] = 'gzip, deflate';
@@ -564,7 +583,7 @@ function normalizeArguments(url, opts) {
 	} else {
 		const {headers} = opts;
 		if (!is.nodeStream(body) && !is.string(body) && !is.buffer(body) && !(opts.form || opts.json)) {
-			throw new TypeError('The `body` option must be a stream.Readable, string, Buffer or plain Object');
+			throw new TypeError('The `body` option must be a stream.Readable, string or Buffer');
 		}
 
 		const canBodyBeStringified = is.plainObject(body) || is.array(body);
@@ -641,36 +660,46 @@ function normalizeArguments(url, opts) {
 	return opts;
 }
 
-function got(url, opts) {
-	try {
-		const normalizedArgs = normalizeArguments(url, opts);
+function create(defaults = {}) {
+	function got(url, opts) {
+		try {
+			opts = extend(true, {}, defaults, opts);
+			const normalizedArgs = normalizeArguments(url, opts);
 
-		if (normalizedArgs.stream) {
-			return asStream(normalizedArgs);
-		}
+			if (normalizedArgs.stream) {
+				return asStream(normalizedArgs);
+			}
 
-		return asPromise(normalizedArgs);
-	} catch (err) {
-		return Promise.reject(err);
+			return asPromise(normalizedArgs);
+		} catch (err) {
+			return Promise.reject(err);
+		}
 	}
-}
 
-got.stream = (url, opts) => asStream(normalizeArguments(url, opts));
+	got.create = (opts = {}) => create(extend(true, {}, defaults, opts));
 
-const methods = [
-	'get',
-	'post',
-	'put',
-	'patch',
-	'head',
-	'delete'
-];
+	got.stream = (url, opts) => {
+		opts = extend(true, {}, defaults, opts);
+		return asStream(normalizeArguments(url, opts));
+	};
 
-for (const method of methods) {
-	got[method] = (url, options) => got(url, {...options, method});
-	got.stream[method] = (url, options) => got.stream(url, {...options, method});
-}
+	const methods = [
+		'get',
+		'post',
+		'put',
+		'patch',
+		'head',
+		'delete'
+	];
+
+	for (const method of methods) {
+		got[method] = (url, options) => got(url, {...options, method});
+		got.stream[method] = (url, options) => got.stream(url, {...options, method});
+	}
+
+	Object.assign(got, errors);
 
-Object.assign(got, errors);
+	return got;
+}
 
 module.exports = got;
diff --git a/package.json b/package.json
index 88f2aa8..5b2f73a 100644
--- a/package.json
+++ b/package.json
@@ -38,6 +38,7 @@
 		"cacheable-request": "^2.1.1",
 		"decompress-response": "^3.3.0",
 		"duplexer3": "^0.1.4",
+		"extend": "^3.0.1",
 		"get-stream": "^3.0.0",
 		"is-retry-allowed": "^1.1.0",
 		"isurl": "^1.0.0-alpha5",
diff --git a/readme.md b/readme.md
index ce9fe85..c190f86 100644
--- a/readme.md
+++ b/readme.md
@@ -105,6 +105,15 @@ Type: `Object`
 
 Any of the [`https.request`](https://nodejs.org/api/https.html#https_https_request_options_callback) options.
 
+###### headers
+
+Type: `Object`<br>
+Default: `{}`
+
+Request headers.
+
+Existing headers will be overwritten. Headers set to `null` or `undefined` will be omitted.
+
 ###### stream
 
 Type: `boolean`<br>
@@ -291,6 +300,25 @@ If it's not possible to retrieve the body size (can happen when streaming), `tot
 
 Sets `options.method` to the method name and makes a request.
 
+#### Instances
+
+#### got.create([options])
+
+Configure a new `got` instance with default `options`:
+
+```js
+(async () => {
+	const client = got.create({headers: {'x-foo': 'bar'}});
+	const {headers} = await client.get('httpbin.org/headers', {json: true}).body;
+	//=> headers['x-foo'] === 'bar'
+
+	const jsonClient = client.create({json: true, headers: {'x-baz': 'qux'}});
+	const {headers: headers2} = await jsonClient.get('httpbin.org/headers').body;
+	//=> headers2['x-foo'] === 'bar'
+	//=> headers2['x-baz'] === 'qux'
+})();
+```
+
 
 ## Errors
 
@@ -616,7 +644,7 @@ const createTestServer = require('create-test-server');
 
 ### User Agent
 
-It's a good idea to set the `'user-agent'` header so the provider can more easily see how their resource is used. By default, it's the URL to this repo.
+It's a good idea to set the `'user-agent'` header so the provider can more easily see how their resource is used. By default, it's the URL to this repo. You can omit this header by setting it to `null` or `undefined`.
 
 ```js
 const got = require('got');
@@ -627,6 +655,12 @@ got('sindresorhus.com', {
 		'user-agent': `my-module/${pkg.version} (https://github.com/username/my-module)`
 	}
 });
+
+got('sindresorhus.com', {
+	headers: {
+		'user-agent': null
+	}
+});
 ```
 
 ### 304 Responses


Test patch:
diff --git a/test/create.js b/test/create.js
new file mode 100644
index 0000000..5bc98be
--- /dev/null
+++ b/test/create.js
@@ -0,0 +1,67 @@
+import test from 'ava';
+import got from '..';
+import {createServer} from './helpers/server';
+
+let s;
+
+test.before('setup', async () => {
+	s = await createServer();
+
+	s.on('/', (req, res) => {
+		req.resume();
+		res.end(JSON.stringify(req.headers));
+	});
+
+	await s.listen(s.port);
+});
+
+test.after('cleanup', async () => {
+	await s.close();
+});
+
+test('preserve global defaults', async t => {
+	const globalHeaders = (await got(s.url, {json: true})).body;
+	const instanceHeaders = (await got.create()(s.url, {json: true})).body;
+	t.deepEqual(instanceHeaders, globalHeaders);
+});
+
+test('support instance defaults', async t => {
+	const instance = got.create({
+		headers: {
+			'user-agent': 'custom-ua-string'
+		}
+	});
+	const headers = (await instance(s.url, {json: true})).body;
+	t.is(headers['user-agent'], 'custom-ua-string');
+});
+
+test('support invocation overrides', async t => {
+	const instance = got.create({
+		headers: {
+			'user-agent': 'custom-ua-string'
+		}
+	});
+	const headers = (await instance(s.url, {
+		json: true,
+		headers: {
+			'user-agent': 'different-ua-string'
+		}
+	})).body;
+	t.is(headers['user-agent'], 'different-ua-string');
+});
+
+test('curry previous instance defaults', async t => {
+	const instanceA = got.create({
+		headers: {
+			'x-foo': 'foo'
+		}
+	});
+	const instanceB = instanceA.create({
+		headers: {
+			'x-bar': 'bar'
+		}
+	});
+	const headers = (await instanceB(s.url, {json: true})).body;
+	t.is(headers['x-foo'], 'foo');
+	t.is(headers['x-bar'], 'bar');
+});
diff --git a/test/error.js b/test/error.js
index ba63c45..d760723 100644
--- a/test/error.js
+++ b/test/error.js
@@ -53,8 +53,18 @@ test('dns message', async t => {
 });
 
 test('options.body error message', async t => {
-	const err = await t.throws(got(s.url, {body: () => { }}));
-	t.regex(err.message, /The `body` option must be a stream\.Readable, string, Buffer or plain Object/);
+	const err = await t.throws(got(s.url, {body: {}}));
+	t.regex(err.message, /The `body` option must be a stream\.Readable, string or Buffer/);
+});
+
+test('options.body json error message', async t => {
+	const err = await t.throws(got(s.url, {body: Buffer.from('test'), json: true}));
+	t.regex(err.message, /The `body` option must be a plain Object or Array when the `form` or `json` option is used/);
+});
+
+test('options.body form error message', async t => {
+	const err = await t.throws(got(s.url, {body: Buffer.from('test'), form: true}));
+	t.regex(err.message, /The `body` option must be a plain Object or Array when the `form` or `json` option is used/);
 });
 
 test('default status message', async t => {
diff --git a/test/headers.js b/test/headers.js
index 82d528e..f7d6537 100644
--- a/test/headers.js
+++ b/test/headers.js
@@ -128,21 +128,21 @@ test('stream as options.body sets content-length', async t => {
 test('remove null value headers', async t => {
 	const {body} = await got(s.url, {
 		headers: {
-			unicorns: null
+			'user-agent': null
 		}
 	});
 	const headers = JSON.parse(body);
-	t.false(Reflect.has(headers, 'unicorns'));
+	t.false(Reflect.has(headers, 'user-agent'));
 });
 
 test('remove undefined value headers', async t => {
 	const {body} = await got(s.url, {
 		headers: {
-			unicorns: undefined
+			'user-agent': undefined
 		}
 	});
 	const headers = JSON.parse(body);
-	t.false(Reflect.has(headers, 'unicorns'));
+	t.false(Reflect.has(headers, 'user-agent'));
 });
 
 test.after('cleanup', async () => {
diff --git a/test/redirects.js b/test/redirects.js
index b1ee333..d7a60b9 100644
--- a/test/redirects.js
+++ b/test/redirects.js
@@ -123,6 +123,13 @@ test.before('setup', async () => {
 		res.end();
 	});
 
+	http.on('/malformedRedirect', (req, res) => {
+		res.writeHead(302, {
+			location: '/%D8'
+		});
+		res.end();
+	});
+
 	await http.listen(http.port);
 	await https.listen(https.port);
 });
@@ -210,3 +217,8 @@ test('redirect response contains old url', async t => {
 test('redirect response contains utf8 with binary encoding', async t => {
 	t.is((await got(`${http.url}/redirect-with-utf8-binary`)).body, 'reached');
 });
+
+test('throws on malformed redirect URI', async t => {
+	const err = await t.throws(got(`${http.url}/malformedRedirect`));
+	t.is(err.name, 'URIError');
+});