-
-
Notifications
You must be signed in to change notification settings - Fork 736
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
Use @mswjs/interceptors for mocking - WIP #2517
base: main
Are you sure you want to change the base?
Changes from 3 commits
576b296
94b9b90
ef59cf6
d7d17bd
855914e
51dd0c6
21cf9c2
091d876
79f5162
2028d4e
bba8bdb
5e521a1
e1c3eb4
e89acec
cbd31cb
bc4327a
7df18f2
53e3d2a
ca9e616
2c0e4f1
bcbdd04
ab8037d
e7a6309
dac5a26
36f6778
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,9 @@ | |
const http = require('http') | ||
const debug = require('debug')('nock.intercept') | ||
const globalEmitter = require('./global_emitter') | ||
const { BatchInterceptor } = require('@mswjs/interceptors') | ||
const { FetchInterceptor } = require('@mswjs/interceptors/fetch') | ||
const { default: nodeInterceptors } = require('@mswjs/interceptors/presets/node') | ||
|
||
/** | ||
* @name NetConnectNotAllowedError | ||
|
@@ -365,75 +368,105 @@ | |
return [].concat(...interceptorScopes().map(scope => scope.activeMocks())) | ||
} | ||
|
||
function activate() { | ||
if (originalClientRequest) { | ||
throw new Error('Nock already active') | ||
} | ||
function getRequestOptionsFromFetchRequest(fetchRequest) { | ||
const requestOptions = { | ||
method: fetchRequest.method, | ||
headers: {}, | ||
}; | ||
|
||
overrideClientRequest() | ||
fetchRequest.headers.forEach((value, key) => { | ||
requestOptions.headers[key] = value; | ||
}); | ||
|
||
// ----- Overriding http.request and https.request: | ||
const url = new URL(fetchRequest.url); | ||
|
||
common.overrideRequests(function (proto, overriddenRequest, args) { | ||
// NOTE: overriddenRequest is already bound to its module. | ||
const options = { | ||
...requestOptions, | ||
hostname: url.hostname, | ||
port: url.port, | ||
path: url.pathname + url.search, | ||
proto: url.protocol === 'https:' ? 'https' : 'http', | ||
}; | ||
|
||
const { options, callback } = common.normalizeClientRequestArgs(...args) | ||
|
||
if (Object.keys(options).length === 0) { | ||
// As weird as it is, it's possible to call `http.request` without | ||
// options, and it makes a request to localhost or somesuch. We should | ||
// support it too, for parity. However it doesn't work today, and fixing | ||
// it seems low priority. Giving an explicit error is nicer than | ||
// crashing with a weird stack trace. `new ClientRequest()`, nock's | ||
// other client-facing entry point, makes a similar check. | ||
// https://github.com/nock/nock/pull/1386 | ||
// https://github.com/nock/nock/pull/1440 | ||
throw Error( | ||
'Making a request with empty `options` is not supported in Nock' | ||
) | ||
} | ||
return options; | ||
} | ||
|
||
// The option per the docs is `protocol`. Its unclear if this line is meant to override that and is misspelled or if | ||
// the intend is to explicitly keep track of which module was called using a separate name. | ||
// Either way, `proto` is used as the source of truth from here on out. | ||
options.proto = proto | ||
function convertFetchRequestToClientRequest(fetchRequest) { | ||
const url = new URL(fetchRequest.url); | ||
const options = { | ||
method: fetchRequest.method, | ||
host: url.hostname, | ||
port: url.port || (url.protocol === 'https:' ? 443 : 80), | ||
path: url.pathname + url.search, | ||
headers: fetchRequest.headers, | ||
proto: url.protocol.slice(0, -1), | ||
headers: Object.fromEntries(fetchRequest.headers.entries()) | ||
}; | ||
|
||
const clientRequest = new http.ClientRequest(options); | ||
// Note: You won't have access to the request body data from the Fetch Request | ||
|
||
return clientRequest; | ||
} | ||
|
||
const interceptors = interceptorsFor(options) | ||
function activate() { | ||
if (originalClientRequest) { | ||
throw new Error('Nock already active') | ||
} | ||
|
||
if (isOn() && interceptors) { | ||
const matches = interceptors.some(interceptor => | ||
interceptor.matchOrigin(options) | ||
) | ||
const allowUnmocked = interceptors.some( | ||
interceptor => interceptor.options.allowUnmocked | ||
) | ||
overrideClientRequest() | ||
const interceptor = new BatchInterceptor({ | ||
name: 'my-interceptor', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Giving this a meaningful name may improve the logging output, afaik. Maybe |
||
interceptors: [...nodeInterceptors, new FetchInterceptor()], | ||
mikicho marked this conversation as resolved.
Show resolved
Hide resolved
|
||
}) | ||
interceptor.apply(); | ||
interceptor.on('request', function ({ request, requestId }) { | ||
return new Promise((resolve) => { | ||
const { options, callback } = common.normalizeClientRequestArgs(request.url) | ||
options.proto = options.protocol.slice(0, -1) | ||
const interceptors = interceptorsFor(options) | ||
if (isOn() && interceptors) { | ||
const matches = interceptors.some(interceptor => | ||
interceptor.matchOrigin(options) | ||
) | ||
const allowUnmocked = interceptors.some( | ||
interceptor => interceptor.options.allowUnmocked | ||
) | ||
|
||
if (!matches && allowUnmocked) { | ||
let req | ||
if (proto === 'https') { | ||
const { ClientRequest } = http | ||
http.ClientRequest = originalClientRequest | ||
req = overriddenRequest(options, callback) | ||
http.ClientRequest = ClientRequest | ||
} else { | ||
req = overriddenRequest(options, callback) | ||
if (!matches && allowUnmocked) { | ||
// TODO: implement unmocked | ||
// let req | ||
// if (proto === 'https') { | ||
// const { ClientRequest } = http | ||
// http.ClientRequest = originalClientRequest | ||
// req = overriddenRequest(options, callback) | ||
// http.ClientRequest = ClientRequest | ||
// } else { | ||
// req = overriddenRequest(options, callback) | ||
// } | ||
// globalEmitter.emit('no match', req) | ||
// return req | ||
throw new Error('TODO') | ||
} | ||
globalEmitter.emit('no match', req) | ||
return req | ||
} | ||
|
||
// NOTE: Since we already overrode the http.ClientRequest we are in fact constructing | ||
// our own OverriddenClientRequest. | ||
return new http.ClientRequest(options, callback) | ||
} else { | ||
globalEmitter.emit('no match', options) | ||
if (isOff() || isEnabledForNetConnect(options)) { | ||
return overriddenRequest(options, callback) | ||
const req = convertFetchRequestToClientRequest(request); | ||
req.on('response', response => { | ||
request.respondWith(new Response('test', { status: 200 })) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: we'd want to read the response stream to the As I mentioned in the issue, let me know if exporting this existing utility from Interceptors would help here. I think it would. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you have an idea how we can wait for the nockResponse
Thanks!! For now, I copied the file. We can update this later. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In this case, we can rely on the internal const stream = new Readable()
const fetchResponse = new Response(stream, {
status: response.statusCode,
statusText: response.statusMessage,
headers: response.headers // pseudo-code
})
response.on('data', (chunk) => stream.write(chunk))
response.on('end', () => {
stream.finish()
request.respndWith(fetchResponse)
}) This is a gist. If you want the actual implementation example, take a look at this function. |
||
resolve() | ||
}) | ||
|
||
req.end() | ||
} else { | ||
const error = new NetConnectNotAllowedError(options.host, options.path) | ||
return new ErroringClientRequest(error) | ||
globalEmitter.emit('no match', options) | ||
if (isOff() || isEnabledForNetConnect(options)) { | ||
// TODO: implement unmocked | ||
return overriddenRequest(options, callback) | ||
} else { | ||
const error = new NetConnectNotAllowedError(options.host, options.path) | ||
return new ErroringClientRequest(error) | ||
} | ||
} | ||
} | ||
}) | ||
}) | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think Node expects
OutgoingHeaders
here, which is not compatible with theHeaders
instance. You should be fine doing this:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great catch! 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just be cautious about the compilation target here. I'm not sure what's the support range is for Nock, but
.fromEntries()
and.entries()
methods may not exist on super old versions of ECMAScript and Node.