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

Use @mswjs/interceptors for mocking - WIP #2517

Draft
wants to merge 23 commits into
base: main
Choose a base branch
from
Draft
44 changes: 1 addition & 43 deletions lib/common.js
Expand Up @@ -67,51 +67,9 @@ let requestOverrides = {}
* - callback - the callback of the issued request
*/
function overrideRequests(newRequest) {
debug('overriding requests')
;['http', 'https'].forEach(function (proto) {
debug('- overriding request for', proto)

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

if (requestOverrides[moduleName]) {
throw new Error(
`Module's request already overridden for ${moduleName} protocol.`
)
}

// Store the properties of the overridden request so that it can be restored later on.
requestOverrides[moduleName] = {
module,
request: overriddenRequest,
get: overriddenGet,
}
// https://nodejs.org/api/http.html#http_http_request_url_options_callback
module.request = function (input, options, callback) {
return newRequest(proto, overriddenRequest.bind(module), [
input,
options,
callback,
])
}
// https://nodejs.org/api/http.html#http_http_get_options_callback
module.get = function (input, options, callback) {
const req = newRequest(proto, overriddenGet.bind(module), [
input,
options,
callback,
])
req.end()
return req
}

debug('- overridden request for', proto)
})

}

/**
Expand Down
60 changes: 60 additions & 0 deletions lib/create_response.js
@@ -0,0 +1,60 @@
const { IncomingHttpHeaders, IncomingMessage } = require('http')

Check failure on line 1 in lib/create_response.js

View workflow job for this annotation

GitHub Actions / Lint JavaScript

Use the global form of 'use strict'

Check failure on line 1 in lib/create_response.js

View workflow job for this annotation

GitHub Actions / Lint JavaScript

'IncomingHttpHeaders' is assigned a value but never used

Check failure on line 1 in lib/create_response.js

View workflow job for this annotation

GitHub Actions / Lint JavaScript

'IncomingMessage' is assigned a value but never used

/**
* Creates a Fetch API `Response` instance from the given
* `http.IncomingMessage` instance.
* copied from: https://github.com/mswjs/interceptors/blob/04152ed914f8041272b6e92ed374216b8177e1b2/src/interceptors/ClientRequest/utils/createResponse.ts#L8
*/

/**
* @param {IncomingMessage} message
*/
function createResponse(message) {
const readable = new ReadableStream({

Check failure on line 13 in lib/create_response.js

View workflow job for this annotation

GitHub Actions / Lint JavaScript

'ReadableStream' is not defined

Choose a reason for hiding this comment

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

Looks like the version of Node Nock uses doesn't have the ReadableStream global. I'd double-check if this isn't the linter's problem as the first measure.

Choose a reason for hiding this comment

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

Sometimes the linter doesn't play well with relatively newly added globals. This can be modified in the linter's globals key, if talking about ESLint.

Copy link
Contributor Author

@mikicho mikicho Sep 18, 2023

Choose a reason for hiding this comment

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

Thanks!
I think we can fix this after we will solve the got error. Most of Nock's tests are based on it and I'm eager to solve it and run all tests to find cases that we haven't covered yet.

Choose a reason for hiding this comment

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

I will try to look into the got issue this week based on my availability. Right now, the best next step is to reliably reproduce it in an automated test. I've already written one (see https://github.com/mswjs/interceptors/pull/432?notification_referrer_id=NT_kwDOAOSmz7M3NzYwMzQ3MDA1OjE0OTg0OTEx&notifications_query=is%3Aunread#issuecomment-1724139617) but you've provided some input that I need to reflect in the test.

Copy link

Choose a reason for hiding this comment

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

I've fixed the got issue, would appreciate a quick review of the pull request!

start(controller) {
message.on('data', (chunk) => controller.enqueue(chunk))
message.on('end', () => controller.close())

/**
* @todo Should also listen to the "error" on the message
* and forward it to the controller. Otherwise the stream
* will pend indefinitely.
*/
},
})

return new Response(readable, {
status: message.statusCode,
statusText: message.statusMessage,
headers: createHeadersFromIncomingHttpHeaders(message.headers),
})
}

/**
* @param {IncomingHttpHeaders} httpHeaders
*/
function createHeadersFromIncomingHttpHeaders(httpHeaders) {
const headers = new Headers()

for (const headerName in httpHeaders) {
const headerValues = httpHeaders[headerName]

if (typeof headerValues === 'undefined') {
continue
}

if (Array.isArray(headerValues)) {
headerValues.forEach((headerValue) => {
headers.append(headerName, headerValue)
})

continue
}

headers.set(headerName, headerValues)
}

return headers
}

module.exports = { createResponse }
125 changes: 68 additions & 57 deletions lib/intercept.js
Expand Up @@ -10,6 +10,11 @@
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')
const { once } = require('events')

Check failure on line 16 in lib/intercept.js

View workflow job for this annotation

GitHub Actions / Lint JavaScript

'once' is assigned a value but never used
const { createResponse } = require('./create_response')

/**
* @name NetConnectNotAllowedError
Expand Down Expand Up @@ -365,75 +370,81 @@
return [].concat(...interceptorScopes().map(scope => scope.activeMocks()))
}

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,
proto: url.protocol.slice(0, -1),
headers: Object.fromEntries(fetchRequest.headers.entries())
};

return new http.ClientRequest(options);
}

function activate() {
if (originalClientRequest) {
throw new Error('Nock already active')
}

overrideClientRequest()
const interceptor = new BatchInterceptor({
name: 'nock-interceptor',
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
)

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

common.overrideRequests(function (proto, overriddenRequest, args) {
// NOTE: overriddenRequest is already bound to its module.

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'
)
}

// 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

const interceptors = interceptorsFor(options)
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')
}

if (isOn() && interceptors) {
const matches = interceptors.some(interceptor =>
interceptor.matchOrigin(options)
)
const allowUnmocked = interceptors.some(
interceptor => interceptor.options.allowUnmocked
)
const nockRequest = convertFetchRequestToClientRequest(request);
nockRequest.on('response', nockResponse => {
const response = createResponse(nockResponse)
nockResponse.on('end', () => {
request.respondWith(response)
resolve()
})
})

if (!matches && allowUnmocked) {
let req
if (proto === 'https') {
const { ClientRequest } = http
http.ClientRequest = originalClientRequest
req = overriddenRequest(options, callback)
http.ClientRequest = ClientRequest
nockRequest.end()
} else {
globalEmitter.emit('no match', options)
if (isOff() || isEnabledForNetConnect(options)) {
// TODO: implement unmocked
return overriddenRequest(options, callback)

Check failure on line 441 in lib/intercept.js

View workflow job for this annotation

GitHub Actions / Lint JavaScript

'overriddenRequest' is not defined
} else {
req = overriddenRequest(options, callback)
const error = new NetConnectNotAllowedError(options.host, options.path)
return new ErroringClientRequest(error)
}
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)
} else {
const error = new NetConnectNotAllowedError(options.host, options.path)
return new ErroringClientRequest(error)
}
}
})
})
}

Expand Down