Skip to content

Commit

Permalink
fix: resolve segfault issue caused by "node:events" import (#1885)
Browse files Browse the repository at this point in the history
Co-authored-by: Artem Zakharchenko <kettanaito@gmail.com>
  • Loading branch information
mattcosta7 and kettanaito committed Nov 24, 2023
1 parent e078e91 commit f188d77
Show file tree
Hide file tree
Showing 12 changed files with 144 additions and 76 deletions.
3 changes: 3 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,9 @@ jobs:
- name: Browser tests
run: pnpm test:browser

- name: Native tests
run: pnpm test:native

- name: Upload test artifacts
if: always()
uses: actions/upload-artifact@v3
Expand Down
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,10 @@
"build": "pnpm clean && cross-env NODE_ENV=production tsup && pnpm patch:dts",
"patch:dts": "node \"./config/scripts/patch-ts.js\"",
"check:exports": "node \"./config/scripts/validate-esm.js\"",
"test": "pnpm test:unit && pnpm test:node && pnpm test:browser",
"test": "pnpm test:unit && pnpm test:node && pnpm test:browser && pnpm test:native",
"test:unit": "vitest",
"test:node": "vitest run --config=./test/node/vitest.config.ts",
"test:native": "vitest run --config=./test/native/vitest.config.ts",
"test:browser": "playwright test -c ./test/browser/playwright.config.ts",
"test:modules:node": "vitest run --config=./test/modules/node/vitest.config.ts",
"test:modules:browser": "playwright test -c ./test/modules/browser/playwright.config.ts",
Expand Down
2 changes: 1 addition & 1 deletion src/core/SetupApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export abstract class SetupApi<EventsMap extends EventMap> extends Disposable {
invariant(
!Array.isArray(handler),
devUtils.formatMessage(
'Failed to construct "%s" given an Array of request handlers. Make sure you spread the request handlers when calling the respective setup function.',
'Failed to apply given request handlers: invalid input. Did you forget to spread the request handlers Array?',
),
this.constructor.name,
)
Expand Down
75 changes: 9 additions & 66 deletions src/node/SetupServerApi.ts
Original file line number Diff line number Diff line change
@@ -1,19 +1,18 @@
import { invariant } from 'outvariant'
import {
BatchInterceptor,
HttpRequestEventMap,
Interceptor,
InterceptorReadyState,
} from '@mswjs/interceptors'
import { invariant } from 'outvariant'
import { SetupApi } from '~/core/SetupApi'
import { RequestHandler } from '~/core/handlers/RequestHandler'
import { LifeCycleEventsMap, SharedOptions } from '~/core/sharedOptions'
import { RequiredDeep } from '~/core/typeUtils'
import { mergeRight } from '~/core/utils/internal/mergeRight'
import { handleRequest } from '~/core/utils/handleRequest'
import { devUtils } from '~/core/utils/internal/devUtils'
import { SetupServer, SetupServerInternalContext } from './glossary'
import { isNodeExceptionLike } from './utils/isNodeExceptionLike'
import { mergeRight } from '~/core/utils/internal/mergeRight'
import { SetupServer } from './glossary'

const DEFAULT_LISTEN_OPTIONS: RequiredDeep<SharedOptions> = {
onUnhandledRequest: 'warn',
Expand All @@ -23,7 +22,6 @@ export class SetupServerApi
extends SetupApi<LifeCycleEventsMap>
implements SetupServer
{
private context: SetupServerInternalContext
protected readonly interceptor: BatchInterceptor<
Array<Interceptor<HttpRequestEventMap>>,
HttpRequestEventMap
Expand All @@ -38,7 +36,6 @@ export class SetupServerApi
) {
super(...handlers)

this.context = this.createContext()
this.interceptor = new BatchInterceptor({
name: 'setup-server',
interceptors: interceptors.map((Interceptor) => new Interceptor()),
Expand All @@ -48,22 +45,12 @@ export class SetupServerApi
this.init()
}

private createContext(): SetupServerInternalContext {
return {
get nodeEvents() {
return import('node:events')
.then((events) => events)
.catch(() => undefined)
},
}
}

/**
* Subscribe to all requests that are using the interceptor object
*/
private init(): void {
this.interceptor.on('request', async ({ request, requestId }) => {
await this.setRequestAbortSignalMaxListeners(request)
this.onRequest(request)

const response = await handleRequest(
request,
Expand Down Expand Up @@ -126,54 +113,10 @@ export class SetupServerApi
this.dispose()
}

/**
* Bump the maximum number of event listeners on the
* request's "AbortSignal". This prepares the request
* for each request handler cloning it at least once.
* Note that cloning a request automatically appends a
* new "abort" event listener to the parent request's
* "AbortController" so if the parent aborts, all the
* clones are automatically aborted.
*/
private async setRequestAbortSignalMaxListeners(
request: Request,
): Promise<void> {
const events = await this.context.nodeEvents

/**
* @note React Native doesn't support "node:events".
*/
if (typeof events === 'undefined') {
return
}

const { setMaxListeners, defaultMaxListeners } = events

if (typeof setMaxListeners !== 'function') {
return
}

try {
setMaxListeners(
Math.max(defaultMaxListeners, this.currentHandlers.length),
request.signal,
)
} catch (error: unknown) {
/**
* @note Mock environments (JSDOM, ...) are not able to implement an internal
* "kIsNodeEventTarget" Symbol that Node.js uses to identify Node.js `EventTarget`s.
* `setMaxListeners` throws an error for non-Node.js `EventTarget`s.
* At the same time, mock environments are also not able to implement the
* internal "events.maxEventTargetListenersWarned" Symbol, which results in
* "MaxListenersExceededWarning" not being printed by Node.js for those anyway.
* The main reason for using `setMaxListeners` is to suppress these warnings in Node.js,
* which won't be printed anyway if `setMaxListeners` fails.
*/
if (
!(isNodeExceptionLike(error) && error.code === 'ERR_INVALID_ARG_TYPE')
) {
throw error
}
}
protected onRequest(request: Request): void {
// Do nothing here.
// Subclasses may hook into the request being intercepted
// to provide additional functionality (e.g. setting the max
// event listener count on request's "AbortController" in Node.js).
}
}
4 changes: 0 additions & 4 deletions src/node/glossary.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,3 @@ export interface SetupServer {
*/
events: LifeCycleEventEmitter<LifeCycleEventsMap>
}

export type SetupServerInternalContext = {
get nodeEvents(): Promise<typeof import('node:events') | undefined>
}
40 changes: 39 additions & 1 deletion src/node/setupServer.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,47 @@
import { ClientRequestInterceptor } from '@mswjs/interceptors/ClientRequest'
import { XMLHttpRequestInterceptor } from '@mswjs/interceptors/XMLHttpRequest'
import { FetchInterceptor } from '@mswjs/interceptors/fetch'
import { defaultMaxListeners, setMaxListeners } from 'node:events'
import { RequestHandler } from '~/core/handlers/RequestHandler'
import { SetupServerApi as BaseSetupServerApi } from './SetupServerApi'
import { SetupServer } from './glossary'
import { SetupServerApi } from './SetupServerApi'
import { isNodeExceptionLike } from './utils/isNodeExceptionLike'

class SetupServerApi extends BaseSetupServerApi implements SetupServer {
/**
* Bump the maximum number of event listeners on the
* request's "AbortSignal". This prepares the request
* for each request handler cloning it at least once.
* Note that cloning a request automatically appends a
* new "abort" event listener to the parent request's
* "AbortController" so if the parent aborts, all the
* clones are automatically aborted.
*/
protected override onRequest(request: Request): void {
try {
setMaxListeners(
Math.max(defaultMaxListeners, this.currentHandlers.length),
request.signal,
)
} catch (error: unknown) {
/**
* @note Mock environments (JSDOM, ...) are not able to implement an internal
* "kIsNodeEventTarget" Symbol that Node.js uses to identify Node.js `EventTarget`s.
* `setMaxListeners` throws an error for non-Node.js `EventTarget`s.
* At the same time, mock environments are also not able to implement the
* internal "events.maxEventTargetListenersWarned" Symbol, which results in
* "MaxListenersExceededWarning" not being printed by Node.js for those anyway.
* The main reason for using `setMaxListeners` is to suppress these warnings in Node.js,
* which won't be printed anyway if `setMaxListeners` fails.
*/
if (
!(isNodeExceptionLike(error) && error.code === 'ERR_INVALID_ARG_TYPE')
) {
throw error
}
}
}
}

/**
* Sets up a requests interception in Node.js with the given request handlers.
Expand Down
2 changes: 1 addition & 1 deletion test/browser/msw-api/setup-worker/input-validation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ test('throws an error given an Array of request handlers to "setupWorker"', asyn
expect(exceptions).toEqual(
expect.arrayContaining([
expect.stringContaining(
'[MSW] Failed to construct "SetupWorkerApi" given an Array of request handlers. Make sure you spread the request handlers when calling the respective setup function.',
'[MSW] Failed to apply given request handlers: invalid input. Did you forget to spread the request handlers Array?',
),
]),
)
Expand Down
22 changes: 22 additions & 0 deletions test/native/node-events.native.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/**
* @see https://github.com/mswjs/msw/pull/1858
* @see https://github.com/mswjs/msw/issues/1868
*/
import { setupServer } from 'msw/native'

test('calls "setupServer" without errors in React Native', async () => {
/**
* @note Asserting that mocking works is not possible with
* the current testing setup. We force Vitest to alias "msw"
* imports to their ESM build, which is a hack.
*
* We need Vitest to load the ESM build here in order to
* use "vi.mock()" to mock the "node:events" imports to throw.
* Vitest doesn't support module mocking in CommonJS.
*
* But aliasing the build isn't enough for it to function.
* The root-level package.json is still CJS, which, I suspects,
* resolves any subsequent in-build imports to their CJS counterparts.
*/
expect(() => setupServer()).not.toThrow()
})
22 changes: 22 additions & 0 deletions test/native/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
{
"extends": "../../tsconfig.base.json",
"compilerOptions": {
"target": "esnext",
"module": "esnext",
"baseUrl": ".",
"paths": {
"msw": ["../.."],
"msw/*": ["../../*"]
},
"noEmit": true,
"declaration": false,
"resolveJsonModule": true,
// Support default imports for modules that have no default exports.
// This way "http" imports stay "import http from 'http'".
// Using wildcard there breaks request interception since it
// encapsulates the values under the ".default" key.
"allowSyntheticDefaultImports": true,
"types": ["node", "vitest/globals"]
},
"include": ["../../global.d.ts", "./**/*.test.ts"]
}
28 changes: 28 additions & 0 deletions test/native/vitest.config.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import * as path from 'node:path'
import { defineConfig } from 'vitest/config'

const LIB_DIR = path.resolve(__dirname, '../../lib')

export default defineConfig({
test: {
root: './test/native',
include: ['**/*.native.test.ts'],
globals: true,
setupFiles: ['./vitest.setup.ts'],
alias: {
/**
* @note Force Vitest load ESM targets of MSW.
* If we run ESM in tests, we can use "vi.mock()" to
* emulate certain standard Node.js modules missing
* (like "node:events") in React Native.
*
* Vitest won't pick up the ESM targets because
* the root-level "package.json" is not "module".
*/
'msw/node': path.resolve(LIB_DIR, 'node/index.mjs'),
'msw/native': path.resolve(LIB_DIR, 'native/index.mjs'),
'msw/browser': path.resolve(LIB_DIR, 'browser/index.mjs'),
msw: path.resolve(LIB_DIR, 'core/index.mjs'),
},
},
})
15 changes: 15 additions & 0 deletions test/native/vitest.setup.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import { vi } from 'vitest'

/**
* @note The list of standard Node.js modules missing
* in the React Native runtime.
*/
const reactNativeMissingModules = ['events', 'node:events']

reactNativeMissingModules.forEach((moduleName) => {

This comment has been minimized.

Copy link
@mattcosta7

mattcosta7 Nov 24, 2023

Author Contributor

🚀

vi.doMock(moduleName, () => {
throw new Error(
`Failed to import module "${moduleName}": it does not exist in React Native. This likely means MSW tries to import something too optimistically in that environment.`,
)
})
})
4 changes: 2 additions & 2 deletions test/node/msw-api/setup-server/input-validation.node.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { HttpResponse, http } from 'msw'
import { setupServer } from 'msw/node'

test('throws an error given an Array of request handlers to setupServer', () => {
test('throws an error given an Array of request handlers to "setupServer"', () => {
const createServer = () => {
// @ts-expect-error intentionally invalid parameters for setupServer to force it to throw
return setupServer([
Expand All @@ -12,6 +12,6 @@ test('throws an error given an Array of request handlers to setupServer', () =>
}

expect(createServer).toThrow(
`[MSW] Failed to construct "SetupServerApi" given an Array of request handlers. Make sure you spread the request handlers when calling the respective setup function.`,
`[MSW] Failed to apply given request handlers: invalid input. Did you forget to spread the request handlers Array?`,
)
})

0 comments on commit f188d77

Please sign in to comment.