fix sindresorhus#929

szmarczak · Mar 4, 2020 · 4fbf4b9 · 4fbf4b9
1 parent dd972be
commit 4fbf4b9
Show file tree

Hide file tree

Showing 5 changed files with 115 additions and 44 deletions.
diff --git a/source/as-promise/create-rejection.ts b/source/as-promise/create-rejection.ts
@@ -1,7 +1,25 @@
-import {CancelableRequest} from './types';
+import {CancelableRequest, BeforeErrorHook, RequestError} from './types';
+
+export default function createRejection(error: Error, ...beforeErrorGroups: Array<BeforeErrorHook[] | undefined>): CancelableRequest<never> {
+	const promise = (async () => {
+		if (error instanceof RequestError) {
+			try {
+				for (const hooks of beforeErrorGroups) {
+					if (hooks) {
+						for (const hook of hooks) {
+							// eslint-disable-next-line no-await-in-loop
+							error = await hook(error as RequestError);
+						}
+					}
+				}
+			} catch (error_) {
+				error = error_;
+			}
+		}
+
+		throw error;
+	})() as CancelableRequest<never>;
 
-export default function createRejection(error: Error): CancelableRequest<never> {
-	const promise = Promise.reject(error) as CancelableRequest<never>;
 	const returnPromise = (): CancelableRequest<never> => promise;
 
 	promise.json = returnPromise;

diff --git a/source/core/index.ts b/source/core/index.ts
@@ -81,7 +81,7 @@ export type Method =
 
 type Promisable<T> = T | Promise<T>;
 
-export type InitHook = (options: Options & {url: string | URL}) => Promisable<void>;
+export type InitHook = (options: Options) => Promisable<void>;
 export type BeforeRequestHook = (options: NormalizedOptions) => Promisable<void | Response | ResponseLike>;
 export type BeforeRedirectHook = (options: NormalizedOptions, response: Response) => Promisable<void>;
 export type BeforeErrorHook = (error: RequestError) => Promisable<RequestError>;
@@ -507,47 +507,31 @@ export default class Request extends Duplex implements RequestEvents<Request> {
 			}
 		});
 
+		const {json, body, form} = options;
+		if (json || body || form) {
+			this._lockWrite();
+		}
+
 		(async (nonNormalizedOptions: Options) => {
 			try {
-				{
-					const {json, body, form} = nonNormalizedOptions;
-					if (json || body || form) {
-						this._lockWrite();
-					}
-				}
-
 				if (nonNormalizedOptions.body instanceof ReadStream) {
 					await waitForOpenFile(nonNormalizedOptions.body);
 				}
 
-				const initHooks = nonNormalizedOptions.hooks?.init;
-				const hasInitHooks = initHooks && initHooks.length !== 0;
-				if (hasInitHooks) {
-					nonNormalizedOptions.url = url;
-
-					for (const hook of initHooks!) {
-						// eslint-disable-next-line no-await-in-loop
-						await hook(nonNormalizedOptions as Options & {url: string | URL});
-					}
-
-					url = nonNormalizedOptions.url;
-					nonNormalizedOptions.url = undefined;
-				}
-
-				if (kIsNormalizedAlready in nonNormalizedOptions && !hasInitHooks) {
+				if (kIsNormalizedAlready in nonNormalizedOptions) {
 					this.options = nonNormalizedOptions as NormalizedOptions;
 				} else {
 					// @ts-ignore Common TypeScript bug saying that `this.constructor` is not accessible
 					this.options = this.constructor.normalizeArguments(url, nonNormalizedOptions, defaults);
 				}
 
-				const {options} = this;
+				const {url: normalizedURL} = this.options;
 
-				if (!options.url) {
+				if (!normalizedURL) {
 					throw new TypeError('Missing `url` property');
 				}
 
-				this.requestUrl = options.url.toString();
+				this.requestUrl = normalizedURL.toString();
 				decodeURI(this.requestUrl);
 
 				await this._finalizeBody();

diff --git a/source/create.ts b/source/create.ts
@@ -1,28 +1,35 @@
 import {URL} from 'url';
 import {CancelError} from 'p-cancelable';
 import asPromise, {
+	// Request & Response
 	PromisableRequest,
-	NormalizedOptions,
 	CancelableRequest,
-	Options,
 	Response,
+
+	// Options
+	Options,
+	NormalizedOptions,
 	Defaults as DefaultOptions,
+	PaginationOptions,
+
+	// Hooks
+	InitHook,
+
+	// Errors
 	ParseError,
-	PaginationOptions
-} from './as-promise';
-import createRejection from './as-promise/create-rejection';
-import Request, {
 	RequestError,
 	CacheError,
 	ReadError,
 	HTTPError,
 	MaxRedirectsError,
 	TimeoutError,
 	UnsupportedProtocolError,
-	UploadError,
-	kIsNormalizedAlready
-} from './core';
+	UploadError
+} from './as-promise';
+import createRejection from './as-promise/create-rejection';
+import Request, {kIsNormalizedAlready} from './core';
 import deepFreeze from './utils/deep-freeze';
+import is from '@sindresorhus/is/dist';
 
 export interface InstanceDefaults {
 	options: DefaultOptions;
@@ -164,6 +171,14 @@ export const mergeOptions = (...sources: Options[]): NormalizedOptions => {
 	return mergedOptions!;
 };
 
+const callInitHooks = (hooks: InitHook[] | undefined, options: Options): void => {
+	if (hooks) {
+		for (const hook of hooks) {
+			hook(options);
+		}
+	}
+};
+
 const create = (defaults: InstanceDefaults): Got => {
 	// Proxy properties from next handlers
 	defaults._rawHandlers = defaults.handlers;
@@ -193,7 +208,7 @@ const create = (defaults: InstanceDefaults): Got => {
 		return result;
 	}));
 
-	const got: Got = ((url: string | URL, options?: Options): GotReturn => {
+	const got: Got = ((url: string | URL, options: Options = {}): GotReturn => {
 		let iteration = 0;
 		const iterateHandlers = (newOptions: NormalizedOptions): GotReturn => {
 			return defaults.handlers[iteration++](
@@ -202,10 +217,33 @@ const create = (defaults: InstanceDefaults): Got => {
 			) as GotReturn;
 		};
 
+		if (is.plainObject(url)) {
+			options = {
+				...url as Options,
+				...options
+			};
+
+			url = undefined as any;
+		}
+
 		try {
+			// Call `init` hooks
+			let initHookError: Error | undefined;
+			try {
+				callInitHooks(defaults.options.hooks.init, options);
+				callInitHooks(options?.hooks?.init, options);
+			} catch (error) {
+				initHookError = error;
+			}
+
+			// Normalize options & call handlers
 			const normalizedOptions = normalizeArguments(url, options, defaults.options);
 			normalizedOptions[kIsNormalizedAlready] = true;
 
+			if (initHookError) {
+				throw new RequestError(initHookError.message, initHookError, normalizedOptions);
+			}
+
 			// A bug.
 			// eslint-disable-next-line @typescript-eslint/return-await
 			return iterateHandlers(normalizedOptions);
@@ -215,7 +253,7 @@ const create = (defaults: InstanceDefaults): Got => {
 			} else {
 				// A bug.
 				// eslint-disable-next-line @typescript-eslint/return-await
-				return createRejection(error);
+				return createRejection(error, defaults.options.hooks.beforeError, options?.hooks?.beforeError);
 			}
 		}
 	}) as Got;
@@ -259,7 +297,7 @@ const create = (defaults: InstanceDefaults): Got => {
 
 		const pagination = normalizedOptions._pagination!;
 
-		if (typeof pagination !== 'object') {
+		if (!is.object(pagination)) {
 			throw new TypeError('`options._pagination` must be implemented');
 		}
 

diff --git a/test/arguments.ts b/test/arguments.ts
@@ -380,7 +380,7 @@ test('throws a helpful error when passing `auth`', async t => {
 	});
 });
 
-test('throws when input starts with a slash and the `prefixUrl` option is present', async t => {
+test('throws on leading slashes', async t => {
 	await t.throwsAsync(got('/asdf', {prefixUrl: 'https://example.com'}), {
 		message: '`input` must not start with a slash when using `prefixUrl`'
 	});

diff --git a/test/hooks.ts b/test/hooks.ts
@@ -62,6 +62,38 @@ test('catches init thrown errors', async t => {
 	}), {message: errorString});
 });
 
+test('passes init thrown errors to beforeError hooks (promise-only)', async t => {
+	t.plan(2);
+
+	await t.throwsAsync(got('https://example.com', {
+		hooks: {
+			init: [() => {
+				throw error;
+			}],
+			beforeError: [error => {
+				t.is(error.message, errorString);
+
+				return error;
+			}]
+		}
+	}), {message: errorString});
+});
+
+test('passes init thrown errors to beforeError hooks (promise-only) - beforeError rejection', async t => {
+	const message = 'foo, bar!';
+
+	await t.throwsAsync(got('https://example.com', {
+		hooks: {
+			init: [() => {
+				throw error;
+			}],
+			beforeError: [() => {
+				throw new Error(message);
+			}]
+		}
+	}), {message});
+});
+
 test('catches beforeRequest thrown errors', async t => {
 	await t.throwsAsync(got('https://example.com', {
 		hooks: {
@@ -233,12 +265,11 @@ test('init allows modifications', withServer, async (t, server, got) => {
 		response.end(request.headers.foo);
 	});
 
-	const {body} = await got('meh', {
+	const {body} = await got('', {
 		headers: {},
 		hooks: {
 			init: [
 				options => {
-					options.url = '';
 					options.headers!.foo = 'bar';
 				}
 			]