Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NockBack] Record "unmatched" requests only. #1235

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
11 changes: 5 additions & 6 deletions lib/back.js
Expand Up @@ -163,7 +163,6 @@ var record = {
recorder.clear();
nock.cleanAll();
nock.activate();
nock.disableNetConnect();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t know why that was here before, but this change seems unrelated to your changes?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's actually related, otherwise you have to manually enable it in order for the requests to go through and be seen by the recorder.

The record mode should not disableNetConnect by default. For that behavior the other modes can be used.

},


Expand All @@ -172,15 +171,12 @@ var record = {
throw new Error('no fs');
}
var context = load(fixture, options);

if( !context.isLoaded ) {
recorder.record(_.assign({
dont_print: true,
output_objects: true
}, options && options.recorder));

context.isRecording = true;
}

return context;
},
Expand All @@ -194,11 +190,14 @@ var record = {
outputs = options.afterRecord(outputs);
}

outputs = JSON.stringify(outputs, null, 4);
debug('recorder outputs:', outputs);

mkdirp.sync(path.dirname(fixture));
fs.writeFileSync(fixture, outputs);
var existingOutputs = []
if (fs.existsSync(fixture)) {
existingOutputs = JSON.parse(fs.readFileSync(fixture))
}
fs.writeFileSync(fixture, JSON.stringify(existingOutputs.concat(outputs), null, 4));
}
}

Expand Down
62 changes: 32 additions & 30 deletions lib/common.js
Expand Up @@ -110,41 +110,43 @@ var requestOverride = [];
* - options - the options of the issued request
* - callback - the callback of the issued request
*/
var overrideRequests = function(newRequest) {
debug('overriding requests');
var secondLayer

['http', 'https'].forEach(function(proto) {
var overrideRequests = function (newRequest) {

['http', 'https'].forEach(function (proto) {
debug('- overriding request for', proto);

var moduleName = proto, // 1 to 1 match of protocol and module is fortunate :)
module = {
http: require('http'),
https: require('https')
}[moduleName],
overriddenRequest = module.request,
overriddenGet = module.get;

if(requestOverride[moduleName]) {
throw new Error('Module\'s request already overridden for ' + moduleName + ' protocol.');
}

module = {
http: require('http'),
https: require('https')
}[moduleName],
overriddenRequest = module.request,
overriddenGet = module.get;

if (requestOverride[moduleName]) {
debug('set recorder as second layer')
secondLayer = newRequest
} else {
// Store the properties of the overridden request so that it can be restored later on.
requestOverride[moduleName] = {
module: module,
request: overriddenRequest,
get: overriddenGet
};

module.request = function(options, callback) {
// debug('request options:', options);
return newRequest(proto, overriddenRequest.bind(module), options, callback);
};

if (semver.satisfies(process.version, '>=8')) {
module.get = function(options, callback) {
var req = newRequest(proto, overriddenRequest.bind(module), options, callback);
req.end();
return req;
requestOverride[moduleName] = {
module: module,
request: overriddenRequest,
get: overriddenGet
};

module.request = function (options, callback) {
// debug('request options:', options);
return newRequest(proto, overriddenRequest.bind(module), options, callback, secondLayer);
};

if (semver.satisfies(process.version, '>=8')) {
module.get = function (options, callback) {
var req = newRequest(proto, overriddenRequest.bind(module), options, callback, secondLayer);
req.end();
return req;
}
}
}

Expand Down
6 changes: 5 additions & 1 deletion lib/intercept.js
Expand Up @@ -351,7 +351,7 @@ function activate() {

// ----- Overriding http.request and https.request:

common.overrideRequests(function(proto, overriddenRequest, options, callback) {
common.overrideRequests(function(proto, overriddenRequest, options, callback, secondLayer) {
// NOTE: overriddenRequest is already bound to its module.
var req,
res;
Expand Down Expand Up @@ -402,7 +402,11 @@ function activate() {
} else {
globalEmitter.emit('no match', options);
if (isOff() || isEnabledForNetConnect(options)) {
if (secondLayer) {
return secondLayer(proto, overriddenRequest, options, callback)
} else {
return overriddenRequest(options, callback);
}
} else {
var error = new NetConnectNotAllowedError(options.host, options.path);
return new ErroringClientRequest(error);
Expand Down
4 changes: 2 additions & 2 deletions lib/recorder.js
Expand Up @@ -230,9 +230,9 @@ function record(rec_options) {
// we restore any requests that may have been overridden by other parts of nock (e.g. intercept)
// NOTE: This is hacky as hell but it keeps the backward compatibility *and* allows correct
// behavior in the face of other modules also overriding ClientRequest.
common.restoreOverriddenRequests();
// common.restoreOverriddenRequests();
// We restore ClientRequest as it messes with recording of modules that also override ClientRequest (e.g. xhr2)
intercept.restoreOverriddenClientRequest();
// intercept.restoreOverriddenClientRequest();

// We override the requests so that we can save information on them before executing.
common.overrideRequests(function(proto, overriddenRequest, options, callback) {
Expand Down