fix(intercept): Better error message when options is falsy (#1440)

paulmelnikow · web-flow · commit 7e447696ebd8 · 2019-04-13T17:53:47.000-04:00
diff --git a/lib/intercept.js b/lib/intercept.js
@@ -249,12 +249,16 @@ function overrideClientRequest() {
   //  Define the overriding client request that nock uses internally.
   function OverriddenClientRequest(options, cb) {
     if (!options) {
-      // In principle, there is no reason we couldn't support this. However it
-      // doesn't work, and fixing it seems low priority. Giving an explicit
-      // error seems nicer than crashing with a weird stack trace.
+      // As weird as it is, it's possible to call `http.request` without
+      // options, and it makes a request to localhost or somesuch. We should
+      // support it too, for parity. However it doesn't work today, and fixing
+      // it seems low priority. Giving an explicit error is nicer than
+      // crashing with a weird stack trace. `http[s].request()`, nock's other
+      // client-facing entry point, makes a similar check.
       // https://github.com/nock/nock/pull/1386
+      // https://github.com/nock/nock/pull/1440
       throw Error(
-        'Creating a client request with empty `options` is not supported in Nock'
+        'Creating a ClientRequest with empty `options` is not supported in Nock'
       )
     }
 
@@ -386,6 +390,18 @@ function activate() {
       options = urlParse(options)
     } else if (options instanceof URL) {
       options = urlParse(options.toString())
+    } else if (!options) {
+      // As weird as it is, it's possible to call `http.request` without
+      // options, and it makes a request to localhost or somesuch. We should
+      // support it too, for parity. However it doesn't work today, and fixing
+      // it seems low priority. Giving an explicit error is nicer than
+      // crashing with a weird stack trace. `new ClientRequest()`, nock's
+      // other client-facing entry point, makes a similar check.
+      // https://github.com/nock/nock/pull/1386
+      // https://github.com/nock/nock/pull/1440
+      throw Error(
+        'Making a request with empty `options` is not supported in Nock'
+      )
     }
     options.proto = proto
 
diff --git a/tests/test_clientrequest.js b/tests/test_clientrequest.js
@@ -73,7 +73,7 @@ test('can use ClientRequest using POST', t => {
 test('creating ClientRequest with empty options throws expected error', t => {
   t.throws(() => new http.ClientRequest(), {
     message:
-      'Creating a client request with empty `options` is not supported in Nock',
+      'Creating a ClientRequest with empty `options` is not supported in Nock',
   })
 
   t.end()
diff --git a/tests/test_intercept.js b/tests/test_intercept.js
@@ -1701,6 +1701,13 @@ test('data is sent with flushHeaders', t => {
     .flushHeaders()
 })
 
+test('should throw expected error when creating request with missing options', t => {
+  t.throws(() => http.request(), {
+    message: 'Making a request with empty `options` is not supported in Nock',
+  })
+  t.end()
+})
+
 test('teardown', t => {
   let leaks = Object.keys(global).splice(globalCount, Number.MAX_VALUE)
 
