Update/clarify interceptor.reply param juggling.

lib/common.js

@@ -40,7 +40,7 @@ const normalizeRequestOptions = function(options) {
  * from its utf8 representation.
  *
  * TODO: Reverse the semantics of this method and refactor calling code
- * accordingly. We've inadvertantly gotten it flipped.
+ * accordingly. We've inadvertently gotten it flipped.
  *
  * @param  {Object} buffer - a Buffer object
  */
@@ -245,7 +245,7 @@ const headersArrayToObject = function(rawHeaders) {
     const value = rawHeaders[i + 1]
 
     if (headers[key]) {
-      headers[key] = _.isArray(headers[key]) ? headers[key] : [headers[key]]
+      headers[key] = _.castArray(headers[key])
       headers[key].push(value)
     } else {
       headers[key] = value
@@ -377,6 +377,27 @@ function isStream(obj) {
   )
 }
 
+/**
+ * Casts a provided status code value to an integer.
+ *
+ * Raises an error if invalid.
+ * Note while RFC 7231 states status codes are only three digit integers,
+ * we don't bother to enforce that here.
+ */
+function statusCodeInt(input) {
+  if (Number.isInteger(input)) {
+    return input
+  }
+
+  const int = Number.parseInt(input)
+
+  if (Number.isNaN(int)) {
+    throw new Error(`Invalid ${typeof input} value for status code`)
+  }
+
+  return int
+}
+
 exports.normalizeRequestOptions = normalizeRequestOptions
 exports.isUtf8Representable = isUtf8Representable
 exports.overrideRequests = overrideRequests
@@ -394,3 +415,4 @@ exports.percentDecode = percentDecode
 exports.matchStringOrRegexp = matchStringOrRegexp
 exports.formatQueryValue = formatQueryValue
 exports.isStream = isStream
+exports.statusCodeInt = statusCodeInt

lib/interceptor.js

@@ -78,13 +78,26 @@ Interceptor.prototype.replyWithError = function replyWithError(errorMessage) {
 }
 
 Interceptor.prototype.reply = function reply(statusCode, body, rawHeaders) {
-  if (arguments.length <= 2 && _.isFunction(statusCode)) {
-    body = statusCode
-    statusCode = 200
+  // support the format of only passing in a callback
+  if (_.isFunction(statusCode)) {
+    if (arguments.length > 1) {
+      // it's not very Javascript-y to throw an error for this kind of thing, but because
+      // of legacy behavior, this error was added to reduce confusion for those migrating.
+      throw Error(
+        'Invalid arguments. When providing a function for the first argument, .reply does not accept other arguments.'
+      )
+    }
+    this.statusCode = null
+    this.callback = statusCode
+    this.fullResponseFromCallback = true
+  } else {
+    this.statusCode = common.statusCodeInt(statusCode)
+    if (_.isFunction(body)) {
+      this.callback = body
+      body = null
+    }
   }
 
-  this.statusCode = statusCode
-
   _.defaults(this.options, this.scope.scopeOptions)
 
   // If needed, convert rawHeaders from Array to Object.
@@ -94,6 +107,7 @@ Interceptor.prototype.reply = function reply(statusCode, body, rawHeaders) {
 
   if (this.scope._defaultReplyHeaders) {
     headers = headers || {}
+    // because of this, this.rawHeaders gets lower-cased versions of the `rawHeaders` param. is that a bug?
     headers = common.headersFieldNamesToLowerCase(headers)
     headers = mixin(this.scope._defaultReplyHeaders, headers)
   }
@@ -107,8 +121,7 @@ Interceptor.prototype.reply = function reply(statusCode, body, rawHeaders) {
     this.rawHeaders = []
 
     for (const key in headers) {
-      this.rawHeaders.push(key)
-      this.rawHeaders.push(headers[key])
+      this.rawHeaders.push(key, headers[key])
     }
 
     // We use lower-case headers throughout Nock.
@@ -120,9 +133,8 @@ Interceptor.prototype.reply = function reply(statusCode, body, rawHeaders) {
 
   //  If the content is not encoded we may need to transform the response body.
   //  Otherwise we leave it as it is.
-  if (!common.isContentEncoded(this.headers)) {
+  if (body && !common.isContentEncoded(this.headers)) {
     if (
-      body &&
       typeof body !== 'string' &&
       typeof body !== 'function' &&
       !Buffer.isBuffer(body) &&
@@ -419,7 +431,7 @@ Interceptor.prototype.matchAddress = function matchAddress(options) {
 
 Interceptor.prototype.filteringPath = function filteringPath() {
   if (_.isFunction(arguments[0])) {
-    this.scope.transformFunction = arguments[0]
+    this.scope.transformPathFunction = arguments[0]
   }
   return this
 }
@@ -454,7 +466,7 @@ Interceptor.prototype.basicAuth = function basicAuth(options) {
 /**
  * Set query strings for the interceptor
  * @name query
- * @param Object Object of query string name,values (accepts regexp values)
+ * @param queries Object of query string name,values (accepts regexp values)
  * @public
  * @example
  * // Will match 'http://zombo.com/?q=t'
@@ -496,7 +508,7 @@ Interceptor.prototype.query = function query(queries) {
 /**
  * Set number of times will repeat the interceptor
  * @name times
- * @param Integer Number of times to repeat (should be > 0)
+ * @param newCounter Number of times to repeat (should be > 0)
  * @public
  * @example
  * // Will repeat mock 5 times for same king of request
@@ -554,7 +566,7 @@ Interceptor.prototype.thrice = function thrice() {
  * @param {(integer|object)} opts - Number of milliseconds to wait, or an object
  * @param {integer} [opts.head] - Number of milliseconds to wait before response is sent
  * @param {integer} [opts.body] - Number of milliseconds to wait before response body is sent
- * @return {interceptor} - the current interceptor for chaining
+ * @return {Interceptor} - the current interceptor for chaining
  */
 Interceptor.prototype.delay = function delay(opts) {
   let headDelay = 0
@@ -565,7 +577,7 @@ Interceptor.prototype.delay = function delay(opts) {
     headDelay = opts.head || 0
     bodyDelay = opts.body || 0
   } else {
-    throw new Error(`Unexpected input opts${opts}`)
+    throw new Error(`Unexpected input opts ${opts}`)
   }
 
   return this.delayConnection(headDelay).delayBody(bodyDelay)
@@ -575,7 +587,7 @@ Interceptor.prototype.delay = function delay(opts) {
  * Delay the response body by a certain number of ms.
  *
  * @param {integer} ms - Number of milliseconds to wait before response is sent
- * @return {interceptor} - the current interceptor for chaining
+ * @return {Interceptor} - the current interceptor for chaining
  */
 Interceptor.prototype.delayBody = function delayBody(ms) {
   this.delayInMs += ms
@@ -586,7 +598,7 @@ Interceptor.prototype.delayBody = function delayBody(ms) {
  * Delay the connection by a certain number of ms.
  *
  * @param  {integer} ms - Number of milliseconds to wait
- * @return {interceptor} - the current interceptor for chaining
+ * @return {Interceptor} - the current interceptor for chaining
  */
 Interceptor.prototype.delayConnection = function delayConnection(ms) {
   this.delayConnectionInMs += ms
@@ -601,7 +613,7 @@ Interceptor.prototype.getTotalDelay = function getTotalDelay() {
  * Make the socket idle for a certain number of ms (simulated).
  *
  * @param  {integer} ms - Number of milliseconds to wait
- * @return {interceptor} - the current interceptor for chaining
+ * @return {Interceptor} - the current interceptor for chaining
  */
 Interceptor.prototype.socketDelay = function socketDelay(ms) {
   this.socketDelayInMs = ms

lib/request_overrider.js

@@ -286,15 +286,13 @@ function RequestOverrider(req, options, interceptors, remove, cb) {
       timers.setTimeout(emitError, interceptor.getTotalDelay(), error)
       return
     }
-    // TODO-coverage: Either add a test or remove `|| 200` if it's not reachable.
-    response.statusCode = Number(interceptor.statusCode) || 200
 
     // Clone headers/rawHeaders to not override them when evaluating later
     response.headers = _.extend({}, interceptor.headers)
     response.rawHeaders = (interceptor.rawHeaders || []).slice()
     debug('response.rawHeaders:', response.rawHeaders)
 
-    if (typeof interceptor.body === 'function') {
+    if (interceptor.callback) {
       if (requestBody && common.isJSONContent(req.headers)) {
         if (common.contentEncoding(req.headers, 'gzip')) {
           requestBody = String(zlib.gunzipSync(Buffer.from(requestBody, 'hex')))
@@ -308,15 +306,15 @@ function RequestOverrider(req, options, interceptors, remove, cb) {
       }
 
       // In case we are waiting for a callback
-      if (interceptor.body.length === 3) {
-        return interceptor.body(
+      if (interceptor.callback.length === 3) {
+        return interceptor.callback(
           options.path,
-          requestBody || '',
+          requestBody,
           continueWithResponseBody
         )
       }
 
-      responseBody = interceptor.body(options.path, requestBody) || ''
+      responseBody = interceptor.callback(options.path, requestBody)
     } else {
       //  If the content is encoded we know that the response body *must* be an array
       //  of response buffers which should be mocked one by one.
@@ -336,12 +334,7 @@ function RequestOverrider(req, options, interceptors, remove, cb) {
           return
         }
 
-        let buffers = interceptor.body
-        if (!_.isArray(buffers)) {
-          buffers = [buffers]
-        }
-
-        responseBuffers = _.map(buffers, function(buffer) {
+        responseBuffers = _.castArray(interceptor.body).map(function(buffer) {
           return Buffer.from(buffer, 'hex')
         })
       } else {
@@ -387,34 +380,23 @@ function RequestOverrider(req, options, interceptors, remove, cb) {
       if (err) {
         response.statusCode = 500
         responseBody = err.stack
+      } else if (interceptor.fullResponseFromCallback) {
+        try {
+          responseBody = parseResponseFromCallbackResult(response, responseBody)
+        } catch (err) {
+          emitError(err)
+          return
+        }
+      } else {
+        response.statusCode = interceptor.statusCode // already cast to int
       }
 
       //  Transform the response body if it exists (it may not exist
       //  if we have `responseBuffers` instead)
 
-      if (responseBody) {
+      if (!responseBuffers) {
         debug('transform the response body')
 
-        if (Array.isArray(responseBody)) {
-          debug('response body is array: %j', responseBody)
-
-          // TODO-coverage: else, throw an error. Add a test.
-          if (!isNaN(Number(responseBody[0]))) {
-            response.statusCode = Number(responseBody[0])
-          }
-
-          if (responseBody.length >= 2 && responseBody.length <= 3) {
-            debug('new headers: %j', responseBody[2])
-            _.assign(response.headers, responseBody[2] || {})
-            debug('response.headers after: %j', response.headers)
-            responseBody = responseBody[1]
-
-            Object.keys(response.headers).forEach(function(key) {
-              response.rawHeaders.push(key, response.headers[key])
-            })
-          }
-        }
-
         if (interceptor.delayInMs) {
           debug(
             'delaying the response for',
@@ -573,4 +555,40 @@ function RequestOverrider(req, options, interceptors, remove, cb) {
   return req
 }
 
+function parseResponseFromCallbackResult(response, callbackResult) {
+  debug('full response from callback result: %j', callbackResult)
+
+  if (!Array.isArray(callbackResult)) {
+    throw Error('A single function provided to .reply MUST return an array')
+  }
+
+  if (callbackResult.length > 3) {
+    throw Error(
+      'The array returned from the .reply callback contains too many values'
+    )
+  }
+
+  const [status, body, headers] = callbackResult
+
+  response.statusCode = common.statusCodeInt(status)
+
+  if (headers) {
+    debug('new headers: %j', headers)
+
+    const rawHeaders = Array.isArray(headers)
+      ? headers
+      : _.flatten(Object.entries(headers))
+    response.rawHeaders.push(...rawHeaders)
+
+    const castHeaders = Array.isArray(headers)
+      ? common.headersArrayToObject(headers)
+      : common.headersFieldNamesToLowerCase(headers)
+
+    _.assign(response.headers, castHeaders)
+    debug('response.headers after: %j', response.headers)
+  }
+
+  return body || ''
+}
+
 module.exports = RequestOverrider

lib/scope.js

@@ -218,6 +218,7 @@ Scope.prototype.matchHeader = function matchHeader(name, value) {
 }
 
 Scope.prototype.defaultReplyHeaders = function defaultReplyHeaders(headers) {
+  // Because these are lower-cased, res.rawHeaders can have the wrong keys. bug?
   this._defaultReplyHeaders = common.headersFieldNamesToLowerCase(headers)
   return this
 }
@@ -333,7 +334,7 @@ function define(nockDefs) {
     options.badheaders = badheaders
 
     //  Response is not always JSON as it could be a string or binary data or
-    //  even an array of binary buffers (e.g. when content is enconded)
+    //  even an array of binary buffers (e.g. when content is encoded)
     let response
     if (!nockDef.response) {
       response = ''
@@ -348,7 +349,7 @@ function define(nockDefs) {
     let nock
     // TODO-coverage: Find out what `body === '*'` means. Add a comment here
     // explaining it, and a test case if necessary. Also, update the readme
-    // example of `filteringRequestBody()` beacuse it doesn't explain what the
+    // example of `filteringRequestBody()` because it doesn't explain what the
     // `*` really means.
     if (body === '*') {
       nock = startScope(nscope, options)

tests/test_common.js

@@ -337,3 +337,25 @@ test('headersArrayToObject', function(t) {
 
   t.end()
 })
+
+test('statusCodeInt casts valid inputs', t => {
+  t.equal(common.statusCodeInt(418), 418)
+  t.equal(common.statusCodeInt('418'), 418)
+  t.equal(common.statusCodeInt('418.000'), 418)
+  t.end()
+})
+
+test('statusCodeInt throws for invalid inputs', t => {
+  t.throws(() => common.statusCodeInt(''), Error('Invalid string value'))
+  t.throws(() => common.statusCodeInt('foobar'), Error('Invalid string value'))
+  t.throws(
+    () => common.statusCodeInt({ foo: 'bar' }),
+    Error('Invalid object value')
+  )
+  t.throws(() => common.statusCodeInt(null), Error('Invalid object value'))
+  t.throws(
+    () => common.statusCodeInt(undefined),
+    Error('Invalid undefined value')
+  )
+  t.end()
+})

tests/test_content_encoding.js

@@ -13,12 +13,13 @@ test('accepts gzipped content', async t => {
 
   nock('http://example.test')
     .get('/foo')
-    .reply(200, compressed, {
+    .reply('200', compressed, {
       'X-Transfer-Length': String(compressed.length),
       'Content-Length': undefined,
       'Content-Encoding': 'gzip',
     })
-  const { body } = await got('http://example.test/foo')
+  const { body, statusCode } = await got('http://example.test/foo')
 
   t.equal(body, message)
+  t.equal(statusCode, 200)
 })