fix (datafile manager): Pass hostname and port in request options (optimizely#18)

mjc1283 · web-flow · commit 3287c367e1c2 · 2019-04-08T17:52:58.000-07:00
Summary: Before this change, we were passing the host property returned from url.parse as the host option going into http.request (or https.request). The host property from url.parse includes the port when the URL contains a port. The problem is, request needs port to be passed as a separate option (not included as part of the host option). To fix, stop passing host, and replace it with hostname: url.hostname (where url.hostname excludes port), and port: url.port. References: https://nodejs.org/api/http.html#http_http_request_url_options_callback https://nodejs.org/api/url.html#url_urlobject_host https://nodejs.org/api/url.html#url_urlobject_hostname https://nodejs.org/api/url.html#url_urlobject_port Test plan: Added a regression test. Manually tested in @mikeng13's prototype datafile management branches for JS-testapp and compatibility suite.
diff --git a/packages/datafile-manager/__test__/nodeRequest.spec.ts b/packages/datafile-manager/__test__/nodeRequest.spec.ts
@@ -140,5 +140,21 @@ describe('nodeEnvironment', () => {
       await expect(req.responsePromise).rejects.toThrow()
       scope.done()
     })
+
+    it('handles a url with a host and a port', async () => {
+      const hostWithPort = 'http://datafiles:3000'
+      const path = '/12/345.json'
+      const scope = nock(hostWithPort)
+        .get(path)
+        .reply(200, '{"foo":"bar"}')
+      const req = makeGetRequest(`${hostWithPort}${path}`, {})
+      const resp = await req.responsePromise
+      expect(resp).toEqual({
+        statusCode: 200,
+        body: '{"foo":"bar"}',
+        headers: {}
+      })
+      scope.done()
+    })
   })
 })
diff --git a/packages/datafile-manager/src/nodeRequest.ts b/packages/datafile-manager/src/nodeRequest.ts
@@ -24,9 +24,10 @@ type ClientRequestCreator = (options: http.RequestOptions) => http.ClientRequest
 
 function getRequestOptionsFromUrl(url: url.UrlWithStringQuery): http.RequestOptions {
   return {
-    protocol: url.protocol,
-    host: url.host,
+    hostname: url.hostname,
     path: url.path,
+    port: url.port,
+    protocol: url.protocol,
   }
 }
 

Original file line number	Diff line number	Diff line change
`@@ -24,9 +24,10 @@ type ClientRequestCreator = (options: http.RequestOptions) => http.ClientRequest`
`24`	`24`
`25`	`25`	`function getRequestOptionsFromUrl(url: url.UrlWithStringQuery): http.RequestOptions {`
`26`	`26`	`return {`
`27`		`- protocol: url.protocol,`
`28`		`- host: url.host,`
	`27`	`+ hostname: url.hostname,`
`29`	`28`	`path: url.path,`
	`29`	`+ port: url.port,`
	`30`	`+ protocol: url.protocol,`
`30`	`31`	`}`
`31`	`32`	`}`
`32`	`33`