Skip to content

Commit a952d9b

Browse files
mastermattmastermatt
authored
fix(recorder): allow recording req headers when not outputting objects (#1617)
* test: add coverage for logging recorded objects There was no test that `output_objects: true`, without also setting `dont_print: true` so this flow was not covered. * fix: allow recording req headers when not outputting objects When exploring coverage gaps a few things were uncovered: - OutgoingMessage._headers has been deprecated in favor of `getHeaders()`. https://nodejs.org/api/deprecations.html#deprecations_dep0066_outgoingmessage_prototype_headers_outgoingmessage_prototype_headernames - IncommingMessage.rawHeaders is never falsy so there isn't a need to fall back to `headers` - There was a bug that kept request headers from being added to `matchHeader` lines in non-object outputs. `enable_reqheaders_recording` was only having an affect if `output_objects` was also true, but headers were never being added despite that because it was looking for headers under the wrong attribute on the request. A test was added to cover the flow of wanting the `matchHeader` as the other flow was already covered.
1 parent 8c582ab commit a952d9b

File tree

4 files changed

+112
-55
lines changed

4 files changed

+112
-55
lines changed

‎lib/recorder.js‎

Lines changed: 39 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,5 @@
11
'use strict'
22

3-
// This file contains some coverage ignores marked TODO-coverage. Before
4-
// refactoring within this file, these lines should be cleaned up.
5-
// https://github.com/nock/nock/issues/1607
6-
73
const debug = require('debug')('nock.recorder')
84
const _ = require('lodash')
95
const qs = require('qs')
@@ -69,7 +65,8 @@ function generateRequestAndResponseObject(
6965
bodyChunks,
7066
options,
7167
res,
72-
dataChunks
68+
dataChunks,
69+
reqheaders
7370
) {
7471
const response = getBodyFromChunks(dataChunks, res.headers)
7572
options.path = req.path
@@ -81,8 +78,11 @@ function generateRequestAndResponseObject(
8178
body: getBodyFromChunks(bodyChunks).body,
8279
status: res.statusCode,
8380
response: response.body,
84-
rawHeaders: res.rawHeaders || res.headers,
85-
reqheaders: req._headers,
81+
rawHeaders: res.rawHeaders,
82+
}
83+
84+
if (reqheaders) {
85+
nockDef.reqheaders = reqheaders
8686
}
8787

8888
if (response.isBinary) {
@@ -92,15 +92,22 @@ function generateRequestAndResponseObject(
9292
return nockDef
9393
}
9494

95-
function generateRequestAndResponse(req, bodyChunks, options, res, dataChunks) {
95+
function generateRequestAndResponse(
96+
req,
97+
bodyChunks,
98+
options,
99+
res,
100+
dataChunks,
101+
reqheaders
102+
) {
96103
const requestBody = getBodyFromChunks(bodyChunks).body
97104
const responseBody = getBodyFromChunks(dataChunks, res.headers).body
98105

99106
// Remove any query params from options.path so they can be added in the query() function
100107
let { path } = options
101-
let queryIndex = 0
108+
const queryIndex = req.path.indexOf('?')
102109
let queryObj = {}
103-
if ((queryIndex = req.path.indexOf('?')) !== -1) {
110+
if (queryIndex !== -1) {
104111
// Remove the query from the path
105112
path = path.substring(0, queryIndex)
106113

@@ -135,16 +142,12 @@ function generateRequestAndResponse(req, bodyChunks, options, res, dataChunks) {
135142
ret.push(JSON.stringify(requestBody))
136143
}
137144
ret.push(')\n')
138-
// TODO-coverage: Try to add coverage of this case.
139-
/* istanbul ignore if */
140-
if (req.headers) {
141-
for (const k in req.headers) {
142-
ret.push(
143-
` .matchHeader(${JSON.stringify(k)}, ${JSON.stringify(
144-
req.headers[k]
145-
)})\n`
146-
)
147-
}
145+
146+
reqheaders = reqheaders || {}
147+
for (const [fieldName, fieldValue] of Object.entries(reqheaders)) {
148+
const safeName = JSON.stringify(fieldName)
149+
const safeValue = JSON.stringify(fieldValue)
150+
ret.push(` .matchHeader(${safeName}, ${safeValue})\n`)
148151
}
149152

150153
if (queryIndex !== -1) {
@@ -157,15 +160,8 @@ function generateRequestAndResponse(req, bodyChunks, options, res, dataChunks) {
157160
ret.push(res.statusCode.toString())
158161
ret.push(', ')
159162
ret.push(JSON.stringify(responseBody))
160-
/* istanbul ignore else */
161-
if (res.rawHeaders) {
162-
ret.push(', ')
163-
ret.push(inspect(res.rawHeaders))
164-
} else if (res.headers) {
165-
// TODO-coverage: Try to add coverage of this case.
166-
ret.push(', ')
167-
ret.push(inspect(res.headers))
168-
}
163+
ret.push(', ')
164+
ret.push(inspect(res.rawHeaders))
169165
ret.push(');\n')
170166

171167
return ret.join('')
@@ -240,34 +236,33 @@ function record(recOptions) {
240236
debug(thisRecordingId, proto, 'intercepted request ended')
241237

242238
let out
239+
let reqheaders
240+
241+
// Ignore request headers completely unless it was explicitly enabled by the user (see README)
242+
if (enableReqHeadersRecording) {
243+
// We never record user-agent headers as they are worse than useless -
244+
// they actually make testing more difficult without providing any benefit (see README)
245+
reqheaders = req.getHeaders()
246+
common.deleteHeadersField(reqheaders, 'user-agent')
247+
}
248+
243249
if (outputObjects) {
244250
out = generateRequestAndResponseObject(
245251
req,
246252
bodyChunks,
247253
options,
248254
res,
249-
dataChunks
255+
dataChunks,
256+
reqheaders
250257
)
251-
/* istanbul ignore else */
252-
// TODO-coverage: The `else` case is missing coverage. Maybe look
253-
// into the enable_reqheaders_recording option.
254-
if (out.reqheaders) {
255-
// We never record user-agent headers as they are worse than useless -
256-
// they actually make testing more difficult without providing any benefit (see README)
257-
common.deleteHeadersField(out.reqheaders, 'user-agent')
258-
259-
// Remove request headers completely unless it was explicitly enabled by the user (see README)
260-
if (!enableReqHeadersRecording) {
261-
delete out.reqheaders
262-
}
263-
}
264258
} else {
265259
out = generateRequestAndResponse(
266260
req,
267261
bodyChunks,
268262
options,
269263
res,
270-
dataChunks
264+
dataChunks,
265+
reqheaders
271266
)
272267
}
273268

@@ -290,10 +285,7 @@ function record(recOptions) {
290285

291286
if (!dontPrint) {
292287
if (useSeparator) {
293-
/* istanbul ignore if */
294288
if (typeof out !== 'string') {
295-
// TODO-coverage: This is missing coverage. Could this be
296-
// connected to the `output_object` option?
297289
out = JSON.stringify(out, null, 2)
298290
}
299291
logging(SEPARATOR + out + SEPARATOR)

‎tests/test_define.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -249,7 +249,7 @@ test('define() uses reqheaders', t => {
249249
t.equal(res.statusCode, 200)
250250

251251
res.once('end', () => {
252-
t.equivalent(res.req._headers, reqheaders)
252+
t.equivalent(res.req.getHeaders(), reqheaders)
253253
t.end()
254254
})
255255
// Streams start in 'paused' mode and must be started.

‎tests/test_intercept.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -946,7 +946,7 @@ test('get correct filtering with scope and request headers filtering', t => {
946946
}
947947
)
948948

949-
t.equivalent(req._headers, { host: requestHeaders.host })
949+
t.equivalent(req.getHeaders(), { host: requestHeaders.host })
950950
})
951951

952952
test('different subdomain with reply callback and filtering scope', async t => {

‎tests/test_recorder.js‎

Lines changed: 71 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,40 @@ test('records objects', t => {
126126
})
127127
})
128128

129+
test('logs recorded objects', async t => {
130+
t.plan(3)
131+
132+
nock.restore()
133+
nock.recorder.clear()
134+
t.equal(nock.recorder.play().length, 0)
135+
136+
const server = http.createServer((request, response) => {
137+
t.pass('server received a request')
138+
response.writeHead(200)
139+
response.end()
140+
})
141+
t.once('end', () => server.close())
142+
await new Promise(resolve => server.listen(resolve))
143+
const { port } = server.address()
144+
145+
const logging = log => {
146+
t.true(
147+
log.startsWith(
148+
'\n<<<<<<-- cut here -->>>>>>\n{\n "scope": "http://localhost:'
149+
)
150+
)
151+
}
152+
153+
nock.recorder.rec({
154+
logging,
155+
output_objects: true,
156+
})
157+
158+
await got.post(`http://localhost:${port}`)
159+
160+
nock.restore()
161+
})
162+
129163
test('records objects and correctly stores JSON object in body', async t => {
130164
nock.restore()
131165
nock.recorder.clear()
@@ -553,7 +587,7 @@ test('records https correctly', t => {
553587
})
554588
})
555589

556-
test('records request headers correctly', t => {
590+
test('records request headers correctly as an object', t => {
557591
const server = http.createServer((request, response) => response.end())
558592
t.once('end', () => server.close())
559593

@@ -598,6 +632,37 @@ test('records request headers correctly', t => {
598632
})
599633
})
600634

635+
test('records request headers correctly when not outputting objects', async t => {
636+
t.plan(5)
637+
638+
nock.restore()
639+
nock.recorder.clear()
640+
t.equal(nock.recorder.play().length, 0)
641+
642+
const server = http.createServer((request, response) => {
643+
t.pass('server received a request')
644+
response.writeHead(200)
645+
response.end()
646+
})
647+
t.once('end', () => server.close())
648+
await new Promise(resolve => server.listen(resolve))
649+
const { port } = server.address()
650+
651+
nock.recorder.rec({
652+
dont_print: true,
653+
enable_reqheaders_recording: true,
654+
})
655+
656+
await got.post(`http://localhost:${port}`, { headers: { 'X-Foo': 'bar' } })
657+
658+
nock.restore()
659+
660+
const recorded = nock.recorder.play()
661+
t.equal(recorded.length, 1)
662+
t.type(recorded[0], 'string')
663+
t.true(recorded[0].includes(' .matchHeader("x-foo", "bar")'))
664+
})
665+
601666
test('records and replays gzipped nocks correctly', t => {
602667
const exampleText = '<html><body>example</body></html>'
603668

@@ -907,7 +972,7 @@ test('includes query parameters from superagent', t => {
907972
superagent
908973
.get(`http://localhost:${server.address().port}`)
909974
.query({ q: 'test search' })
910-
.end(res => {
975+
.end(() => {
911976
nock.restore()
912977
const ret = nock.recorder.play()
913978
t.true(ret.length >= 1)
@@ -917,7 +982,7 @@ test('includes query parameters from superagent', t => {
917982
})
918983
})
919984

920-
test('encodes the query parameters when not outputing objects', t => {
985+
test('encodes the query parameters when not outputting objects', t => {
921986
const server = http.createServer((request, response) => {
922987
response.writeHead(200)
923988
response.end()
@@ -937,7 +1002,7 @@ test('encodes the query parameters when not outputing objects', t => {
9371002
superagent
9381003
.get(`http://localhost:${server.address().port}`)
9391004
.query({ q: 'test search++' })
940-
.end(res => {
1005+
.end(() => {
9411006
nock.restore()
9421007
const recording = nock.recorder.play()
9431008
t.true(recording.length >= 1)
@@ -1183,7 +1248,7 @@ test('records and replays binary response correctly', t => {
11831248
'47494638396101000100800000000000ffffff21f90401000000002c000000000100010000020144003b'
11841249
const transparentGifBuffer = Buffer.from(transparentGifHex, 'hex')
11851250

1186-
// start server that always respondes with transparent gif at available port
1251+
// start server that always responds with transparent gif at available port
11871252
const server = http.createServer((request, response) => {
11881253
response.writeHead(201, {
11891254
'Content-Type': 'image/gif',
@@ -1220,7 +1285,7 @@ test('records and replays binary response correctly', t => {
12201285

12211286
const recordedFixtures = nock.recorder.play()
12221287

1223-
// stope server, stope recording, start intercepting
1288+
// stop server, stop recording, start intercepting
12241289
server.close(error => {
12251290
t.error(error)
12261291

0 commit comments

Comments
 (0)