From 26fc08fcd331a8903f5c8966127c5a4ebe9c2c8f Mon Sep 17 00:00:00 2001 From: "Matt R. Wilson" Date: Thu, 1 Aug 2019 13:47:45 -0600 Subject: [PATCH] Async Reply functions (always emit errors) (#1596) Functions passed to `Interceptor.reply`, either to respond with an array of `[status, body, headers]` or with just the body, can now be async/promise-returning functions. BREAKING CHANGE: uncaught errors thrown inside of user provided reply functions, whether async or not, will no longer be caught, and will no longer generate a successful response with a status code of 500. Instead, the error will be emitted by the request just like any other unhandled error during the request processing. --- README.md | 3 +- lib/request_overrider.js | 75 +++++++------------- tests/test_reply_function_async.js | 110 ++++++++++++++++++----------- 3 files changed, 94 insertions(+), 94 deletions(-) diff --git a/README.md b/README.md index 1138a43ad..33c86182b 100644 --- a/README.md +++ b/README.md @@ -395,7 +395,8 @@ const scope = nock('http://www.google.com') }) ``` -> Note: When using a callback, if you call back with an error as the first argument, that error will be sent in the response body, with a 500 HTTP response status code. +In Nock 11 and later, if an error is passed to the callback, Nock will rethrow it as a programmer error. +In Nock 10 and earlier, the error was sent in the response body, with a 500 HTTP response status code. You can also return the status code and body using just one function: diff --git a/lib/request_overrider.js b/lib/request_overrider.js index 48e91f364..5aaa3dc72 100644 --- a/lib/request_overrider.js +++ b/lib/request_overrider.js @@ -6,6 +6,7 @@ const { IncomingMessage, ClientRequest } = require('http') const _ = require('lodash') const propagate = require('propagate') const timers = require('timers') +const util = require('util') const zlib = require('zlib') const common = require('./common') @@ -213,8 +214,6 @@ function RequestOverrider(req, options, interceptors, remove) { ended = true let requestBody, responseBody, responseBuffers - let continued = false - // When request body is a binary buffer we internally use in its hexadecimal representation. const requestBodyBuffer = Buffer.concat(requestBodyBuffers) const isBinaryRequestBodyBuffer = common.isUtf8Representable( @@ -307,41 +306,31 @@ function RequestOverrider(req, options, interceptors, remove) { if (interceptor.replyFunction) { const parsedRequestBody = parseJSONRequestBody(req, requestBody) - if (interceptor.replyFunction.length === 3) { + let fn = interceptor.replyFunction + if (fn.length === 3) { // Handle the case of an async reply function, the third parameter being the callback. - interceptor.replyFunction( - options.path, - parsedRequestBody, - continueWithResponseBody - ) - return + fn = util.promisify(fn) } - const replyResponseBody = interceptor.replyFunction( - options.path, - parsedRequestBody - ) - continueWithResponseBody(null, replyResponseBody) + // At this point `fn` is either a synchronous function or a promise-returning function; + // wrapping in `Promise.resolve` makes it into a promise either way. + Promise.resolve(fn.call(interceptor, options.path, parsedRequestBody)) + .then(continueWithResponseBody) + .catch(emitError) return } if (interceptor.fullReplyFunction) { const parsedRequestBody = parseJSONRequestBody(req, requestBody) - if (interceptor.fullReplyFunction.length === 3) { - interceptor.fullReplyFunction( - options.path, - parsedRequestBody, - continueWithFullResponse - ) - return + let fn = interceptor.fullReplyFunction + if (fn.length === 3) { + fn = util.promisify(fn) } - const fullReplyResult = interceptor.fullReplyFunction( - options.path, - parsedRequestBody - ) - continueWithFullResponse(null, fullReplyResult) + Promise.resolve(fn.call(interceptor, options.path, parsedRequestBody)) + .then(continueWithFullResponse) + .catch(emitError) return } @@ -367,7 +356,7 @@ function RequestOverrider(req, options, interceptors, remove) { ? interceptor.body : [interceptor.body] responseBuffers = bufferData.map(data => Buffer.from(data, 'hex')) - continueWithResponseBody(null, undefined) + continueWithResponseBody() return } @@ -391,36 +380,22 @@ function RequestOverrider(req, options, interceptors, remove) { } } - return continueWithResponseBody(null, responseBody) - - function continueWithFullResponse(err, fullReplyResult) { - if (!err) { - try { - responseBody = parseFullReplyResult(response, fullReplyResult) - } catch (innerErr) { - emitError(innerErr) - return - } - } - - continueWithResponseBody(err, responseBody) - } + return continueWithResponseBody(responseBody) - function continueWithResponseBody(err, responseBody) { - if (continued) { - // subsequent calls from reply callbacks are ignored + function continueWithFullResponse(fullReplyResult) { + try { + responseBody = parseFullReplyResult(response, fullReplyResult) + } catch (innerErr) { + emitError(innerErr) return } - continued = true - if (err) { - response.statusCode = 500 - responseBody = err.stack - } + continueWithResponseBody(responseBody) + } + function continueWithResponseBody(responseBody) { // Transform the response body if it exists (it may not exist // if we have `responseBuffers` instead) - if (responseBody !== undefined) { debug('transform the response body') diff --git a/tests/test_reply_function_async.js b/tests/test_reply_function_async.js index 88999c074..05f68afda 100644 --- a/tests/test_reply_function_async.js +++ b/tests/test_reply_function_async.js @@ -4,8 +4,6 @@ // callback with the response body or an array containing the status code and // optional response body and headers. -const assertRejects = require('assert-rejects') -const http = require('http') const { test } = require('tap') const nock = require('..') const got = require('./got_client') @@ -51,60 +49,30 @@ test('reply takes a callback for status code', async t => { scope.done() }) -test('reply should throw on error on the callback', t => { - let dataCalled = false - - const scope = nock('http://example.test') +test('reply should throw on error on the callback', async t => { + nock('http://example.test') .get('/') .reply(500, (path, requestBody, callback) => callback(new Error('Database failed')) ) - // TODO When this request is converted to `got`, it causes the request not - // to match. - const req = http.request( - { - host: 'example.test', - path: '/', - port: 80, - }, - res => { - t.equal(res.statusCode, 500, 'Status code is 500') - - res.on('data', data => { - dataCalled = true - t.ok(data instanceof Buffer, 'data should be buffer') - t.ok( - data.toString().indexOf('Error: Database failed') === 0, - 'response should match' - ) - }) - - res.on('end', () => { - t.ok(dataCalled, 'data handler was called') - scope.done() - t.end() - }) - } - ) - - req.end() + await t.rejects(got('http://example.test'), { + name: 'RequestError', + message: 'Database failed', + }) }) test('an error passed to the callback propagates when [err, fullResponseArray] is expected', async t => { - const scope = nock('http://example.test') + nock('http://example.test') .get('/') .reply((path, requestBody, callback) => { callback(Error('boom')) }) - await assertRejects(got('http://example.test/'), ({ statusCode, body }) => { - t.is(statusCode, 500) - t.matches(body, 'Error: boom') - return true + await t.rejects(got('http://example.test'), { + name: 'RequestError', + message: 'boom', }) - - scope.done() }) test('subsequent calls to the reply callback are ignored', async t => { @@ -115,7 +83,7 @@ test('subsequent calls to the reply callback are ignored', async t => { .reply(201, (path, requestBody, callback) => { callback(null, 'one') callback(null, 'two') - callback(null, 'three') + callback(new Error('three')) t.pass() }) @@ -125,3 +93,59 @@ test('subsequent calls to the reply callback are ignored', async t => { t.is(statusCode, 201) t.equal(body, 'one') }) + +test('reply can take a status code with an 2-arg async function, and passes it the correct arguments', async t => { + const scope = nock('http://example.com') + .post('/foo') + .reply(201, async (path, requestBody) => { + t.equal(path, '/foo') + t.equal(requestBody, 'request-body') + return 'response-body' + }) + + const response = await got.post('http://example.com/foo', { + body: 'request-body', + }) + + t.equal(response.statusCode, 201) + t.equal(response.body, 'response-body') + scope.done() +}) + +test('reply can take a status code with a 0-arg async function, and passes it the correct arguments', async t => { + const scope = nock('http://example.com') + .get('/') + .reply(async () => [201, 'Hello World!']) + + const response = await got('http://example.com/') + + t.equal(response.statusCode, 201) + t.equal(response.body, 'Hello World!') + scope.done() +}) + +test('when reply is called with a status code and an async function that throws, it propagates the error', async t => { + nock('http://example.test') + .get('/') + .reply(201, async () => { + throw Error('oh no!') + }) + + await t.rejects(got('http://example.test'), { + name: 'RequestError', + message: 'oh no!', + }) +}) + +test('when reply is called with an async function that throws, it propagates the error', async t => { + nock('http://example.test') + .get('/') + .reply(async () => { + throw Error('oh no!') + }) + + await t.rejects(got('http://example.test'), { + name: 'RequestError', + message: 'oh no!', + }) +})