Fix overriding some options in a beforeRequest hook (sindresorhus#1293

)
jaulz · Jul 6, 2020 · 71e0ee3 · 71e0ee3
1 parent 9a7c8f1
commit 71e0ee3
Show file tree

Hide file tree

Showing 2 changed files with 58 additions and 2 deletions.
diff --git a/source/core/index.ts b/source/core/index.ts
@@ -1313,7 +1313,8 @@ export default class Request extends Duplex implements RequestEvents<Request> {
 
 	async _makeRequest(): Promise<void> {
 		const {options} = this;
-		const {url, headers, request, agent, timeout} = options;
+
+		const {headers} = options;
 
 		for (const key in headers) {
 			if (is.undefined(headers[key])) {
@@ -1348,6 +1349,8 @@ export default class Request extends Duplex implements RequestEvents<Request> {
 			}
 		}
 
+		const {agent, request, timeout, url} = options;
+
 		if (options.dnsCache && !('lookup' in options)) {
 			options.lookup = options.dnsCache.lookup;
 		}

diff --git a/test/hooks.ts b/test/hooks.ts
@@ -1,7 +1,9 @@
 import {URL} from 'url';
-import test from 'ava';
+import {Agent as HttpAgent} from 'http';
+import test, {Constructor} from 'ava';
 import nock = require('nock');
 import getStream from 'get-stream';
+import sinon = require('sinon');
 import delay = require('delay');
 import {Handler} from 'express';
 import Responselike = require('responselike');
@@ -36,6 +38,13 @@ const redirectEndpoint: Handler = (_request, response) => {
 	response.end();
 };
 
+const createAgentSpy = <T extends HttpAgent>(AgentClass: Constructor): {agent: T; spy: sinon.SinonSpy} => {
+	const agent: T = new AgentClass({keepAlive: true});
+	// @ts-ignore This IS correct
+	const spy = sinon.spy(agent, 'addRequest');
+	return {agent, spy};
+};
+
 test('async hooks', withServer, async (t, server, got) => {
 	server.get('/', echoHeaders);
 
@@ -899,3 +908,47 @@ test('async afterResponse allows to retry with allowGetBody and json payload', w
 	});
 	t.is(statusCode, 200);
 });
+
+test('beforeRequest hook respect `agent` option', withServer, async (t, server, got) => {
+	server.get('/', (_request, response) => {
+		response.end('ok');
+	});
+
+	const {agent, spy} = createAgentSpy(HttpAgent);
+
+	t.truthy((await got({
+		hooks: {
+			beforeRequest: [
+				options => {
+					options.agent = {
+						http: agent
+					};
+				}
+			]
+		}
+	})).body);
+	t.true(spy.calledOnce);
+
+	// Make sure to close all open sockets
+	agent.destroy();
+});
+
+test('beforeRequest hook respect `url` option', withServer, async (t, server, got) => {
+	server.get('/', (_request, response) => {
+		response.end('ko');
+	});
+
+	server.get('/changed', (_request, response) => {
+		response.end('ok');
+	});
+
+	t.is((await got(server.sslHostname, {
+		hooks: {
+			beforeRequest: [
+				options => {
+					options.url = new URL(server.url + '/changed');
+				}
+			]
+		}
+	})).body, 'ok');
+});