From c87fa5233cda1cc0b193fa448392fc357c55e153 Mon Sep 17 00:00:00 2001 From: "Matt R. Wilson" Date: Wed, 1 May 2019 13:58:30 -0600 Subject: [PATCH 01/19] chore: update/clarify interceptor.reply param juggling. Based on conversation https://github.com/nock/nock/pull/1517/files#r280139478. The `arguments.length <= 2` was confusing/misleading, and clearing the `rawHeaders` clarifies the method's intention to anyone digging around, plus reduces the changes for bugs in the future. --- lib/interceptor.js | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/interceptor.js b/lib/interceptor.js index a43ed8e3a..f2b3a8f13 100644 --- a/lib/interceptor.js +++ b/lib/interceptor.js @@ -78,9 +78,11 @@ Interceptor.prototype.replyWithError = function replyWithError(errorMessage) { } Interceptor.prototype.reply = function reply(statusCode, body, rawHeaders) { - if (arguments.length <= 2 && _.isFunction(statusCode)) { + // support the format of only passing in a callback + if (_.isFunction(statusCode)) { body = statusCode statusCode = 200 + rawHeaders = undefined } this.statusCode = statusCode @@ -107,8 +109,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. From 9b92974909335390efd96f0237ec1e11e64262db Mon Sep 17 00:00:00 2001 From: "Matt R. Wilson" Date: Wed, 1 May 2019 13:58:55 -0600 Subject: [PATCH 02/19] chore(docs): update JSDoc types in interceptor. --- lib/interceptor.js | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/lib/interceptor.js b/lib/interceptor.js index f2b3a8f13..c998c636a 100644 --- a/lib/interceptor.js +++ b/lib/interceptor.js @@ -454,7 +454,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 +496,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 +554,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 +565,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 +575,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 +586,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 +601,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 From 4cae0ba4154073642641851b8baefda6e5e17831 Mon Sep 17 00:00:00 2001 From: Paul Melnikow Date: Sun, 5 May 2019 19:38:13 -0400 Subject: [PATCH 03/19] Add explicit status code and body checks --- tests/test_reply_function_sync.js | 52 +++++++++++++++++-------------- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/tests/test_reply_function_sync.js b/tests/test_reply_function_sync.js index fbd992c54..d32217b6b 100644 --- a/tests/test_reply_function_sync.js +++ b/tests/test_reply_function_sync.js @@ -14,9 +14,10 @@ require('./cleanup_after_each')() test('reply with status code and function returning body as string', async t => { const scope = nock('http://example.com') .get('/') - .reply(200, () => 'OK!') + .reply(201, () => 'OK!') - const { body } = await got('http://example.com') + const { statusCode, body } = await got('http://example.com') + t.is(statusCode, 201) t.equal(body, 'OK!') scope.done() }) @@ -26,9 +27,10 @@ test('reply with status code and function returning body object', async t => { const scope = nock('http://example.test') .get('/') - .reply(200, () => exampleResponse) + .reply(201, () => exampleResponse) - const { body } = await got('http://example.test') + const { statusCode, body } = await got('http://example.test') + t.is(statusCode, 201) t.equal(body, JSON.stringify(exampleResponse)) scope.done() }) @@ -36,9 +38,10 @@ test('reply with status code and function returning body object', async t => { test('reply with status code and function returning body as number', async t => { const scope = nock('http://example.test') .get('/') - .reply(200, () => 123) + .reply(201, () => 123) - const { body } = await got('http://example.test') + const { statusCode, body } = await got('http://example.test') + t.is(statusCode, 201) t.equal(body, '123') scope.done() }) @@ -46,16 +49,17 @@ test('reply with status code and function returning body as number', async t => // The observed behavior is that this returns a 123 status code. // // The expected behavior is that this should either throw an error or reply -// with 200 and the JSON-stringified '[123]'. +// with 201 and the JSON-stringified '[123]'. test( 'reply with status code and function returning array', { skip: true }, async t => { const scope = nock('http://example.test') .get('/') - .reply(200, () => [123]) + .reply(201, () => [123]) - const { body } = await got('http://example.test') + const { statusCode, body } = await got('http://example.test') + t.is(statusCode, 201) t.equal(body, '[123]') scope.done() } @@ -74,7 +78,7 @@ test('reply function with string body using POST', async t => { body: exampleRequestBody, }), ({ statusCode, body }) => { - t.equal(statusCode, 404) + t.is(statusCode, 404) t.equal(body, exampleResponseBody) return true } @@ -107,49 +111,51 @@ test('reply function receives the request URL and body', async t => { }) test('when content-type is json, reply function receives parsed body', async t => { - t.plan(3) + t.plan(4) const exampleRequestBody = JSON.stringify({ id: 1, name: 'bob' }) const scope = nock('http://example.test') .post('/') - .reply(200, (uri, requestBody) => { + .reply(201, (uri, requestBody) => { t.type(requestBody, 'object') t.deepEqual(requestBody, JSON.parse(exampleRequestBody)) }) - const { statusCode } = await got('http://example.test/', { + const { statusCode, body } = await got('http://example.test/', { headers: { 'Content-Type': 'application/json' }, body: exampleRequestBody, }) - t.is(statusCode, 200) + t.is(statusCode, 201) + t.equal(body, '') scope.done() }) test('without content-type header, body sent to reply function is not parsed', async t => { - t.plan(3) + t.plan(4) const exampleRequestBody = JSON.stringify({ id: 1, name: 'bob' }) const scope = nock('http://example.test') .post('/') - .reply(200, (uri, requestBody) => { + .reply(201, (uri, requestBody) => { t.type(requestBody, 'string') t.equal(requestBody, exampleRequestBody) }) - const { statusCode } = await got.post('http://example.test/', { + const { statusCode, body } = await got.post('http://example.test/', { body: exampleRequestBody, }) - t.is(statusCode, 200) + t.is(statusCode, 201) + t.equal(body, '') scope.done() }) // This signature is supported today, however it seems unnecessary. This is // just as easily accomplished with a function returning an array: -// `.reply(() => [200, 'ABC', { 'X-My-Headers': 'My custom header value' }])` +// `.reply(() => [201, 'ABC', { 'X-My-Headers': 'My custom header value' }])` test('reply with status code, function returning string body, and header object', async t => { const scope = nock('http://example.com') .get('/') - .reply(200, () => 'ABC', { 'X-My-Headers': 'My custom header value' }) + .reply(201, () => 'ABC', { 'X-My-Headers': 'My custom header value' }) const { headers } = await got('http://example.com/') @@ -180,9 +186,9 @@ test('reply function returning array with status code and string body', async t .get('/') .reply(() => [401, 'This is a body']) - await assertRejects(got('http://example.com/'), err => { - t.equal(err.statusCode, 401) - t.equal(err.body, 'This is a body') + await assertRejects(got('http://example.com/'), ({ statusCode, body }) => { + t.is(statusCode, 401) + t.equal(body, 'This is a body') return true }) scope.done() From be2b3101264ed4938ff6e2a5a934c8f1ab618402 Mon Sep 17 00:00:00 2001 From: Paul Melnikow Date: Sun, 5 May 2019 20:00:02 -0400 Subject: [PATCH 04/19] Add tests of reply() with no arguments --- tests/test_reply_body.js | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/test_reply_body.js b/tests/test_reply_body.js index 17f116a57..cb238cd77 100644 --- a/tests/test_reply_body.js +++ b/tests/test_reply_body.js @@ -91,3 +91,27 @@ test('unencodable object throws the expected error', t => { t.end() }) + +test('reply with missing body defaults to empty', async t => { + const scope = nock('http://example.test') + .get('/') + .reply(204) + + const { statusCode, body } = await got('http://example.test/') + + t.is(statusCode, 204) + t.equal(body, '') + scope.done() +}) + +test('reply with missing status code defaults to 200 + empty body', async t => { + const scope = nock('http://example.test') + .get('/') + .reply() + + const { statusCode, body } = await got('http://example.test/') + + t.is(statusCode, 200) + t.equal(body, '') + scope.done() +}) From 14fe6d0c724402372a9ed4c53505d961b0caf2b0 Mon Sep 17 00:00:00 2001 From: "Matt R. Wilson" Date: Mon, 6 May 2019 10:43:10 -0600 Subject: [PATCH 05/19] Add statusCodeInt helper too common. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit only includes the implementation and tests. Callers are to follow, but I didn’t want to muddy commit history. --- lib/common.js | 22 ++++++++++++++++++++++ tests/test_common.js | 22 ++++++++++++++++++++++ tests/test_content_encoding.js | 5 +++-- 3 files changed, 47 insertions(+), 2 deletions(-) diff --git a/lib/common.js b/lib/common.js index 9a4213911..c48879de7 100644 --- a/lib/common.js +++ b/lib/common.js @@ -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 diff --git a/tests/test_common.js b/tests/test_common.js index 3f09e61e5..b384e47b7 100644 --- a/tests/test_common.js +++ b/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() +}) diff --git a/tests/test_content_encoding.js b/tests/test_content_encoding.js index 19bc4b1bd..39aa85d37 100644 --- a/tests/test_content_encoding.js +++ b/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) }) From 9913f68f5a3d7c1319fb70c9eb6054f2c7ba14f7 Mon Sep 17 00:00:00 2001 From: "Matt R. Wilson" Date: Mon, 6 May 2019 11:32:07 -0600 Subject: [PATCH 06/19] Simplify Interceptor.reply signature. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Includes breaking changes. - When a status code is provided followed by a function, the function returns the body. No magic. - When a function is provided on its own, it _MUST_ return an array of `status, [body, [headers]]`. Again, no magic. This change uses errors to enforce the following two signatures: `.reply(status, [body, [headers]])` where `body` is any of the existing types that can result in a scalar value. `.reply(() => [status, body = ‘’, headers = {}])` where the callback is called using the existing mechanics. #### Breaking changes - There is no longer a default value for the response status code. It must be provided as the first arg of `reply` or the first element in the returned array if using the single-function style. - `.reply()` will now throw an error - Status code args will throw an error if they cannot be cast to ints. - The single-function style callback **MUST** return an array with a length of 1-3 or else an error is thrown. - If **NOT** using the single-function style callback, the resulting body will never be treated as a special value for status code or headers, it will always result to the responses body value. - `reply(200, () => [400, ‘foo’])` - **previous result** status: 400 body: ‘foo’ - **new result** status: 200 body: “[400, ‘foo’]” - `reply(200, () => [400])` - **previous result** status: 400 body: “[400]” - **new result** status: 200 body: “[400]” --- lib/interceptor.js | 24 ++++++--- lib/request_overrider.js | 86 ++++++++++++++++++------------ tests/test_dynamic_mock.js | 78 ++++++++++++++++++++------- tests/test_events.js | 2 +- tests/test_gzip_request.js | 4 +- tests/test_reply_body.js | 17 +++--- tests/test_reply_function_async.js | 6 ++- tests/test_reply_function_sync.js | 53 +++++++----------- tests/test_stream.js | 1 + 9 files changed, 164 insertions(+), 107 deletions(-) diff --git a/lib/interceptor.js b/lib/interceptor.js index 733d4b74d..59888edba 100644 --- a/lib/interceptor.js +++ b/lib/interceptor.js @@ -80,13 +80,24 @@ Interceptor.prototype.replyWithError = function replyWithError(errorMessage) { Interceptor.prototype.reply = function reply(statusCode, body, rawHeaders) { // support the format of only passing in a callback if (_.isFunction(statusCode)) { - body = statusCode - statusCode = 200 - rawHeaders = undefined + 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. @@ -121,9 +132,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) && diff --git a/lib/request_overrider.js b/lib/request_overrider.js index 34912fd66..cddc68629 100644 --- a/lib/request_overrider.js +++ b/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 diff --git a/tests/test_dynamic_mock.js b/tests/test_dynamic_mock.js index 1b7b9fd08..a7f29fb8c 100644 --- a/tests/test_dynamic_mock.js +++ b/tests/test_dynamic_mock.js @@ -6,19 +6,49 @@ const nock = require('..') require('./cleanup_after_each')() -test('one function returning the body defines a full mock', function(t) { +test('one function not returning an array causes an error', function(t) { nock('http://example.test') .get('/abc') .reply(function() { return 'ABC' }) - request.get('http://example.test/abc', function(err, resp, body) { - if (err) { - throw err - } - t.equal(resp.statusCode, 200) - t.equal(body, 'ABC') + request.get('http://example.test/abc', function(err) { + t.match( + err, + Error('A single function provided to .reply MUST return an array') + ) + t.end() + }) +}) + +test('one function returning an empty array causes an error', function(t) { + nock('http://example.test') + .get('/abc') + .reply(function() { + return [] + }) + + request.get('http://example.test/abc', function(err) { + t.match(err, Error('Invalid undefined value for status code')) + t.end() + }) +}) + +test('one function returning too large an array causes an error', function(t) { + nock('http://example.test') + .get('/abc') + .reply(function() { + return ['user', 'probably', 'intended', 'this', 'to', 'be', 'JSON'] + }) + + request.get('http://example.test/abc', function(err) { + t.match( + err, + Error( + 'The array returned from the .reply callback contains too many values' + ) + ) t.end() }) }) @@ -31,15 +61,29 @@ test('one function returning the status code and body defines a full mock', func }) request.get('http://example.test/def', function(err, resp, body) { - if (err) { - throw err - } + t.error(err) t.equal(resp.statusCode, 201) t.equal(body, 'DEF') t.end() }) }) +test('one function returning an array can set headers', function(t) { + nock('http://example.test') + .get('/def') + .reply(function() { + return [201, 'DEF', { 'X-My-Header': 'some-value' }] + }) + + request.get('http://example.test/def', function(err, resp, body) { + t.error(err) + t.equal(resp.statusCode, 201) + t.equal(body, 'DEF') + t.deepEqual(resp.headers, { 'x-my-header': 'some-value' }) + t.end() + }) +}) + test('one asynchronous function returning the status code and body defines a full mock', function(t) { nock('http://example.test') .get('/ghi') @@ -50,9 +94,7 @@ test('one asynchronous function returning the status code and body defines a ful }) request.get('http://example.test/ghi', function(err, resp, body) { - if (err) { - throw err - } + t.error(err) t.equal(resp.statusCode, 201) t.equal(body, 'GHI') t.end() @@ -70,7 +112,7 @@ test('asynchronous function gets request headers', function(t) { host: 'example.test', }) setTimeout(function() { - cb(null, [201, 'GHI']) + cb(null, 'foobar') }, 1e3) }) @@ -84,11 +126,9 @@ test('asynchronous function gets request headers', function(t) { }, }, function(err, resp, body) { - if (err) { - throw err - } - t.equal(resp.statusCode, 201) - t.equal(body, 'GHI') + t.error(err) + t.equal(resp.statusCode, 200) + t.equal(body, 'foobar') t.end() } ) diff --git a/tests/test_events.js b/tests/test_events.js index 45d21f90a..9c039c90d 100644 --- a/tests/test_events.js +++ b/tests/test_events.js @@ -71,7 +71,7 @@ test('emits no match when no match and no mock', function(t) { test('emits no match when no match and mocked', function(t) { nock('http://example.test') .get('/') - .reply('howdy') + .reply(418) const assertion = function(req) { t.equal(req.path, '/definitelymaybe') diff --git a/tests/test_gzip_request.js b/tests/test_gzip_request.js index 9d1f8d0d8..abf84cc1a 100644 --- a/tests/test_gzip_request.js +++ b/tests/test_gzip_request.js @@ -19,7 +19,7 @@ test('accepts and decodes gzip encoded application/json', t => { .reply(function(url, actual) { t.same(actual, message) t.end() - return 200 + return [200] }) const req = http.request({ @@ -50,7 +50,7 @@ test('accepts and decodes deflate encoded application/json', t => { .reply(function(url, actual) { t.same(actual, message) t.end() - return 200 + return [200] }) const req = http.request({ diff --git a/tests/test_reply_body.js b/tests/test_reply_body.js index cb238cd77..18239466f 100644 --- a/tests/test_reply_body.js +++ b/tests/test_reply_body.js @@ -104,14 +104,11 @@ test('reply with missing body defaults to empty', async t => { scope.done() }) -test('reply with missing status code defaults to 200 + empty body', async t => { - const scope = nock('http://example.test') - .get('/') - .reply() - - const { statusCode, body } = await got('http://example.test/') - - t.is(statusCode, 200) - t.equal(body, '') - scope.done() +test('reply with missing status code throws an error', async t => { + const interceptor = nock('http://example.test').get('/') + t.throws( + () => interceptor.reply(), + Error('Invalid undefined value for status code') + ) + t.end() }) diff --git a/tests/test_reply_function_async.js b/tests/test_reply_function_async.js index 256111b88..b78eccfea 100644 --- a/tests/test_reply_function_async.js +++ b/tests/test_reply_function_async.js @@ -41,7 +41,11 @@ test('reply takes a callback for status code', async t => { const response = await got('http://example.com/') t.equal(response.statusCode, expectedStatusCode, 'sends status code') - t.deepEqual(response.headers, headers, 'sends headers') + t.deepEqual( + response.headers, + { 'x-custom-header': 'abcdef' }, + 'sends headers' + ) t.equal(response.body, responseBody, 'sends request body') scope.done() }) diff --git a/tests/test_reply_function_sync.js b/tests/test_reply_function_sync.js index d32217b6b..f1b9e4a6f 100644 --- a/tests/test_reply_function_sync.js +++ b/tests/test_reply_function_sync.js @@ -46,24 +46,16 @@ test('reply with status code and function returning body as number', async t => scope.done() }) -// The observed behavior is that this returns a 123 status code. -// -// The expected behavior is that this should either throw an error or reply -// with 201 and the JSON-stringified '[123]'. -test( - 'reply with status code and function returning array', - { skip: true }, - async t => { - const scope = nock('http://example.test') - .get('/') - .reply(201, () => [123]) - - const { statusCode, body } = await got('http://example.test') - t.is(statusCode, 201) - t.equal(body, '[123]') - scope.done() - } -) +test('reply with status code and function returning array', async t => { + const scope = nock('http://example.test') + .get('/') + .reply(201, () => [123]) + + const { statusCode, body } = await got('http://example.test') + t.is(statusCode, 201) + t.equal(body, '[123]') + scope.done() +}) test('reply function with string body using POST', async t => { const exampleRequestBody = 'key=val' @@ -164,27 +156,22 @@ test('reply with status code, function returning string body, and header object' scope.done() }) -test( - 'reply function returning array with status code', - // Seems likely a bug related to https://github.com/nock/nock/issues/1222. - { skip: true }, - async t => { - const scope = nock('http://example.test') - .get('/') - .reply(() => [202]) +test('reply function returning array with status code', async t => { + const scope = nock('http://example.test') + .get('/') + .reply(() => [202]) - const { statusCode, body } = await got('http://example.test/') + const { statusCode, body } = await got('http://example.test/') - t.is(statusCode, 202) - t.equal(body, '') - scope.done() - } -) + t.equal(statusCode, 202) + t.equal(body, '') + scope.done() +}) test('reply function returning array with status code and string body', async t => { const scope = nock('http://example.com') .get('/') - .reply(() => [401, 'This is a body']) + .reply(() => ['401', 'This is a body']) await assertRejects(got('http://example.com/'), ({ statusCode, body }) => { t.is(statusCode, 401) diff --git a/tests/test_stream.js b/tests/test_stream.js index b74eb82be..26eb781e0 100644 --- a/tests/test_stream.js +++ b/tests/test_stream.js @@ -245,6 +245,7 @@ test( res.setEncoding('utf8') let body = '' + t.equal(res.statusCode, 200) res.on('data', function(chunk) { body += chunk From 7d1a3be52db90e3e3850833d821967977d307b38 Mon Sep 17 00:00:00 2001 From: "Matt R. Wilson" Date: Mon, 6 May 2019 11:32:35 -0600 Subject: [PATCH 07/19] Notes and tests about inconsistent raw headers. --- lib/common.js | 4 +-- lib/interceptor.js | 1 + lib/scope.js | 5 +-- tests/test_reply_headers.js | 70 +++++++++++++++++++++++++++++++++++++ 4 files changed, 76 insertions(+), 4 deletions(-) diff --git a/lib/common.js b/lib/common.js index c48879de7..982449f32 100644 --- a/lib/common.js +++ b/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 diff --git a/lib/interceptor.js b/lib/interceptor.js index 59888edba..9b3c7ea5d 100644 --- a/lib/interceptor.js +++ b/lib/interceptor.js @@ -107,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) } diff --git a/lib/scope.js b/lib/scope.js index c19339169..fd7c539c5 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -218,6 +218,7 @@ Scope.prototype.matchHeader = function matchHeader(name, value) { } Scope.prototype.defaultReplyHeaders = function defaultReplyHeaders(headers) { + // Because there 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) diff --git a/tests/test_reply_headers.js b/tests/test_reply_headers.js index f1e086754..2b55d9670 100644 --- a/tests/test_reply_headers.js +++ b/tests/test_reply_headers.js @@ -36,6 +36,76 @@ test('reply headers as function work', async t => { scope.done() }) +// Skipping these two test because of the inconsistencies around raw headers. +// - they often receive the lower-cased versions of the keys +// - the resulting order differs depending if overrides are provided to .reply directly or via a callback +// - replacing values with function results isn't guaranteed to keep the correct order +// - the resulting `headers` object itself is fine and these assertions pass +test('reply headers and defaults', { skip: true }, async t => { + const scope = nock('http://example.com') + .defaultReplyHeaders({ + 'X-Powered-By': 'Meeee', + 'X-Another-Header': 'Hey man!', + }) + .get('/') + .reply(200, 'Success!', { + 'X-Custom-Header': 'boo!', + 'x-another-header': 'foobar', + }) + + const { headers, rawHeaders } = await got('http://example.com/') + + t.equivalent(headers, { + 'x-custom-header': 'boo!', + 'x-another-header': 'foobar', // note this overrode the default value, despite the case difference + 'x-powered-by': 'Meeee', + }) + t.equivalent(rawHeaders, [ + 'X-Powered-By', + 'Meeee', + 'X-Another-Header', + 'Hey man!', + 'X-Custom-Header', + 'boo!', + 'x-another-header', + 'foobar', + ]) + scope.done() +}) + +test('reply headers from callback and defaults', { skip: true }, async t => { + const scope = nock('http://example.com') + .defaultReplyHeaders({ + 'X-Powered-By': 'Meeee', + 'X-Another-Header': 'Hey man!', + }) + .get('/') + .reply(() => [ + 200, + 'Success!', + { 'X-Custom-Header': 'boo!', 'x-another-header': 'foobar' }, + ]) + + const { headers, rawHeaders } = await got('http://example.com/') + + t.equivalent(headers, { + 'x-custom-header': 'boo!', + 'x-another-header': 'foobar', + 'x-powered-by': 'Meeee', + }) + t.equivalent(rawHeaders, [ + 'X-Powered-By', + 'Meeee', + 'X-Another-Header', + 'Hey man!', + 'X-Custom-Header', + 'boo!', + 'x-another-header', + 'foobar', + ]) + scope.done() +}) + test('reply headers as function are evaluated only once per request', async t => { let counter = 0 const scope = nock('http://example.com') From 5baf9ed5777209abba45c5e6dd6fac384ecc3cb5 Mon Sep 17 00:00:00 2001 From: "Matt R. Wilson" Date: Mon, 6 May 2019 12:14:00 -0600 Subject: [PATCH 08/19] Bugfix Interceptor.filteringPath Calling `filteringPath` on the intercept instance was broken as the transform fn set on the scope had the wrong name. Found when looking at Uncovered lines in coveralls. --- lib/interceptor.js | 2 +- tests/test_intercept.js | 18 ++++++++++++++++-- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/lib/interceptor.js b/lib/interceptor.js index 9b3c7ea5d..1474f2dc0 100644 --- a/lib/interceptor.js +++ b/lib/interceptor.js @@ -431,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 } diff --git a/tests/test_intercept.js b/tests/test_intercept.js index b697a59f3..e5dd2cc75 100644 --- a/tests/test_intercept.js +++ b/tests/test_intercept.js @@ -275,9 +275,9 @@ test('encoding', async t => { scope.done() }) -test('filter path with function', async t => { +test('filter path with function on scope', async t => { const scope = nock('http://example.test') - .filteringPath(path => '/?a=2&b=1') + .filteringPath(() => '/?a=2&b=1') .get('/?a=2&b=1') .reply(200, 'Hello World!') @@ -289,6 +289,20 @@ test('filter path with function', async t => { scope.done() }) +test('filter path with function on intercept', async t => { + const scope = nock('http://example.test') + .get('/?a=2&b=1') + .filteringPath(() => '/?a=2&b=1') + .reply(200, 'Hello World!') + + const { statusCode } = await got('http://example.test/', { + query: { a: '1', b: '2' }, + }) + + t.equal(statusCode, 200) + scope.done() +}) + test('filter path with regexp', async t => { const scope = nock('http://example.test') .filteringPath(/\d/g, '3') From 833dae2a4a1fb30bac00459318bfdc0713715066 Mon Sep 17 00:00:00 2001 From: "Matt R. Wilson" Date: Mon, 6 May 2019 12:14:32 -0600 Subject: [PATCH 09/19] Add tests to increase coverage in new error flows. --- lib/scope.js | 2 +- tests/test_delay.js | 9 +++++++++ tests/test_dynamic_mock.js | 10 ++++++++++ tests/test_repeating.js | 14 ++++++++++++++ 4 files changed, 34 insertions(+), 1 deletion(-) diff --git a/lib/scope.js b/lib/scope.js index fd7c539c5..4658281fb 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -218,7 +218,7 @@ Scope.prototype.matchHeader = function matchHeader(name, value) { } Scope.prototype.defaultReplyHeaders = function defaultReplyHeaders(headers) { - // Because there are lower-cased, res.rawHeaders can have the wrong keys. bug? + // Because these are lower-cased, res.rawHeaders can have the wrong keys. bug? this._defaultReplyHeaders = common.headersFieldNamesToLowerCase(headers) return this } diff --git a/tests/test_delay.js b/tests/test_delay.js index 92f6f438a..7f9d34830 100644 --- a/tests/test_delay.js +++ b/tests/test_delay.js @@ -274,6 +274,15 @@ test('using reply callback with delay can reply JSON', t => { ) }) +test('delay with invalid arguments', t => { + const interceptor = nock('http://example.test').get('/') + t.throws( + () => interceptor.delay('one million seconds'), + Error('Unexpected input') + ) + t.end() +}) + test('delay works with replyWithFile', t => { // Do not base new tests on this one. Write async tests using // `resolvesInAtLeast` instead. diff --git a/tests/test_dynamic_mock.js b/tests/test_dynamic_mock.js index a7f29fb8c..1b24a1b5a 100644 --- a/tests/test_dynamic_mock.js +++ b/tests/test_dynamic_mock.js @@ -53,6 +53,16 @@ test('one function returning too large an array causes an error', function(t) { }) }) +test('one function throws an error if extraneous args are provided', function(t) { + const interceptor = nock('http://example.test').get('/') + t.throws( + () => interceptor.reply(() => [200], { 'x-my-header': 'some-value' }), + Error('Invalid arguments') + ) + + t.end() +}) + test('one function returning the status code and body defines a full mock', function(t) { nock('http://example.test') .get('/def') diff --git a/tests/test_repeating.js b/tests/test_repeating.js index 78316613a..301e4b716 100644 --- a/tests/test_repeating.js +++ b/tests/test_repeating.js @@ -81,6 +81,20 @@ test('repeating response 4 times', t => { ) }) +test('times with invalid argument is ignored', t => { + nock.disableNetConnect() + + nock('http://example.test') + .get('/') + .times(0) + .reply(200, 'Hello World!') + + http.get('http://example.test', function(res) { + t.equal(200, res.statusCode, 'first request') + t.end() + }) +}) + test('isDone() must consider repeated responses', t => { const scope = nock('http://example.test') .get('/') From f6f1f0bf81d8f6d1ece03bb7c525d9493c9eb177 Mon Sep 17 00:00:00 2001 From: "Matt R. Wilson" Date: Wed, 8 May 2019 10:06:40 -0600 Subject: [PATCH 10/19] Remove statusCodeInt helper. Prefer to not cast and instead have a hard requirement for the arg to already be an int. Allow status code to not be provided and default to 200 again. --- lib/common.js | 22 ---------------------- lib/interceptor.js | 6 +++++- lib/request_overrider.js | 6 +++++- tests/test_common.js | 22 ---------------------- tests/test_content_encoding.js | 2 +- tests/test_dynamic_mock.js | 4 ++-- tests/test_reply_body.js | 18 +++++++++++------- tests/test_reply_function_sync.js | 2 +- 8 files changed, 25 insertions(+), 57 deletions(-) diff --git a/lib/common.js b/lib/common.js index 982449f32..503c9c567 100644 --- a/lib/common.js +++ b/lib/common.js @@ -377,27 +377,6 @@ 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 @@ -415,4 +394,3 @@ exports.percentDecode = percentDecode exports.matchStringOrRegexp = matchStringOrRegexp exports.formatQueryValue = formatQueryValue exports.isStream = isStream -exports.statusCodeInt = statusCodeInt diff --git a/lib/interceptor.js b/lib/interceptor.js index ac4f390fe..a1b2a4011 100644 --- a/lib/interceptor.js +++ b/lib/interceptor.js @@ -91,7 +91,11 @@ Interceptor.prototype.reply = function reply(statusCode, body, rawHeaders) { this.callback = statusCode this.fullResponseFromCallback = true } else { - this.statusCode = common.statusCodeInt(statusCode) + if (statusCode && !Number.isInteger(statusCode)) { + throw new Error(`Invalid ${typeof statusCode} value for status code`) + } + + this.statusCode = statusCode || 200 if (_.isFunction(body)) { this.callback = body body = null diff --git a/lib/request_overrider.js b/lib/request_overrider.js index cddc68629..2db9c1b88 100644 --- a/lib/request_overrider.js +++ b/lib/request_overrider.js @@ -570,7 +570,11 @@ function parseResponseFromCallbackResult(response, callbackResult) { const [status, body, headers] = callbackResult - response.statusCode = common.statusCodeInt(status) + if (!Number.isInteger(status)) { + throw new Error(`Invalid ${typeof status} value for status code`) + } + + response.statusCode = status if (headers) { debug('new headers: %j', headers) diff --git a/tests/test_common.js b/tests/test_common.js index b384e47b7..3f09e61e5 100644 --- a/tests/test_common.js +++ b/tests/test_common.js @@ -337,25 +337,3 @@ 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() -}) diff --git a/tests/test_content_encoding.js b/tests/test_content_encoding.js index 39aa85d37..fac64840f 100644 --- a/tests/test_content_encoding.js +++ b/tests/test_content_encoding.js @@ -13,7 +13,7 @@ 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', diff --git a/tests/test_dynamic_mock.js b/tests/test_dynamic_mock.js index 1b24a1b5a..449669289 100644 --- a/tests/test_dynamic_mock.js +++ b/tests/test_dynamic_mock.js @@ -114,7 +114,7 @@ test('one asynchronous function returning the status code and body defines a ful test('asynchronous function gets request headers', function(t) { nock('http://example.test') .get('/yo') - .reply(200, function(path, reqBody, cb) { + .reply(201, function(path, reqBody, cb) { t.equal(this.req.path, '/yo') t.deepEqual(this.req.headers, { 'x-my-header': 'some-value', @@ -137,7 +137,7 @@ test('asynchronous function gets request headers', function(t) { }, function(err, resp, body) { t.error(err) - t.equal(resp.statusCode, 200) + t.equal(resp.statusCode, 201) t.equal(body, 'foobar') t.end() } diff --git a/tests/test_reply_body.js b/tests/test_reply_body.js index 18239466f..7cae423f9 100644 --- a/tests/test_reply_body.js +++ b/tests/test_reply_body.js @@ -104,11 +104,15 @@ test('reply with missing body defaults to empty', async t => { scope.done() }) -test('reply with missing status code throws an error', async t => { - const interceptor = nock('http://example.test').get('/') - t.throws( - () => interceptor.reply(), - Error('Invalid undefined value for status code') - ) - t.end() + +test('reply with missing status code defaults to 200', async t => { + const scope = nock('http://example.test') + .get('/') + .reply() + + const { statusCode, body } = await got('http://example.test/') + + t.is(statusCode, 200) + t.equal(body, '') + scope.done() }) diff --git a/tests/test_reply_function_sync.js b/tests/test_reply_function_sync.js index f1b9e4a6f..5e9e5c072 100644 --- a/tests/test_reply_function_sync.js +++ b/tests/test_reply_function_sync.js @@ -171,7 +171,7 @@ test('reply function returning array with status code', async t => { test('reply function returning array with status code and string body', async t => { const scope = nock('http://example.com') .get('/') - .reply(() => ['401', 'This is a body']) + .reply(() => [401, 'This is a body']) await assertRejects(got('http://example.com/'), ({ statusCode, body }) => { t.is(statusCode, 401) From b2ad0ad424721291c14ef1320813352a6e247171 Mon Sep 17 00:00:00 2001 From: "Matt R. Wilson" Date: Wed, 8 May 2019 10:12:11 -0600 Subject: [PATCH 11/19] Allow native falsy values as body. While `false` and `null` are falsy, they are valid JSON value so they should be returned as a strings that JSON.parse would convert back to native values. Should fix #1446 --- lib/request_overrider.js | 9 +++++---- tests/test_reply_body.js | 25 +++++++++++++++++++++++++ tests/test_reply_function_sync.js | 22 ++++++++++++++++++++++ 3 files changed, 52 insertions(+), 4 deletions(-) diff --git a/lib/request_overrider.js b/lib/request_overrider.js index 2db9c1b88..2d25c4a82 100644 --- a/lib/request_overrider.js +++ b/lib/request_overrider.js @@ -394,7 +394,7 @@ function RequestOverrider(req, options, interceptors, remove, cb) { // Transform the response body if it exists (it may not exist // if we have `responseBuffers` instead) - if (!responseBuffers) { + if (responseBody !== undefined) { debug('transform the response body') if (interceptor.delayInMs) { @@ -424,7 +424,7 @@ function RequestOverrider(req, options, interceptors, remove, cb) { // TODO-coverage: Add a test. response.emit('error', err) }) - } else if (responseBody && !Buffer.isBuffer(responseBody)) { + } else if (!Buffer.isBuffer(responseBody)) { if (typeof responseBody === 'string') { responseBody = Buffer.from(responseBody) } else { @@ -432,6 +432,7 @@ function RequestOverrider(req, options, interceptors, remove, cb) { response.headers['content-type'] = 'application/json' } } + // why are strings converted to a Buffer, but JSON data is left as a string? related #1542? } interceptor.interceptionCounter++ @@ -568,7 +569,7 @@ function parseResponseFromCallbackResult(response, callbackResult) { ) } - const [status, body, headers] = callbackResult + const [status, body = '', headers] = fullReplyResult if (!Number.isInteger(status)) { throw new Error(`Invalid ${typeof status} value for status code`) @@ -592,7 +593,7 @@ function parseResponseFromCallbackResult(response, callbackResult) { debug('response.headers after: %j', response.headers) } - return body || '' + return body } module.exports = RequestOverrider diff --git a/tests/test_reply_body.js b/tests/test_reply_body.js index 7cae423f9..2790ab1ff 100644 --- a/tests/test_reply_body.js +++ b/tests/test_reply_body.js @@ -104,6 +104,31 @@ test('reply with missing body defaults to empty', async t => { scope.done() }) +// while `false` and `null` are falsy, they are valid JSON value so they should be returned as a strings +// that JSON.parse would convert back to native values +test('reply with native boolean as the body', async t => { + const scope = nock('http://example.test') + .get('/') + .reply(204, false) + + const { statusCode, body } = await got('http://example.test/') + + t.is(statusCode, 204) + t.equal(body, 'false') + scope.done() +}) + +test('reply with native null as the body', async t => { + const scope = nock('http://example.test') + .get('/') + .reply(204, null) + + const { statusCode, body } = await got('http://example.test/') + + t.is(statusCode, 204) + t.equal(body, 'null') + scope.done() +}) test('reply with missing status code defaults to 200', async t => { const scope = nock('http://example.test') diff --git a/tests/test_reply_function_sync.js b/tests/test_reply_function_sync.js index 5e9e5c072..b7d00aa67 100644 --- a/tests/test_reply_function_sync.js +++ b/tests/test_reply_function_sync.js @@ -57,6 +57,28 @@ test('reply with status code and function returning array', async t => { scope.done() }) +test('reply with status code and function returning a native boolean', async t => { + const scope = nock('http://example.test') + .get('/') + .reply(201, () => false) + + const { statusCode, body } = await got('http://example.test') + t.is(statusCode, 201) + t.equal(body, 'false') + scope.done() +}) + +test('reply with status code and function returning a native null', async t => { + const scope = nock('http://example.test') + .get('/') + .reply(201, () => null) + + const { statusCode, body } = await got('http://example.test') + t.is(statusCode, 201) + t.equal(body, 'null') + scope.done() +}) + test('reply function with string body using POST', async t => { const exampleRequestBody = 'key=val' const exampleResponseBody = 'foo' From af427833ffc6386f0cd1013941fd3f3296115d79 Mon Sep 17 00:00:00 2001 From: "Matt R. Wilson" Date: Wed, 8 May 2019 10:12:51 -0600 Subject: [PATCH 12/19] Move new tests out of dynamic_mock, use got. --- tests/test_dynamic_mock.js | 73 ------------------------------- tests/test_reply_function_sync.js | 65 ++++++++++++++++++++++++++- 2 files changed, 63 insertions(+), 75 deletions(-) diff --git a/tests/test_dynamic_mock.js b/tests/test_dynamic_mock.js index 449669289..28145bf2a 100644 --- a/tests/test_dynamic_mock.js +++ b/tests/test_dynamic_mock.js @@ -6,63 +6,6 @@ const nock = require('..') require('./cleanup_after_each')() -test('one function not returning an array causes an error', function(t) { - nock('http://example.test') - .get('/abc') - .reply(function() { - return 'ABC' - }) - - request.get('http://example.test/abc', function(err) { - t.match( - err, - Error('A single function provided to .reply MUST return an array') - ) - t.end() - }) -}) - -test('one function returning an empty array causes an error', function(t) { - nock('http://example.test') - .get('/abc') - .reply(function() { - return [] - }) - - request.get('http://example.test/abc', function(err) { - t.match(err, Error('Invalid undefined value for status code')) - t.end() - }) -}) - -test('one function returning too large an array causes an error', function(t) { - nock('http://example.test') - .get('/abc') - .reply(function() { - return ['user', 'probably', 'intended', 'this', 'to', 'be', 'JSON'] - }) - - request.get('http://example.test/abc', function(err) { - t.match( - err, - Error( - 'The array returned from the .reply callback contains too many values' - ) - ) - t.end() - }) -}) - -test('one function throws an error if extraneous args are provided', function(t) { - const interceptor = nock('http://example.test').get('/') - t.throws( - () => interceptor.reply(() => [200], { 'x-my-header': 'some-value' }), - Error('Invalid arguments') - ) - - t.end() -}) - test('one function returning the status code and body defines a full mock', function(t) { nock('http://example.test') .get('/def') @@ -78,22 +21,6 @@ test('one function returning the status code and body defines a full mock', func }) }) -test('one function returning an array can set headers', function(t) { - nock('http://example.test') - .get('/def') - .reply(function() { - return [201, 'DEF', { 'X-My-Header': 'some-value' }] - }) - - request.get('http://example.test/def', function(err, resp, body) { - t.error(err) - t.equal(resp.statusCode, 201) - t.equal(body, 'DEF') - t.deepEqual(resp.headers, { 'x-my-header': 'some-value' }) - t.end() - }) -}) - test('one asynchronous function returning the status code and body defines a full mock', function(t) { nock('http://example.test') .get('/ghi') diff --git a/tests/test_reply_function_sync.js b/tests/test_reply_function_sync.js index b7d00aa67..5b8a1151e 100644 --- a/tests/test_reply_function_sync.js +++ b/tests/test_reply_function_sync.js @@ -101,7 +101,7 @@ test('reply function with string body using POST', async t => { }) test('reply function receives the request URL and body', async t => { - t.plan(3) + t.plan(4) const exampleRequestBody = 'key=val' @@ -118,6 +118,7 @@ test('reply function receives the request URL and body', async t => { }), ({ statusCode, body }) => { t.equal(statusCode, 404) + t.equal(body, '') return true } ) @@ -178,7 +179,7 @@ test('reply with status code, function returning string body, and header object' scope.done() }) -test('reply function returning array with status code', async t => { +test('reply function returning array with only status code', async t => { const scope = nock('http://example.test') .get('/') .reply(() => [202]) @@ -248,3 +249,63 @@ test('reply function returning array with status code, string body, and headers t.deepEqual(rawHeaders, ['x-key', 'value', 'x-key-2', 'value 2']) scope.done() }) + +test('one function not returning an array causes an error', async t => { + nock('http://example.test') + .get('/abc') + .reply(function() { + return 'ABC' + }) + + await assertRejects(got('http://example.test/abc'), err => { + t.match( + err, + Error('A single function provided to .reply MUST return an array') + ) + return true + }) + t.end() +}) + +test('one function returning an empty array causes an error', async t => { + nock('http://example.test') + .get('/abc') + .reply(function() { + return [] + }) + + await assertRejects(got('http://example.test/abc'), err => { + t.match(err, Error('Invalid undefined value for status code')) + return true + }) + t.end() +}) + +test('one function returning too large an array causes an error', async t => { + nock('http://example.test') + .get('/abc') + .reply(function() { + return ['user', 'probably', 'intended', 'this', 'to', 'be', 'JSON'] + }) + + await assertRejects(got('http://example.test/abc'), err => { + t.match( + err, + Error( + 'The array returned from the .reply callback contains too many values' + ) + ) + return true + }) + t.end() +}) + +test('one function throws an error if extraneous args are provided', async t => { + const interceptor = nock('http://example.test').get('/') + t.throws( + () => interceptor.reply(() => [200], { 'x-my-header': 'some-value' }), + Error('Invalid arguments') + ) + + t.end() +}) From cb272d45072fed5baa12745eeae34c3afb9a9187 Mon Sep 17 00:00:00 2001 From: "Matt R. Wilson" Date: Wed, 8 May 2019 10:14:43 -0600 Subject: [PATCH 13/19] More native JS, less Lodash. --- lib/common.js | 2 +- lib/request_overrider.js | 14 ++++++++------ 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/lib/common.js b/lib/common.js index 503c9c567..666e92c64 100644 --- a/lib/common.js +++ b/lib/common.js @@ -245,7 +245,7 @@ const headersArrayToObject = function(rawHeaders) { const value = rawHeaders[i + 1] if (headers[key]) { - headers[key] = _.castArray(headers[key]) + headers[key] = Array.isArray(headers[key]) ? headers[key] : [headers[key]] headers[key].push(value) } else { headers[key] = value diff --git a/lib/request_overrider.js b/lib/request_overrider.js index 2d25c4a82..4ec03e311 100644 --- a/lib/request_overrider.js +++ b/lib/request_overrider.js @@ -288,7 +288,7 @@ function RequestOverrider(req, options, interceptors, remove, cb) { } // Clone headers/rawHeaders to not override them when evaluating later - response.headers = _.extend({}, interceptor.headers) + response.headers = { ...interceptor.headers } response.rawHeaders = (interceptor.rawHeaders || []).slice() debug('response.rawHeaders:', response.rawHeaders) @@ -457,9 +457,7 @@ function RequestOverrider(req, options, interceptors, remove, cb) { // Evaluate functional headers. const evaluatedHeaders = {} - Object.keys(response.headers).forEach(function(key) { - const value = response.headers[key] - + Object.entries(response.headers).forEach(function([key, value]) { if (typeof value === 'function') { response.headers[key] = evaluatedHeaders[key] = value( req, @@ -582,14 +580,18 @@ function parseResponseFromCallbackResult(response, callbackResult) { const rawHeaders = Array.isArray(headers) ? headers - : _.flatten(Object.entries(headers)) + : [].concat(...Object.entries(headers)) response.rawHeaders.push(...rawHeaders) const castHeaders = Array.isArray(headers) ? common.headersArrayToObject(headers) : common.headersFieldNamesToLowerCase(headers) - _.assign(response.headers, castHeaders) + response.headers = { + ...response.headers, + ...castHeaders, + } + debug('response.headers after: %j', response.headers) } From 13f347b35c15a2874b261c977d7d1e4ad7b32bb1 Mon Sep 17 00:00:00 2001 From: "Matt R. Wilson" Date: Wed, 8 May 2019 10:15:07 -0600 Subject: [PATCH 14/19] Bugfix? `a` is not a local var. --- lib/common.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/common.js b/lib/common.js index 666e92c64..7e9fb3a2a 100644 --- a/lib/common.js +++ b/lib/common.js @@ -371,7 +371,7 @@ function formatQueryValue(key, value, stringFormattingFn) { function isStream(obj) { return ( obj && - typeof a !== 'string' && + typeof obj !== 'string' && !Buffer.isBuffer(obj) && _.isFunction(obj.setEncoding) ) From c491360ec7daea3782f20d48ae9b890d4e98fdfc Mon Sep 17 00:00:00 2001 From: "Matt R. Wilson" Date: Wed, 8 May 2019 10:22:00 -0600 Subject: [PATCH 15/19] CR feedback. Separate two interceptor reply fns. Use different attribute names for the reply functions that return a body vs return an array for the whole response (dynamic). Rejigger RequestOverrider.end to return early based which flow is used. It would be nice if this method was split up into more pure functions to reduce the complexity. --- lib/common.js | 2 +- lib/interceptor.js | 47 +++++----- lib/request_overrider.js | 189 ++++++++++++++++++++++++--------------- 3 files changed, 140 insertions(+), 98 deletions(-) diff --git a/lib/common.js b/lib/common.js index 7e9fb3a2a..a839c68ff 100644 --- a/lib/common.js +++ b/lib/common.js @@ -324,7 +324,7 @@ function matchStringOrRegexp(target, pattern) { * @param stringFormattingFn The function used to format string values. Can * be used to encode or decode the query value. * - * @returns the formatted [key, value] pair. + * @returns *[] the formatted [key, value] pair. */ function formatQueryValue(key, value, stringFormattingFn) { // TODO-coverage: Find out what's not covered. Probably refactor code to diff --git a/lib/interceptor.js b/lib/interceptor.js index a1b2a4011..335114803 100644 --- a/lib/interceptor.js +++ b/lib/interceptor.js @@ -81,15 +81,14 @@ Interceptor.prototype.reply = function reply(statusCode, body, rawHeaders) { // 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 + // It's not very Javascript-y to throw an error for extra args to a function, 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 + this.fullReplyFunction = statusCode } else { if (statusCode && !Number.isInteger(statusCode)) { throw new Error(`Invalid ${typeof statusCode} value for status code`) @@ -97,7 +96,7 @@ Interceptor.prototype.reply = function reply(statusCode, body, rawHeaders) { this.statusCode = statusCode || 200 if (_.isFunction(body)) { - this.callback = body + this.replyFunction = body body = null } } @@ -137,27 +136,27 @@ 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 (body && !common.isContentEncoded(this.headers)) { - if ( - typeof body !== 'string' && - typeof body !== 'function' && - !Buffer.isBuffer(body) && - !common.isStream(body) - ) { - try { - body = stringify(body) - if (!this.headers) { - this.headers = {} - } - if (!this.headers['content-type']) { - this.headers['content-type'] = 'application/json' - } - if (this.scope.contentLen) { - this.headers['content-length'] = body.length - } - } catch (err) { - throw new Error('Error encoding response body into JSON') + if ( + body && + typeof body !== 'string' && + typeof body !== 'function' && + !Buffer.isBuffer(body) && + !common.isStream(body) && + !common.isContentEncoded(this.headers) + ) { + try { + body = stringify(body) + if (!this.headers) { + this.headers = {} + } + if (!this.headers['content-type']) { + this.headers['content-type'] = 'application/json' + } + if (this.scope.contentLen) { + this.headers['content-length'] = body.length } + } catch (err) { + throw new Error('Error encoding response body into JSON') } } diff --git a/lib/request_overrider.js b/lib/request_overrider.js index 4ec03e311..0ef7816be 100644 --- a/lib/request_overrider.js +++ b/lib/request_overrider.js @@ -35,7 +35,7 @@ function setHeader(request, name, value) { function setRequestHeaders(req, options, interceptor) { // If a filtered scope is being used we have to use scope's host // in the header, otherwise 'host' header won't match. - // NOTE: We use lower-case header field names throught Nock. + // NOTE: We use lower-case header field names throughout Nock. const HOST_HEADER = 'host' if (interceptor.__nock_filteredScope && interceptor.__nock_scopeHost) { if (options && options.headers) { @@ -287,89 +287,127 @@ function RequestOverrider(req, options, interceptors, remove, cb) { return } + // This will be null if we have a fullReplyFunction, + // in that case status code will be set in `parseFullReplyResult` + response.statusCode = interceptor.statusCode + // Clone headers/rawHeaders to not override them when evaluating later response.headers = { ...interceptor.headers } response.rawHeaders = (interceptor.rawHeaders || []).slice() debug('response.rawHeaders:', response.rawHeaders) - if (interceptor.callback) { - if (requestBody && common.isJSONContent(req.headers)) { - if (common.contentEncoding(req.headers, 'gzip')) { - requestBody = String(zlib.gunzipSync(Buffer.from(requestBody, 'hex'))) - } else if (common.contentEncoding(req.headers, 'deflate')) { - requestBody = String( - zlib.inflateSync(Buffer.from(requestBody, 'hex')) - ) - } + if (interceptor.replyFunction) { + const parsedRequestBody = parseJSONRequestBody(req, requestBody) - requestBody = JSON.parse(requestBody) + if (interceptor.replyFunction.length === 3) { + interceptor.replyFunction( + options.path, + parsedRequestBody, + continueWithResponseBody + ) + return } - // In case we are waiting for a callback - if (interceptor.callback.length === 3) { - return interceptor.callback( + const replyResponseBody = interceptor.replyFunction( + options.path, + parsedRequestBody + ) + continueWithResponseBody(null, replyResponseBody) + return + } + + if (interceptor.fullReplyFunction) { + const parsedRequestBody = parseJSONRequestBody(req, requestBody) + + if (interceptor.fullReplyFunction.length === 3) { + interceptor.fullReplyFunction( options.path, - requestBody, - continueWithResponseBody + parsedRequestBody, + continueWithFullResponse ) + return } - responseBody = interceptor.callback(options.path, requestBody) - } else { + const fullReplyResult = interceptor.fullReplyFunction( + options.path, + parsedRequestBody + ) + continueWithFullResponse(null, fullReplyResult) + return + } + + if ( + common.isContentEncoded(response.headers) && + !common.isStream(interceptor.body) + ) { // 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. // (otherwise decompressions after the first one fails as unzip expects to receive // buffer by buffer and not one single merged buffer) - if ( - common.isContentEncoded(response.headers) && - !common.isStream(interceptor.body) - ) { - if (interceptor.delayInMs) { - // TODO-coverage: Add a test of this error case. - emitError( - new Error( - 'Response delay is currently not supported with content-encoded responses.' - ) + + if (interceptor.delayInMs) { + // TODO-coverage: Add a test of this error case. + emitError( + new Error( + 'Response delay is currently not supported with content-encoded responses.' ) - return - } + ) + return + } - responseBuffers = _.castArray(interceptor.body).map(function(buffer) { - return Buffer.from(buffer, 'hex') - }) - } else { - responseBody = interceptor.body - - // If the request was binary then we assume that the response will be binary as well. - // In that case we send the response as a Buffer object as that's what the client will expect. - if (isBinaryRequestBodyBuffer && typeof responseBody === 'string') { - // Try to create the buffer from the interceptor's body response as hex. - try { - responseBody = Buffer.from(responseBody, 'hex') - } catch (err) { - // TODO-coverage: Add a test of this error case. - debug( - 'exception during Buffer construction from hex data:', - responseBody, - '-', - err - ) - } + const bufferData = Array.isArray(interceptor.body) + ? interceptor.body + : [interceptor.body] + responseBuffers = bufferData.map(data => Buffer.from(data, 'hex')) + continueWithResponseBody(null, undefined) + return + } - // Creating buffers does not necessarily throw errors, check for difference in size - if ( - !responseBody || - (interceptor.body.length > 0 && responseBody.length === 0) - ) { - // We fallback on constructing buffer from utf8 representation of the body. - responseBody = Buffer.from(interceptor.body, 'utf8') - } - } + // If we get to this point, the body is either a string or an + // object that will eventually be JSON stringified + responseBody = interceptor.body + + // If the request was binary then we assume that the response will be binary as well. + // In that case we send the response as a Buffer object as that's what the client will expect. + if (isBinaryRequestBodyBuffer && typeof responseBody === 'string') { + // Try to create the buffer from the interceptor's body response as hex. + try { + responseBody = Buffer.from(responseBody, 'hex') + } catch (err) { + // TODO-coverage: Add a test of this error case. + debug( + 'exception during Buffer construction from hex data:', + responseBody, + '-', + err + ) + } + + // Creating buffers does not necessarily throw errors, check for difference in size + if ( + !responseBody || + (interceptor.body.length > 0 && responseBody.length === 0) + ) { + // We fallback on constructing buffer from utf8 representation of the body. + responseBody = Buffer.from(interceptor.body, 'utf8') } } return continueWithResponseBody(null, responseBody) + function continueWithFullResponse(err, fullReplyResult) { + if (!err) { + try { + responseBody = parseFullReplyResult(response, fullReplyResult) + } catch (innerErr) { + emitError(innerErr) + return + } + } + + continueWithResponseBody(err, responseBody) + } + function continueWithResponseBody(err, responseBody) { // TODO-coverage: Try to find out when this happens and add a test. if (continued) { @@ -380,15 +418,6 @@ 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 @@ -554,14 +583,28 @@ function RequestOverrider(req, options, interceptors, remove, cb) { return req } -function parseResponseFromCallbackResult(response, callbackResult) { - debug('full response from callback result: %j', callbackResult) +function parseJSONRequestBody(req, requestBody) { + if (!requestBody || !common.isJSONContent(req.headers)) { + return requestBody + } + + if (common.contentEncoding(req.headers, 'gzip')) { + requestBody = String(zlib.gunzipSync(Buffer.from(requestBody, 'hex'))) + } else if (common.contentEncoding(req.headers, 'deflate')) { + requestBody = String(zlib.inflateSync(Buffer.from(requestBody, 'hex'))) + } + + return JSON.parse(requestBody) +} + +function parseFullReplyResult(response, fullReplyResult) { + debug('full response from callback result: %j', fullReplyResult) - if (!Array.isArray(callbackResult)) { + if (!Array.isArray(fullReplyResult)) { throw Error('A single function provided to .reply MUST return an array') } - if (callbackResult.length > 3) { + if (fullReplyResult.length > 3) { throw Error( 'The array returned from the .reply callback contains too many values' ) From 98eb51135cd8bac3751ac409a11398f914c33e47 Mon Sep 17 00:00:00 2001 From: "Matt R. Wilson" Date: Wed, 8 May 2019 10:27:41 -0600 Subject: [PATCH 16/19] Add test to cover a JSON array for the body. This is a regression test for #1208. --- tests/test_reply_body.js | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/tests/test_reply_body.js b/tests/test_reply_body.js index 2790ab1ff..93161ee0d 100644 --- a/tests/test_reply_body.js +++ b/tests/test_reply_body.js @@ -23,6 +23,21 @@ test('reply with JSON', async t => { scope.done() }) +test('reply with JSON array', async t => { + const scope = nock('http://example.test') + .get('/') + .reply(200, [{ hello: 'world' }]) + + const { statusCode, headers, body } = await got('http://example.test/') + + t.equal(statusCode, 200) + t.type(headers.date, 'undefined') + t.type(headers['content-length'], 'undefined') + t.equal(headers['content-type'], 'application/json') + t.equal(body, '[{"hello":"world"}]', 'response should match') + scope.done() +}) + test('JSON encoded replies set the content-type header', async t => { const scope = nock('http://example.test') .get('/') From 14cf47360b2582af825306e6d368dd6b338b2a44 Mon Sep 17 00:00:00 2001 From: "Matt R. Wilson" Date: Wed, 8 May 2019 10:45:05 -0600 Subject: [PATCH 17/19] Update new `times` test to use async and got. --- tests/test_repeating.js | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/tests/test_repeating.js b/tests/test_repeating.js index 301e4b716..6f19f7f2f 100644 --- a/tests/test_repeating.js +++ b/tests/test_repeating.js @@ -4,6 +4,7 @@ const http = require('http') const async = require('async') const { test } = require('tap') const nock = require('..') +const got = require('./got_client') require('./cleanup_after_each')() @@ -81,18 +82,18 @@ test('repeating response 4 times', t => { ) }) -test('times with invalid argument is ignored', t => { +test('times with invalid argument is ignored', async t => { nock.disableNetConnect() - nock('http://example.test') + const scope = nock('http://example.test') .get('/') .times(0) .reply(200, 'Hello World!') - http.get('http://example.test', function(res) { - t.equal(200, res.statusCode, 'first request') - t.end() - }) + const { statusCode } = await got('http://example.test/') + t.is(statusCode, 200) + + scope.done() }) test('isDone() must consider repeated responses', t => { From 704306488a0ba993cc781c8ea0068da5b2a63e23 Mon Sep 17 00:00:00 2001 From: Paul Melnikow Date: Sun, 19 May 2019 16:20:39 -0400 Subject: [PATCH 18/19] Add a note to the readme --- README.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/README.md b/README.md index 44b720c6f..c6d9897b8 100644 --- a/README.md +++ b/README.md @@ -360,6 +360,14 @@ const scope = nock('http://www.google.com') .reply(201, (uri, requestBody) => requestBody) ``` +In Nock 11.x it was possible to invoke `.reply()` with a status code and a +function that returns an array containing a status code and body. (The status +code from the array would take precedence over the one passed directly to +reply.) This is no longer allowed. In 12.x, either call `.reply()` with a +status code and a function that returns the body, or call it with a single +argument: a function that returns an array containing both the status code and +body. + An asynchronous function that gets an error-first callback as its last argument also works: ```js From f317877613a0264ba2548bf597d217717d8ad090 Mon Sep 17 00:00:00 2001 From: "Matt R. Wilson" Date: Mon, 20 May 2019 10:54:48 -0600 Subject: [PATCH 19/19] CR feedback updates. --- lib/interceptor.js | 3 ++- lib/request_overrider.js | 4 +++- lib/scope.js | 3 ++- tests/test_reply_body.js | 2 ++ tests/test_reply_headers.js | 1 + tests/test_stream.js | 4 ++-- 6 files changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/interceptor.js b/lib/interceptor.js index 335114803..3877f602c 100644 --- a/lib/interceptor.js +++ b/lib/interceptor.js @@ -110,7 +110,8 @@ 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? + // Because of this, this.rawHeaders gets lower-cased versions of the `rawHeaders` param. + // https://github.com/nock/nock/issues/1553 headers = common.headersFieldNamesToLowerCase(headers) headers = mixin(this.scope._defaultReplyHeaders, headers) } diff --git a/lib/request_overrider.js b/lib/request_overrider.js index 0ef7816be..8a6bb753c 100644 --- a/lib/request_overrider.js +++ b/lib/request_overrider.js @@ -300,6 +300,7 @@ function RequestOverrider(req, options, interceptors, remove, cb) { const parsedRequestBody = parseJSONRequestBody(req, requestBody) if (interceptor.replyFunction.length === 3) { + // Handle the case of an async reply function, the third parameter being the callback. interceptor.replyFunction( options.path, parsedRequestBody, @@ -461,7 +462,8 @@ function RequestOverrider(req, options, interceptors, remove, cb) { response.headers['content-type'] = 'application/json' } } - // why are strings converted to a Buffer, but JSON data is left as a string? related #1542? + // Why are strings converted to a Buffer, but JSON data is left as a string? + // Related to https://github.com/nock/nock/issues/1542 ? } interceptor.interceptionCounter++ diff --git a/lib/scope.js b/lib/scope.js index 4658281fb..02298330b 100644 --- a/lib/scope.js +++ b/lib/scope.js @@ -218,7 +218,8 @@ 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? + // Because these are lower-cased, res.rawHeaders can have the wrong keys. + // https://github.com/nock/nock/issues/1553 this._defaultReplyHeaders = common.headersFieldNamesToLowerCase(headers) return this } diff --git a/tests/test_reply_body.js b/tests/test_reply_body.js index 93161ee0d..ba006a43b 100644 --- a/tests/test_reply_body.js +++ b/tests/test_reply_body.js @@ -129,6 +129,7 @@ test('reply with native boolean as the body', async t => { const { statusCode, body } = await got('http://example.test/') t.is(statusCode, 204) + // `'false'` is json-stringified `false`. t.equal(body, 'false') scope.done() }) @@ -141,6 +142,7 @@ test('reply with native null as the body', async t => { const { statusCode, body } = await got('http://example.test/') t.is(statusCode, 204) + // `'null'` is json-stringified `null`. t.equal(body, 'null') scope.done() }) diff --git a/tests/test_reply_headers.js b/tests/test_reply_headers.js index d599d3cfb..4568f350c 100644 --- a/tests/test_reply_headers.js +++ b/tests/test_reply_headers.js @@ -70,6 +70,7 @@ test('reply header function is evaluated and the result sent in the mock respons // - the resulting order differs depending if overrides are provided to .reply directly or via a callback // - replacing values with function results isn't guaranteed to keep the correct order // - the resulting `headers` object itself is fine and these assertions pass +// https://github.com/nock/nock/issues/1553 test('reply headers and defaults', { skip: true }, async t => { const scope = nock('http://example.com') .defaultReplyHeaders({ diff --git a/tests/test_stream.js b/tests/test_stream.js index 26eb781e0..0a640d4ab 100644 --- a/tests/test_stream.js +++ b/tests/test_stream.js @@ -236,7 +236,7 @@ test( nock('http://localhost') .get('/') - .reply(200, function(path, reqBody) { + .reply(201, function(path, reqBody) { return new SimpleStream() }) @@ -245,7 +245,7 @@ test( res.setEncoding('utf8') let body = '' - t.equal(res.statusCode, 200) + t.equal(res.statusCode, 201) res.on('data', function(chunk) { body += chunk