Repository: sindresorhus/got
Pull request #940 (2019-11-23 13:11:14)

Use `responseType` instead of `options.encoding` to get a Buffer
https://github.com/sindresorhus/got/pull/921#discussion_r347079358

#### Checklist

- [x] I have read the documentation.
- [x] I have included a pull request description of my changes.
- [x] I have included some tests.
- [x] If it's a new feature, I have included documentation updates.



Patch:
diff --git a/readme.md b/readme.md
index b9c8c57..bd3028c 100644
--- a/readme.md
+++ b/readme.md
@@ -248,17 +248,21 @@ Default: `'default'`
 
 Parsing method used to retrieve the body from the response.
 
-- `'default'` - if `options.encoding` is `null`, the body will be a Buffer. Otherwise it will be a string unless it's overwritten in a `afterResponse` hook,
-- `'text'` - will always give a string, no matter what's the `options.encoding` or if the body is a custom object,
-- `'json'` - will always give an object, unless it's invalid JSON - then it will throw.
-- `'buffer'` - will always give a Buffer, no matter what's the `options.encoding`. It will throw if the body is a custom object.
+- `'default'` - Will give a string unless the body is overwritten in a `afterResponse` hook or if `options.decompress` is set to false - Will give a Buffer if the response is compresssed.
+- `'text'` - Will give a string no matter what.
+- `'json'` - Will give an object, unless the body is invalid JSON, then it will throw.
+- `'buffer'` - Will give a Buffer, ignoring `options.encoding`. It will throw if the body is a custom object.
 
-The promise has `.json()` and `.buffer()` and `.text()` functions which set this option automatically.
+The promise has `.json()` and `.buffer()` and `.text()` methods which set this option automatically.
 
 Example:
 
 ```js
+// This
 const body = await got(url).json();
+
+// is the same as this
+const body = await got(url, {responseType: 'json'});
 ```
 
 ###### resolveBodyOnly
@@ -297,10 +301,12 @@ Ignore invalid cookies instead of throwing an error. Only useful when the `cooki
 
 ###### encoding
 
-Type: `string | null`<br>
+Type: `string`<br>
 Default: `'utf8'`
 
-[Encoding](https://nodejs.org/api/buffer.html#buffer_buffers_and_character_encodings) to be used on `setEncoding` of the response data. If `null`, the body is returned as a [`Buffer`](https://nodejs.org/api/buffer.html) (binary data).
+[Encoding](https://nodejs.org/api/buffer.html#buffer_buffers_and_character_encodings) to be used on `setEncoding` of the response data.
+
+To get a [`Buffer`](https://nodejs.org/api/buffer.html), you need to set [`responseType`](#responseType) to `buffer` instead.
 
 ###### form
 
@@ -754,7 +760,7 @@ Progress events for uploading (sending a request) and downloading (receiving a r
 }
 ```
 
-If it's not possible to retrieve the body size (can happen when streaming), `total` will be `null`.
+If it's not possible to retrieve the body size (can happen when streaming), `total` will be `undefined`.
 
 ```js
 (async () => {
diff --git a/source/as-promise.ts b/source/as-promise.ts
index df4d9a1..97fa9b5 100644
--- a/source/as-promise.ts
+++ b/source/as-promise.ts
@@ -53,10 +53,8 @@ export default function asPromise(options: NormalizedOptions) {
 		emitter.on('response', async (response: Response) => {
 			proxy.emit('response', response);
 
-			const streamAsPromise = is.null_(options.encoding) ? getStream.buffer(response) : getStream(response, {encoding: options.encoding});
-
 			try {
-				response.body = await streamAsPromise;
+				response.body = await getStream(response, {encoding: options.encoding});
 			} catch (error) {
 				emitError(new ReadError(error, options));
 				return;
diff --git a/source/errors.ts b/source/errors.ts
index b880772..f624547 100644
--- a/source/errors.ts
+++ b/source/errors.ts
@@ -7,7 +7,7 @@ export class GotError extends Error {
 	code?: string;
 	readonly options: NormalizedOptions;
 
-	constructor(message: string, error: (Error & {code?: string}) | {code?: string}, options: NormalizedOptions) {
+	constructor(message: string, error: Partial<Error & {code?: string}>, options: NormalizedOptions) {
 		super(message);
 		Error.captureStackTrace(this, this.constructor);
 		this.name = 'GotError';
@@ -19,6 +19,20 @@ export class GotError extends Error {
 		Object.defineProperty(this, 'options', {
 			value: options
 		});
+
+		// Recover the original stacktrace
+		if (!is.undefined(error.stack)) {
+			const indexOfMessage = this.stack.indexOf(this.message) + this.message.length;
+			const thisStackTrace = this.stack.slice(indexOfMessage).split('\n').reverse();
+			const errorStackTrace = error.stack.slice(error.stack.indexOf(error.message) + error.message.length).split('\n').reverse();
+
+			// Remove duplicated traces
+			while (errorStackTrace.length !== 0 && errorStackTrace[0] === thisStackTrace[0]) {
+				thisStackTrace.shift();
+			}
+
+			this.stack = `${this.stack.slice(0, indexOfMessage)}${thisStackTrace.reverse().join('\n')}${errorStackTrace.reverse().join('\n')}`;
+		}
 	}
 }
 
@@ -96,7 +110,7 @@ export class TimeoutError extends GotError {
 	event: string;
 
 	constructor(error: TimedOutError, timings: Timings, options: NormalizedOptions) {
-		super(error.message, {code: 'ETIMEDOUT'}, options);
+		super(error.message, error, options);
 		this.name = 'TimeoutError';
 		this.event = error.event;
 		this.timings = timings;
diff --git a/source/get-response.ts b/source/get-response.ts
index ee45615..d17fdea 100644
--- a/source/get-response.ts
+++ b/source/get-response.ts
@@ -21,7 +21,10 @@ export default async (response: IncomingMessage, options: NormalizedOptions, emi
 	) as Response;
 
 	if (!options.decompress && ['gzip', 'deflate', 'br'].includes(response.headers['content-encoding'] ?? '')) {
-		options.encoding = null;
+		options.responseType = 'default';
+
+		// @ts-ignore Internal use.
+		options.encoding = 'buffer';
 	}
 
 	emitter.emit('response', newResponse);
diff --git a/source/normalize-arguments.ts b/source/normalize-arguments.ts
index 4b99a94..74ad8ac 100644
--- a/source/normalize-arguments.ts
+++ b/source/normalize-arguments.ts
@@ -164,6 +164,11 @@ export const preNormalizeArguments = (options: Options, defaults?: NormalizedOpt
 		options.cookieJar = {setCookie, getCookieString};
 	}
 
+	// `options.encoding`
+	if (is.null_(options.encoding)) {
+		throw new TypeError('To get a Buffer, set `options.responseType` to `buffer` instead');
+	}
+
 	return options as NormalizedOptions;
 };
 
diff --git a/source/request-as-event-emitter.ts b/source/request-as-event-emitter.ts
index 04812f2..87be014 100644
--- a/source/request-as-event-emitter.ts
+++ b/source/request-as-event-emitter.ts
@@ -14,6 +14,7 @@ import {CacheError, MaxRedirectsError, RequestError, TimeoutError} from './error
 import urlToOptions from './utils/url-to-options';
 import {NormalizedOptions, Response, ResponseObject} from './utils/types';
 
+const setImmediateAsync = () => new Promise(resolve => setImmediate(resolve));
 const pipeline = promisify(stream.pipeline);
 
 const redirectCodes: ReadonlySet<number> = new Set([300, 301, 302, 303, 304, 307, 308]);
@@ -142,7 +143,6 @@ export default (options: NormalizedOptions) => {
 
 		const handleRequest = async (request: http.ClientRequest): Promise<void> => {
 			// `request.aborted` is a boolean since v11.0.0: https://github.com/nodejs/node/commit/4b00c4fafaa2ae8c41c1f78823c0feb810ae4723#diff-e3bc37430eb078ccbafe3aa3b570c91a
-			// We need to allow `TimedOutTimeoutError` here, because it `stream.pipeline(…)` aborts it automatically.
 			const isAborted = () => typeof request.aborted === 'number' || (request.aborted as unknown as boolean) === true;
 
 			currentRequest = request;
@@ -161,6 +161,7 @@ export default (options: NormalizedOptions) => {
 
 			const attachErrorHandler = () => {
 				request.once('error', error => {
+					// We need to allow `TimedOutTimeoutError` here, because `stream.pipeline(…)` aborts the request automatically.
 					if (isAborted() && !(error instanceof TimedOutTimeoutError)) {
 						return;
 					}
@@ -283,7 +284,13 @@ export default (options: NormalizedOptions) => {
 		}
 	};
 
-	setImmediate(async () => {
+	(async () => {
+		// Promises are executed immediately.
+		// If there were no `setImmediate` here,
+		// `promise.json()` would have no effect
+		// as the request would be sent already.
+		await setImmediateAsync();
+
 		try {
 			for (const hook of options.hooks.beforeRequest) {
 				// eslint-disable-next-line no-await-in-loop
@@ -294,7 +301,7 @@ export default (options: NormalizedOptions) => {
 		} catch (error) {
 			emitError(error);
 		}
-	});
+	})();
 
 	return emitter;
 };
diff --git a/source/utils/types.ts b/source/utils/types.ts
index 571ab45..b279ffe 100644
--- a/source/utils/types.ts
+++ b/source/utils/types.ts
@@ -131,7 +131,7 @@ export interface Options extends URLOptions {
 	hooks?: Partial<Hooks>;
 	decompress?: boolean;
 	isStream?: boolean;
-	encoding?: BufferEncoding | null;
+	encoding?: BufferEncoding;
 	method?: Method;
 	retry?: number | RetryOptions;
 	throwHttpErrors?: boolean;


Test patch:
diff --git a/test/arguments.ts b/test/arguments.ts
index df3ea31..ac36b55 100644
--- a/test/arguments.ts
+++ b/test/arguments.ts
@@ -336,3 +336,9 @@ test('`context` option is accessible when extending instances', t => {
 	t.is(instance.defaults.options.context, context);
 	t.false({}.propertyIsEnumerable.call(instance.defaults.options, 'context'));
 });
+
+test('throws if `options.encoding` is `null`', async t => {
+	await t.throwsAsync(got('https://example.com', {
+		encoding: null
+	}), 'To get a Buffer, set `options.responseType` to `buffer` instead');
+});
diff --git a/test/error.ts b/test/error.ts
index 369a75a..ece3a38 100644
--- a/test/error.ts
+++ b/test/error.ts
@@ -197,3 +197,17 @@ test('errors are thrown directly when options.stream is true', t => {
 		message: 'Parameter `hooks` must be an Object, not boolean'
 	});
 });
+
+test('the old stacktrace is recovered', async t => {
+	const error = await t.throwsAsync(got('https://example.com', {
+		request: () => {
+			throw new Error('foobar');
+		}
+	}));
+
+	t.true(error.stack.includes('at Object.request'));
+
+	// The first `at get` points to where the error was wrapped,
+	// the second `at get` points to the real cause.
+	t.not(error.stack.indexOf('at get'), error.stack.lastIndexOf('at get'));
+});
diff --git a/test/http.ts b/test/http.ts
index f13cd6e..076ad89 100644
--- a/test/http.ts
+++ b/test/http.ts
@@ -1,4 +1,3 @@
-import is from '@sindresorhus/is';
 import test from 'ava';
 import got from '../source';
 import withServer from './helpers/with-server';
@@ -72,13 +71,15 @@ test('invalid protocol throws', async t => {
 	});
 });
 
-test('gives buffer if `options.encoding` is null', withServer, async (t, server, got) => {
+test('custom `options.encoding`', withServer, async (t, server, got) => {
+	const string = 'ok';
+
 	server.get('/', (_request, response) => {
-		response.end('ok');
+		response.end(string);
 	});
 
-	const data = (await got({encoding: null})).body;
-	t.true(is.buffer(data));
+	const data = (await got({encoding: 'base64'})).body;
+	t.is(data, Buffer.from(string).toString('base64'));
 });
 
 test('`searchParams` option', withServer, async (t, server, got) => {
diff --git a/test/progress.ts b/test/progress.ts
index c08b7e7..2b9ed68 100644
--- a/test/progress.ts
+++ b/test/progress.ts
@@ -71,7 +71,7 @@ test('download progress', withServer, async (t, server, got) => {
 
 	const events = [];
 
-	const {body} = await got({encoding: null})
+	const {body} = await got({responseType: 'buffer'})
 		.on('downloadProgress', event => events.push(event));
 
 	checkEvents(t, events, body.length);
@@ -92,7 +92,7 @@ test('download progress - stream', withServer, async (t, server, got) => {
 
 	const events = [];
 
-	const stream = got.stream({encoding: null})
+	const stream = got.stream({responseType: 'buffer'})
 		.on('downloadProgress', event => events.push(event));
 
 	await getStream(stream);