From 6abdc70bce201d3008752bff662f4e5da2b0de9b Mon Sep 17 00:00:00 2001 From: Remi Date: Sun, 25 Oct 2020 11:45:41 +0100 Subject: [PATCH 01/20] feat(jest-worker): detect infinite loops --- packages/jest-haste-map/src/index.ts | 58 +++++++++----- .../jest-worker/src/base/BaseWorkerPool.ts | 3 +- packages/jest-worker/src/index.ts | 11 +-- packages/jest-worker/src/types.ts | 7 +- .../src/workers/ChildProcessWorker.ts | 76 ++++++++++++++++++- .../jest-worker/src/workers/processChild.ts | 8 ++ 6 files changed, 137 insertions(+), 26 deletions(-) diff --git a/packages/jest-haste-map/src/index.ts b/packages/jest-haste-map/src/index.ts index 5969c12df484..35945f4a495f 100644 --- a/packages/jest-haste-map/src/index.ts +++ b/packages/jest-haste-map/src/index.ts @@ -50,6 +50,8 @@ import {getSha1, worker} from './worker'; // understand `require`. const {version: VERSION} = require('../package.json'); +import inspector = require('inspector'); + type Options = { cacheDirectory?: string; computeDependencies?: boolean; @@ -543,14 +545,16 @@ export default class HasteMap extends EventEmitter { if (this._options.retainAllFiles && filePath.includes(NODE_MODULES)) { if (computeSha1) { return this._getWorker(workerOptions) - .getSha1({ - computeDependencies: this._options.computeDependencies, - computeSha1, - dependencyExtractor: this._options.dependencyExtractor, - filePath, - hasteImplModulePath: this._options.hasteImplModulePath, - rootDir, - }) + .then(worker => + worker.getSha1({ + computeDependencies: this._options.computeDependencies, + computeSha1, + dependencyExtractor: this._options.dependencyExtractor, + filePath, + hasteImplModulePath: this._options.hasteImplModulePath, + rootDir, + }), + ) .then(workerReply, workerError); } @@ -619,14 +623,16 @@ export default class HasteMap extends EventEmitter { } return this._getWorker(workerOptions) - .worker({ - computeDependencies: this._options.computeDependencies, - computeSha1, - dependencyExtractor: this._options.dependencyExtractor, - filePath, - hasteImplModulePath: this._options.hasteImplModulePath, - rootDir, - }) + .then(worker => + worker.worker({ + computeDependencies: this._options.computeDependencies, + computeSha1, + dependencyExtractor: this._options.dependencyExtractor, + filePath, + hasteImplModulePath: this._options.hasteImplModulePath, + rootDir, + }), + ) .then(workerReply, workerError); } @@ -708,17 +714,33 @@ export default class HasteMap extends EventEmitter { serializer.writeFileSync(this._cachePath, hasteMap); } + private async setUpInspector() { + // Open V8 Inspector + await inspector.open(); + + const inspectorUrl = inspector.url(); + let session; + if (inspectorUrl !== undefined) { + session = new inspector.Session(); + await session.connect(); + await session.post('Debugger.enable'); + } + return {session}; + } + /** * Creates workers or parses files and extracts metadata in-process. */ - private _getWorker(options?: {forceInBand: boolean}): WorkerInterface { + private async _getWorker(options?: {forceInBand: boolean}): Promise { if (!this._worker) { if ((options && options.forceInBand) || this._options.maxWorkers <= 1) { this._worker = {getSha1, worker}; } else { - // @ts-expect-error: assignment of a worker with custom properties. + const {session} = await this.setUpInspector(); + // @ts-ignore: assignment of a worker with custom properties. this._worker = new Worker(require.resolve('./worker'), { exposedMethods: ['getSha1', 'worker'], + inspector: session, maxRetries: 3, numWorkers: this._options.maxWorkers, }) as WorkerInterface; diff --git a/packages/jest-worker/src/base/BaseWorkerPool.ts b/packages/jest-worker/src/base/BaseWorkerPool.ts index 4d51517b4a33..6d65913b90df 100644 --- a/packages/jest-worker/src/base/BaseWorkerPool.ts +++ b/packages/jest-worker/src/base/BaseWorkerPool.ts @@ -39,11 +39,12 @@ export default class BaseWorkerPool { const stdout = mergeStream(); const stderr = mergeStream(); - const {forkOptions, maxRetries, resourceLimits, setupArgs} = options; + const {forkOptions, maxRetries, setupArgs, resourceLimits, inspector} = options; for (let i = 0; i < options.numWorkers; i++) { const workerOptions: WorkerOptions = { forkOptions, + inspector, maxRetries, resourceLimits, setupArgs, diff --git a/packages/jest-worker/src/index.ts b/packages/jest-worker/src/index.ts index 50be5683dfba..f5db5541343e 100644 --- a/packages/jest-worker/src/index.ts +++ b/packages/jest-worker/src/index.ts @@ -81,12 +81,13 @@ export class Worker { this._ending = false; const workerPoolOptions: WorkerPoolOptions = { - enableWorkerThreads: this._options.enableWorkerThreads ?? false, - forkOptions: this._options.forkOptions ?? {}, - maxRetries: this._options.maxRetries ?? 3, - numWorkers: this._options.numWorkers ?? Math.max(cpus().length - 1, 1), + enableWorkerThreads: this._options.enableWorkerThreads || false, + forkOptions: this._options.forkOptions || {}, + inspector: this._options.inspector, + maxRetries: this._options.maxRetries || 3, + numWorkers: this._options.numWorkers || Math.max(cpus().length - 1, 1), resourceLimits: this._options.resourceLimits ?? {}, - setupArgs: this._options.setupArgs ?? [], + setupArgs: this._options.setupArgs || [], }; if (this._options.WorkerPool) { diff --git a/packages/jest-worker/src/types.ts b/packages/jest-worker/src/types.ts index 7211d3d93993..f7a2089ca708 100644 --- a/packages/jest-worker/src/types.ts +++ b/packages/jest-worker/src/types.ts @@ -28,10 +28,12 @@ export const PARENT_MESSAGE_OK: 0 = 0; export const PARENT_MESSAGE_CLIENT_ERROR: 1 = 1; export const PARENT_MESSAGE_SETUP_ERROR: 2 = 2; export const PARENT_MESSAGE_CUSTOM: 3 = 3; +export const PARENT_MESSAGE_HEARTBEAT: 4 = 4; export type PARENT_MESSAGE_ERROR = | typeof PARENT_MESSAGE_CLIENT_ERROR - | typeof PARENT_MESSAGE_SETUP_ERROR; + | typeof PARENT_MESSAGE_SETUP_ERROR + | typeof PARENT_MESSAGE_HEARTBEAT; export interface WorkerPoolInterface { getStderr(): NodeJS.ReadableStream; @@ -100,6 +102,7 @@ export type FarmOptions = { resourceLimits?: ResourceLimits; setupArgs?: Array; maxRetries?: number; + inspector?: any; numWorkers?: number; taskQueue?: TaskQueue; WorkerPool?: ( @@ -115,6 +118,7 @@ export type WorkerPoolOptions = { resourceLimits: ResourceLimits; maxRetries: number; numWorkers: number; + inspector?: any; enableWorkerThreads: boolean; }; @@ -122,6 +126,7 @@ export type WorkerOptions = { forkOptions: ForkOptions; resourceLimits: ResourceLimits; setupArgs: Array; + inspector?: any; maxRetries: number; workerId: number; workerPath: string; diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index ba47862cadfe..0f0c861f7431 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -8,6 +8,7 @@ import {ChildProcess, fork} from 'child_process'; import {PassThrough} from 'stream'; import mergeStream = require('merge-stream'); +import {clearInterval} from 'timers'; import {stdout as stdoutSupportsColor} from 'supports-color'; import { CHILD_MESSAGE_INITIALIZE, @@ -17,6 +18,7 @@ import { OnStart, PARENT_MESSAGE_CLIENT_ERROR, PARENT_MESSAGE_CUSTOM, + PARENT_MESSAGE_HEARTBEAT, PARENT_MESSAGE_OK, PARENT_MESSAGE_SETUP_ERROR, ParentMessage, @@ -65,6 +67,8 @@ export default class ChildProcessWorker implements WorkerInterface { private _exitPromise: Promise; private _resolveExitPromise!: () => void; + private heartbeatTimeout: any; + constructor(options: WorkerOptions) { this._options = options; @@ -126,6 +130,8 @@ export default class ChildProcessWorker implements WorkerInterface { this._options.setupArgs, ]); + this.monitorHeartbeat(() => this.monitorHeartbeatError()); + this._child = child; this._retries++; @@ -146,6 +152,12 @@ export default class ChildProcessWorker implements WorkerInterface { } } + monitorHeartbeat(onExceeded: () => any) { + this.heartbeatTimeout = setTimeout(() => { + onExceeded(); + }, 7000); + } + private _shutdown() { // End the temporary streams so the merged streams end too if (this._fakeStream) { @@ -156,6 +168,61 @@ export default class ChildProcessWorker implements WorkerInterface { this._resolveExitPromise(); } + // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types + async monitorHeartbeatError() { + let error; + + if (this._options.inspector !== undefined) { + error = new Error( + `Test worker was unresponsive for 10 seconds. There was an inspector connected so we were unable to capture stack frames before it was terminated.`, + ); + this._options.inspector.on('Debugger.paused', (message: any) => { + const callFrames = message.params.callFrames.slice(0, 20); + for (const callFrame of callFrames) { + const loc = callFrame['location']; + + const url = callFrame['url']; + const lineNumber = loc['lineNumber']; + const columnNumber = loc['columnNumber']; + + const name = callFrame['scopeChain'][0].name; + + // TODO: process error with stack trace + console.log({url, lineNumber, columnNumber, name}); + } + }); + + await new Promise((resolve, reject) => { + this._options.inspector.post( + 'Debugger.pause', + (err: any, params: any) => { + if (err === null) { + resolve(params); + } else { + reject(err); + } + }, + ); + }); + + this._options.inspector.disconnect(); + this._options.inspector.close(); + } else { + error = new Error( + `Test worker was unresponsive for 10 seconds. There was no inspector connected so we were unable to capture stack frames before it was terminated.`, + ); + } + // @ts-ignore: adding custom properties to errors. + error.type = 1; + error.stack = 'test stack'; + + if (this.heartbeatTimeout !== undefined) { + clearTimeout(this.heartbeatTimeout); + } + + this._onProcessEnd(error, null); + } + private _onMessage(response: ParentMessage) { // TODO: Add appropriate type check let error: any; @@ -163,6 +230,7 @@ export default class ChildProcessWorker implements WorkerInterface { switch (response[0]) { case PARENT_MESSAGE_OK: this._onProcessEnd(null, response[1]); + clearInterval(this.heartbeatTimeout); break; case PARENT_MESSAGE_CLIENT_ERROR: @@ -182,7 +250,7 @@ export default class ChildProcessWorker implements WorkerInterface { error[key] = extra[key]; } } - + clearInterval(this.heartbeatTimeout); this._onProcessEnd(error, null); break; @@ -193,11 +261,17 @@ export default class ChildProcessWorker implements WorkerInterface { error.stack = response[3]; this._onProcessEnd(error, null); + clearInterval(this.heartbeatTimeout); + break; + + case PARENT_MESSAGE_HEARTBEAT: + this.monitorHeartbeat(() => this.monitorHeartbeatError()); break; case PARENT_MESSAGE_CUSTOM: this._onCustomMessage(response[1]); break; default: + clearInterval(this.heartbeatTimeout); throw new TypeError('Unexpected response from worker: ' + response[0]); } } diff --git a/packages/jest-worker/src/workers/processChild.ts b/packages/jest-worker/src/workers/processChild.ts index 64d29e19e132..c078d415771a 100644 --- a/packages/jest-worker/src/workers/processChild.ts +++ b/packages/jest-worker/src/workers/processChild.ts @@ -13,6 +13,7 @@ import { ChildMessageInitialize, PARENT_MESSAGE_CLIENT_ERROR, PARENT_MESSAGE_ERROR, + PARENT_MESSAGE_HEARTBEAT, PARENT_MESSAGE_OK, PARENT_MESSAGE_SETUP_ERROR, } from '../types'; @@ -59,6 +60,12 @@ const messageListener: NodeJS.MessageListener = request => { }; process.on('message', messageListener); +function sendParentMessageHeartbeat() { + if (process && process.send) { + process.send([PARENT_MESSAGE_HEARTBEAT]); + } +} + function reportSuccess(result: unknown) { if (!process || !process.send) { throw new Error('Child can only be used on a forked process'); @@ -149,6 +156,7 @@ function execFunction( onError: (error: Error) => void, ): void { let result; + sendParentMessageHeartbeat(); try { result = fn.apply(ctx, args); From f3feded92853966c6f24110f2ef128c58b24f797 Mon Sep 17 00:00:00 2001 From: Remi Date: Mon, 26 Oct 2020 14:33:58 +0100 Subject: [PATCH 02/20] feat(jest-worker): inspector initialization + workerHeartbeatTimeout props --- packages/jest-haste-map/src/index.ts | 57 ++++++---------- packages/jest-worker/README.md | 4 ++ .../jest-worker/src/base/BaseWorkerPool.ts | 10 ++- packages/jest-worker/src/index.ts | 24 ++++++- packages/jest-worker/src/types.ts | 9 ++- .../src/workers/ChildProcessWorker.ts | 66 +++++++++---------- 6 files changed, 93 insertions(+), 77 deletions(-) diff --git a/packages/jest-haste-map/src/index.ts b/packages/jest-haste-map/src/index.ts index 35945f4a495f..238e6032ec56 100644 --- a/packages/jest-haste-map/src/index.ts +++ b/packages/jest-haste-map/src/index.ts @@ -51,6 +51,7 @@ import {getSha1, worker} from './worker'; const {version: VERSION} = require('../package.json'); import inspector = require('inspector'); +type HType = typeof H; type Options = { cacheDirectory?: string; @@ -545,16 +546,14 @@ export default class HasteMap extends EventEmitter { if (this._options.retainAllFiles && filePath.includes(NODE_MODULES)) { if (computeSha1) { return this._getWorker(workerOptions) - .then(worker => - worker.getSha1({ - computeDependencies: this._options.computeDependencies, - computeSha1, - dependencyExtractor: this._options.dependencyExtractor, - filePath, - hasteImplModulePath: this._options.hasteImplModulePath, - rootDir, - }), - ) + .getSha1({ + computeDependencies: this._options.computeDependencies, + computeSha1, + dependencyExtractor: this._options.dependencyExtractor, + filePath, + hasteImplModulePath: this._options.hasteImplModulePath, + rootDir, + }) .then(workerReply, workerError); } @@ -623,16 +622,14 @@ export default class HasteMap extends EventEmitter { } return this._getWorker(workerOptions) - .then(worker => - worker.worker({ - computeDependencies: this._options.computeDependencies, - computeSha1, - dependencyExtractor: this._options.dependencyExtractor, - filePath, - hasteImplModulePath: this._options.hasteImplModulePath, - rootDir, - }), - ) + .worker({ + computeDependencies: this._options.computeDependencies, + computeSha1, + dependencyExtractor: this._options.dependencyExtractor, + filePath, + hasteImplModulePath: this._options.hasteImplModulePath, + rootDir, + }) .then(workerReply, workerError); } @@ -714,33 +711,17 @@ export default class HasteMap extends EventEmitter { serializer.writeFileSync(this._cachePath, hasteMap); } - private async setUpInspector() { - // Open V8 Inspector - await inspector.open(); - - const inspectorUrl = inspector.url(); - let session; - if (inspectorUrl !== undefined) { - session = new inspector.Session(); - await session.connect(); - await session.post('Debugger.enable'); - } - return {session}; - } - /** * Creates workers or parses files and extracts metadata in-process. */ - private async _getWorker(options?: {forceInBand: boolean}): Promise { + private _getWorker(options?: {forceInBand: boolean}): WorkerInterface { if (!this._worker) { if ((options && options.forceInBand) || this._options.maxWorkers <= 1) { this._worker = {getSha1, worker}; } else { - const {session} = await this.setUpInspector(); - // @ts-ignore: assignment of a worker with custom properties. + // @ts-expect-error: assignment of a worker with custom properties. this._worker = new Worker(require.resolve('./worker'), { exposedMethods: ['getSha1', 'worker'], - inspector: session, maxRetries: 3, numWorkers: this._options.maxWorkers, }) as WorkerInterface; diff --git a/packages/jest-worker/README.md b/packages/jest-worker/README.md index d89d936b368e..a97ab0ae4e18 100644 --- a/packages/jest-worker/README.md +++ b/packages/jest-worker/README.md @@ -63,6 +63,10 @@ List of method names that can be called on the child processes from the parent p Amount of workers to spawn. Defaults to the number of CPUs minus 1. +#### `workerHeartbeatTimeout: number` (optional) + +Heartbeat timeout used to ping the parent process when child workers are alive. Defaults to 5000 ms. + #### `maxRetries: number` (optional) Maximum amount of times that a dead child can be re-spawned, per call. Defaults to `3`, pass `Infinity` to allow endless retries. diff --git a/packages/jest-worker/src/base/BaseWorkerPool.ts b/packages/jest-worker/src/base/BaseWorkerPool.ts index 6d65913b90df..4a4a23b7113e 100644 --- a/packages/jest-worker/src/base/BaseWorkerPool.ts +++ b/packages/jest-worker/src/base/BaseWorkerPool.ts @@ -39,7 +39,14 @@ export default class BaseWorkerPool { const stdout = mergeStream(); const stderr = mergeStream(); - const {forkOptions, maxRetries, setupArgs, resourceLimits, inspector} = options; + const { + forkOptions, + inspector, + maxRetries, + setupArgs, + resourceLimits, + workerHeartbeatTimeout, + } = options; for (let i = 0; i < options.numWorkers; i++) { const workerOptions: WorkerOptions = { @@ -48,6 +55,7 @@ export default class BaseWorkerPool { maxRetries, resourceLimits, setupArgs, + workerHeartbeatTimeout, workerId: i, workerPath, }; diff --git a/packages/jest-worker/src/index.ts b/packages/jest-worker/src/index.ts index f5db5541343e..dbe8977b70ca 100644 --- a/packages/jest-worker/src/index.ts +++ b/packages/jest-worker/src/index.ts @@ -8,6 +8,8 @@ /* eslint-disable local/ban-types-eventually */ import {cpus} from 'os'; +import * as inspector from 'inspector'; +import type {Session} from 'inspector'; import Farm from './Farm'; import WorkerPool from './WorkerPool'; import type { @@ -75,19 +77,23 @@ export class Worker { private _farm: Farm; private _options: FarmOptions; private _workerPool: WorkerPoolInterface; + private _inspector: Session | undefined; constructor(workerPath: string, options?: FarmOptions) { this._options = {...options}; this._ending = false; + this._inspector = this.setUpInspector(); + const workerPoolOptions: WorkerPoolOptions = { enableWorkerThreads: this._options.enableWorkerThreads || false, forkOptions: this._options.forkOptions || {}, - inspector: this._options.inspector, + inspector: this._inspector, maxRetries: this._options.maxRetries || 3, numWorkers: this._options.numWorkers || Math.max(cpus().length - 1, 1), resourceLimits: this._options.resourceLimits ?? {}, setupArgs: this._options.setupArgs || [], + workerHeartbeatTimeout: this._options.workerHeartbeatTimeout || 5000, }; if (this._options.WorkerPool) { @@ -113,6 +119,20 @@ export class Worker { this._bindExposedWorkerMethods(workerPath, this._options); } + private setUpInspector() { + // Open V8 Inspector + inspector.open(); + + const inspectorUrl = inspector.url(); + let session; + if (inspectorUrl !== undefined) { + session = new inspector.Session(); + session.connect(); + session.post('Debugger.enable'); + } + return session; + } + private _bindExposedWorkerMethods( workerPath: string, options: FarmOptions, @@ -155,6 +175,8 @@ export class Worker { throw new Error('Farm is ended, no more calls can be done to it'); } this._ending = true; + this._inspector?.disconnect(); + inspector.close(); return this._workerPool.end(); } diff --git a/packages/jest-worker/src/types.ts b/packages/jest-worker/src/types.ts index f7a2089ca708..be6ec9ca7fc6 100644 --- a/packages/jest-worker/src/types.ts +++ b/packages/jest-worker/src/types.ts @@ -7,6 +7,7 @@ import type {ForkOptions} from 'child_process'; import type {EventEmitter} from 'events'; +import type {Session} from 'inspector'; // import type {ResourceLimits} from 'worker_threads'; // This is not present in the Node 12 typings @@ -102,9 +103,9 @@ export type FarmOptions = { resourceLimits?: ResourceLimits; setupArgs?: Array; maxRetries?: number; - inspector?: any; numWorkers?: number; taskQueue?: TaskQueue; + workerHeartbeatTimeout?: number; WorkerPool?: ( workerPath: string, options?: WorkerPoolOptions, @@ -118,7 +119,8 @@ export type WorkerPoolOptions = { resourceLimits: ResourceLimits; maxRetries: number; numWorkers: number; - inspector?: any; + inspector: Session | undefined; + workerHeartbeatTimeout: number; enableWorkerThreads: boolean; }; @@ -126,7 +128,8 @@ export type WorkerOptions = { forkOptions: ForkOptions; resourceLimits: ResourceLimits; setupArgs: Array; - inspector?: any; + inspector: Session | undefined; + workerHeartbeatTimeout: number; maxRetries: number; workerId: number; workerPath: string; diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 0f0c861f7431..5695739605c0 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -8,7 +8,6 @@ import {ChildProcess, fork} from 'child_process'; import {PassThrough} from 'stream'; import mergeStream = require('merge-stream'); -import {clearInterval} from 'timers'; import {stdout as stdoutSupportsColor} from 'supports-color'; import { CHILD_MESSAGE_INITIALIZE, @@ -67,7 +66,7 @@ export default class ChildProcessWorker implements WorkerInterface { private _exitPromise: Promise; private _resolveExitPromise!: () => void; - private heartbeatTimeout: any; + private _heartbeatTimeout: any; constructor(options: WorkerOptions) { this._options = options; @@ -130,8 +129,6 @@ export default class ChildProcessWorker implements WorkerInterface { this._options.setupArgs, ]); - this.monitorHeartbeat(() => this.monitorHeartbeatError()); - this._child = child; this._retries++; @@ -149,13 +146,16 @@ export default class ChildProcessWorker implements WorkerInterface { error.stack!, {type: 'WorkerError'}, ]); + + this.monitorHeartbeat(() => this.monitorHeartbeatError()); } } - monitorHeartbeat(onExceeded: () => any) { - this.heartbeatTimeout = setTimeout(() => { + monitorHeartbeat(onExceeded: () => any): void { + clearTimeout(this._heartbeatTimeout); + this._heartbeatTimeout = setTimeout(() => { onExceeded(); - }, 7000); + }, this._options.workerHeartbeatTimeout); } private _shutdown() { @@ -168,58 +168,55 @@ export default class ChildProcessWorker implements WorkerInterface { this._resolveExitPromise(); } - // eslint-disable-next-line @typescript-eslint/explicit-module-boundary-types - async monitorHeartbeatError() { + private async monitorHeartbeatError() { let error; if (this._options.inspector !== undefined) { error = new Error( - `Test worker was unresponsive for 10 seconds. There was an inspector connected so we were unable to capture stack frames before it was terminated.`, + `Test worker was unresponsive for 10 seconds. There was an inspector connected so we were able to capture stack frames before it was terminated.`, ); this._options.inspector.on('Debugger.paused', (message: any) => { const callFrames = message.params.callFrames.slice(0, 20); for (const callFrame of callFrames) { const loc = callFrame['location']; - const url = callFrame['url']; - const lineNumber = loc['lineNumber']; const columnNumber = loc['columnNumber']; + const lineNumber = loc['lineNumber']; + const url = callFrame['url']; const name = callFrame['scopeChain'][0].name; // TODO: process error with stack trace - console.log({url, lineNumber, columnNumber, name}); + console.log({ + columnNumber, + lineNumber, + name, + url, + }); } }); await new Promise((resolve, reject) => { - this._options.inspector.post( - 'Debugger.pause', - (err: any, params: any) => { - if (err === null) { - resolve(params); - } else { - reject(err); - } - }, - ); + this._options.inspector?.post('Debugger.pause', (err: Error) => { + if (err === null) { + resolve(); + } else { + reject(err); + } + }); }); - - this._options.inspector.disconnect(); - this._options.inspector.close(); } else { error = new Error( `Test worker was unresponsive for 10 seconds. There was no inspector connected so we were unable to capture stack frames before it was terminated.`, ); } - // @ts-ignore: adding custom properties to errors. + // @ts-expect-error: adding custom properties to errors. error.type = 1; error.stack = 'test stack'; - if (this.heartbeatTimeout !== undefined) { - clearTimeout(this.heartbeatTimeout); + if (this._heartbeatTimeout !== undefined) { + clearTimeout(this._heartbeatTimeout); } - this._onProcessEnd(error, null); } @@ -229,11 +226,12 @@ export default class ChildProcessWorker implements WorkerInterface { switch (response[0]) { case PARENT_MESSAGE_OK: + clearTimeout(this._heartbeatTimeout); this._onProcessEnd(null, response[1]); - clearInterval(this.heartbeatTimeout); break; case PARENT_MESSAGE_CLIENT_ERROR: + clearTimeout(this._heartbeatTimeout); error = response[4]; if (error != null && typeof error === 'object') { @@ -250,28 +248,28 @@ export default class ChildProcessWorker implements WorkerInterface { error[key] = extra[key]; } } - clearInterval(this.heartbeatTimeout); this._onProcessEnd(error, null); break; case PARENT_MESSAGE_SETUP_ERROR: + clearTimeout(this._heartbeatTimeout); error = new Error('Error when calling setup: ' + response[2]); error.type = response[1]; error.stack = response[3]; this._onProcessEnd(error, null); - clearInterval(this.heartbeatTimeout); break; case PARENT_MESSAGE_HEARTBEAT: this.monitorHeartbeat(() => this.monitorHeartbeatError()); break; case PARENT_MESSAGE_CUSTOM: + clearTimeout(this._heartbeatTimeout); this._onCustomMessage(response[1]); break; default: - clearInterval(this.heartbeatTimeout); + clearTimeout(this._heartbeatTimeout); throw new TypeError('Unexpected response from worker: ' + response[0]); } } From 101ef461198fb23ae15b36ccbfb1eca6b54b405a Mon Sep 17 00:00:00 2001 From: Remi Date: Mon, 26 Oct 2020 14:35:21 +0100 Subject: [PATCH 03/20] feat(jest-worker): inspector logic inside node thread worker + threadChild --- .../src/workers/NodeThreadsWorker.ts | 69 +++++++++++++++++++ .../jest-worker/src/workers/threadChild.ts | 8 +++ 2 files changed, 77 insertions(+) diff --git a/packages/jest-worker/src/workers/NodeThreadsWorker.ts b/packages/jest-worker/src/workers/NodeThreadsWorker.ts index 0d13ce958dd4..599e565380af 100644 --- a/packages/jest-worker/src/workers/NodeThreadsWorker.ts +++ b/packages/jest-worker/src/workers/NodeThreadsWorker.ts @@ -17,6 +17,7 @@ import { OnStart, PARENT_MESSAGE_CLIENT_ERROR, PARENT_MESSAGE_CUSTOM, + PARENT_MESSAGE_HEARTBEAT, PARENT_MESSAGE_OK, PARENT_MESSAGE_SETUP_ERROR, ParentMessage, @@ -41,6 +42,8 @@ export default class ExperimentalWorker implements WorkerInterface { private _resolveExitPromise!: () => void; private _forceExited: boolean; + private _heartbeatTimeout: any; + constructor(options: WorkerOptions) { this._options = options; @@ -126,6 +129,13 @@ export default class ExperimentalWorker implements WorkerInterface { } } + monitorHeartbeat(onExceeded: () => any): void { + clearTimeout(this._heartbeatTimeout); + this._heartbeatTimeout = setTimeout(() => { + onExceeded(); + }, this._options.workerHeartbeatTimeout); + } + private _shutdown() { // End the permanent stream so the merged stream end too if (this._fakeStream) { @@ -136,15 +146,68 @@ export default class ExperimentalWorker implements WorkerInterface { this._resolveExitPromise(); } + private async monitorHeartbeatError() { + let error; + if (this._options.inspector !== undefined) { + error = new Error( + `Test worker was unresponsive for 10 seconds. There was an inspector connected so we were able to capture stack frames before it was terminated.`, + ); + this._options.inspector.on('Debugger.paused', (message: any) => { + const callFrames = message.params.callFrames.slice(0, 20); + for (const callFrame of callFrames) { + const loc = callFrame['location']; + + const columnNumber = loc['columnNumber']; + const lineNumber = loc['lineNumber']; + const url = callFrame['url']; + + const name = callFrame['scopeChain'][0].name; + + // TODO: process error with stack trace + console.log({ + columnNumber, + lineNumber, + name, + url, + }); + } + }); + + await new Promise((resolve, reject) => { + this._options.inspector?.post('Debugger.pause', (err: Error) => { + if (err === null) { + resolve(); + } else { + reject(err); + } + }); + }); + } else { + error = new Error( + `Test worker was unresponsive for 10 seconds. There was no inspector connected so we were unable to capture stack frames before it was terminated.`, + ); + } + // @ts-expect-error: adding custom properties to errors. + error.type = 1; + error.stack = 'test stack'; + + if (this._heartbeatTimeout !== undefined) { + clearTimeout(this._heartbeatTimeout); + } + this._onProcessEnd(error, null); + } + private _onMessage(response: ParentMessage) { let error; switch (response[0]) { case PARENT_MESSAGE_OK: + clearTimeout(this._heartbeatTimeout); this._onProcessEnd(null, response[1]); break; case PARENT_MESSAGE_CLIENT_ERROR: + clearTimeout(this._heartbeatTimeout); error = response[4]; if (error != null && typeof error === 'object') { @@ -166,6 +229,7 @@ export default class ExperimentalWorker implements WorkerInterface { this._onProcessEnd(error, null); break; case PARENT_MESSAGE_SETUP_ERROR: + clearTimeout(this._heartbeatTimeout); error = new Error('Error when calling setup: ' + response[2]); // @ts-expect-error: adding custom properties to errors. @@ -174,10 +238,15 @@ export default class ExperimentalWorker implements WorkerInterface { this._onProcessEnd(error, null); break; + case PARENT_MESSAGE_HEARTBEAT: + this.monitorHeartbeat(() => this.monitorHeartbeatError()); + break; case PARENT_MESSAGE_CUSTOM: + clearTimeout(this._heartbeatTimeout); this._onCustomMessage(response[1]); break; default: + clearTimeout(this._heartbeatTimeout); throw new TypeError('Unexpected response from worker: ' + response[0]); } } diff --git a/packages/jest-worker/src/workers/threadChild.ts b/packages/jest-worker/src/workers/threadChild.ts index 6783ec843510..481bb0b53da2 100644 --- a/packages/jest-worker/src/workers/threadChild.ts +++ b/packages/jest-worker/src/workers/threadChild.ts @@ -14,6 +14,7 @@ import { ChildMessageInitialize, PARENT_MESSAGE_CLIENT_ERROR, PARENT_MESSAGE_ERROR, + PARENT_MESSAGE_HEARTBEAT, PARENT_MESSAGE_OK, PARENT_MESSAGE_SETUP_ERROR, } from '../types'; @@ -68,6 +69,12 @@ function reportSuccess(result: unknown) { parentPort!.postMessage([PARENT_MESSAGE_OK, result]); } +function sendParentMessageHeartbeat() { + if (process && process.send) { + process.send([PARENT_MESSAGE_HEARTBEAT]); + } +} + function reportClientError(error: Error) { return reportError(error, PARENT_MESSAGE_CLIENT_ERROR); } @@ -150,6 +157,7 @@ function execFunction( onError: (error: Error) => void, ): void { let result; + sendParentMessageHeartbeat(); try { result = fn.apply(ctx, args); From e8a056b971844048778878ef8e723c771041602a Mon Sep 17 00:00:00 2001 From: Remi Date: Tue, 27 Oct 2020 10:57:41 +0100 Subject: [PATCH 04/20] feat(jest-worker): child interval heartbeats --- .../jest-worker/src/workers/ChildProcessWorker.ts | 15 +++++++-------- .../jest-worker/src/workers/NodeThreadsWorker.ts | 10 +++++----- packages/jest-worker/src/workers/processChild.ts | 8 +++++++- packages/jest-worker/src/workers/threadChild.ts | 8 +++++++- 4 files changed, 26 insertions(+), 15 deletions(-) diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 5695739605c0..e07290463bf5 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -32,6 +32,8 @@ const SIGTERM_EXIT_CODE = SIGNAL_BASE_EXIT_CODE + 15; // How long to wait after SIGTERM before sending SIGKILL const SIGKILL_DELAY = 500; +const CHILD_HEARTBEAT_INTERVAL = 1_000; + /** * This class wraps the child process and provides a nice interface to * communicate with. It takes care of: @@ -127,6 +129,7 @@ export default class ChildProcessWorker implements WorkerInterface { false, this._options.workerPath, this._options.setupArgs, + CHILD_HEARTBEAT_INTERVAL, ]); this._child = child; @@ -146,8 +149,6 @@ export default class ChildProcessWorker implements WorkerInterface { error.stack!, {type: 'WorkerError'}, ]); - - this.monitorHeartbeat(() => this.monitorHeartbeatError()); } } @@ -173,7 +174,7 @@ export default class ChildProcessWorker implements WorkerInterface { if (this._options.inspector !== undefined) { error = new Error( - `Test worker was unresponsive for 10 seconds. There was an inspector connected so we were able to capture stack frames before it was terminated.`, + `Test worker was unresponsive for ${this._options.workerHeartbeatTimeout} milliseconds. There was an inspector connected so we were able to capture stack frames before it was terminated.`, ); this._options.inspector.on('Debugger.paused', (message: any) => { const callFrames = message.params.callFrames.slice(0, 20); @@ -207,7 +208,7 @@ export default class ChildProcessWorker implements WorkerInterface { }); } else { error = new Error( - `Test worker was unresponsive for 10 seconds. There was no inspector connected so we were unable to capture stack frames before it was terminated.`, + `Test worker was unresponsive for ${this._options.workerHeartbeatTimeout} milliseconds. There was no inspector connected so we were unable to capture stack frames before it was terminated.`, ); } // @ts-expect-error: adding custom properties to errors. @@ -226,12 +227,10 @@ export default class ChildProcessWorker implements WorkerInterface { switch (response[0]) { case PARENT_MESSAGE_OK: - clearTimeout(this._heartbeatTimeout); this._onProcessEnd(null, response[1]); break; case PARENT_MESSAGE_CLIENT_ERROR: - clearTimeout(this._heartbeatTimeout); error = response[4]; if (error != null && typeof error === 'object') { @@ -252,7 +251,6 @@ export default class ChildProcessWorker implements WorkerInterface { break; case PARENT_MESSAGE_SETUP_ERROR: - clearTimeout(this._heartbeatTimeout); error = new Error('Error when calling setup: ' + response[2]); error.type = response[1]; @@ -265,7 +263,6 @@ export default class ChildProcessWorker implements WorkerInterface { this.monitorHeartbeat(() => this.monitorHeartbeatError()); break; case PARENT_MESSAGE_CUSTOM: - clearTimeout(this._heartbeatTimeout); this._onCustomMessage(response[1]); break; default: @@ -288,6 +285,7 @@ export default class ChildProcessWorker implements WorkerInterface { } } else { this._shutdown(); + clearTimeout(this._heartbeatTimeout); } } @@ -323,6 +321,7 @@ export default class ChildProcessWorker implements WorkerInterface { SIGKILL_DELAY, ); this._exitPromise.then(() => clearTimeout(sigkillTimeout)); + clearTimeout(this._heartbeatTimeout); } getWorkerId(): number { diff --git a/packages/jest-worker/src/workers/NodeThreadsWorker.ts b/packages/jest-worker/src/workers/NodeThreadsWorker.ts index 599e565380af..e18cf7fb4bd4 100644 --- a/packages/jest-worker/src/workers/NodeThreadsWorker.ts +++ b/packages/jest-worker/src/workers/NodeThreadsWorker.ts @@ -25,6 +25,8 @@ import { WorkerOptions, } from '../types'; +const CHILD_HEARTBEAT_INTERVAL = 1_000; + export default class ExperimentalWorker implements WorkerInterface { private _worker!: Worker; private _options: WorkerOptions; @@ -109,6 +111,7 @@ export default class ExperimentalWorker implements WorkerInterface { false, this._options.workerPath, this._options.setupArgs, + CHILD_HEARTBEAT_INTERVAL, ]); this._retries++; @@ -202,12 +205,10 @@ export default class ExperimentalWorker implements WorkerInterface { switch (response[0]) { case PARENT_MESSAGE_OK: - clearTimeout(this._heartbeatTimeout); this._onProcessEnd(null, response[1]); break; case PARENT_MESSAGE_CLIENT_ERROR: - clearTimeout(this._heartbeatTimeout); error = response[4]; if (error != null && typeof error === 'object') { @@ -229,7 +230,6 @@ export default class ExperimentalWorker implements WorkerInterface { this._onProcessEnd(error, null); break; case PARENT_MESSAGE_SETUP_ERROR: - clearTimeout(this._heartbeatTimeout); error = new Error('Error when calling setup: ' + response[2]); // @ts-expect-error: adding custom properties to errors. @@ -242,11 +242,9 @@ export default class ExperimentalWorker implements WorkerInterface { this.monitorHeartbeat(() => this.monitorHeartbeatError()); break; case PARENT_MESSAGE_CUSTOM: - clearTimeout(this._heartbeatTimeout); this._onCustomMessage(response[1]); break; default: - clearTimeout(this._heartbeatTimeout); throw new TypeError('Unexpected response from worker: ' + response[0]); } } @@ -260,6 +258,7 @@ export default class ExperimentalWorker implements WorkerInterface { } } else { this._shutdown(); + clearTimeout(this._heartbeatTimeout); } } @@ -270,6 +269,7 @@ export default class ExperimentalWorker implements WorkerInterface { forceExit(): void { this._forceExited = true; this._worker.terminate(); + clearTimeout(this._heartbeatTimeout); } send( diff --git a/packages/jest-worker/src/workers/processChild.ts b/packages/jest-worker/src/workers/processChild.ts index c078d415771a..85524e25937f 100644 --- a/packages/jest-worker/src/workers/processChild.ts +++ b/packages/jest-worker/src/workers/processChild.ts @@ -21,6 +21,8 @@ import { let file: string | null = null; let setupArgs: Array = []; let initialized = false; +let monitorHeartbeat: NodeJS.Timeout; +let heartbeatIntervalValue: number; /** * This file is a small bootstrapper for workers. It sets up the communication @@ -41,6 +43,10 @@ const messageListener: NodeJS.MessageListener = request => { const init: ChildMessageInitialize = request; file = init[2]; setupArgs = request[3]; + heartbeatIntervalValue = request[4]; + monitorHeartbeat = setInterval(() => { + sendParentMessageHeartbeat(); + }, heartbeatIntervalValue); break; case CHILD_MESSAGE_CALL: @@ -115,6 +121,7 @@ function end(): void { function exitProcess(): void { // Clean up open handles so the process ideally exits gracefully process.removeListener('message', messageListener); + clearInterval(monitorHeartbeat); } function execMethod(method: string, args: Array): void { @@ -156,7 +163,6 @@ function execFunction( onError: (error: Error) => void, ): void { let result; - sendParentMessageHeartbeat(); try { result = fn.apply(ctx, args); diff --git a/packages/jest-worker/src/workers/threadChild.ts b/packages/jest-worker/src/workers/threadChild.ts index 481bb0b53da2..67a877de8f7a 100644 --- a/packages/jest-worker/src/workers/threadChild.ts +++ b/packages/jest-worker/src/workers/threadChild.ts @@ -22,6 +22,8 @@ import { let file: string | null = null; let setupArgs: Array = []; let initialized = false; +let monitorHeartbeat: NodeJS.Timeout; +let heartbeatIntervalValue: number; /** * This file is a small bootstrapper for workers. It sets up the communication @@ -42,6 +44,10 @@ const messageListener = (request: any) => { const init: ChildMessageInitialize = request; file = init[2]; setupArgs = request[3]; + heartbeatIntervalValue = request[4]; + monitorHeartbeat = setInterval(() => { + sendParentMessageHeartbeat(); + }, heartbeatIntervalValue); break; case CHILD_MESSAGE_CALL: @@ -116,6 +122,7 @@ function end(): void { function exitProcess(): void { // Clean up open handles so the worker ideally exits gracefully parentPort!.removeListener('message', messageListener); + clearInterval(monitorHeartbeat); } function execMethod(method: string, args: Array): void { @@ -157,7 +164,6 @@ function execFunction( onError: (error: Error) => void, ): void { let result; - sendParentMessageHeartbeat(); try { result = fn.apply(ctx, args); From d51310aafc84c5fc35d1c1df70104dd40058241a Mon Sep 17 00:00:00 2001 From: Remi Date: Wed, 28 Oct 2020 08:00:22 +0100 Subject: [PATCH 05/20] fest(jest-worker): add types + refactor --- packages/jest-worker/src/index.ts | 21 +++++++-------- .../src/workers/ChildProcessWorker.ts | 26 +++++++++---------- .../src/workers/NodeThreadsWorker.ts | 24 ++++++++--------- .../jest-worker/src/workers/processChild.ts | 4 +-- .../jest-worker/src/workers/threadChild.ts | 4 +-- 5 files changed, 37 insertions(+), 42 deletions(-) diff --git a/packages/jest-worker/src/index.ts b/packages/jest-worker/src/index.ts index dbe8977b70ca..c6b20ba4fb0b 100644 --- a/packages/jest-worker/src/index.ts +++ b/packages/jest-worker/src/index.ts @@ -9,7 +9,6 @@ import {cpus} from 'os'; import * as inspector from 'inspector'; -import type {Session} from 'inspector'; import Farm from './Farm'; import WorkerPool from './WorkerPool'; import type { @@ -77,23 +76,23 @@ export class Worker { private _farm: Farm; private _options: FarmOptions; private _workerPool: WorkerPoolInterface; - private _inspector: Session | undefined; + private _inspectorSession: inspector.Session | undefined; constructor(workerPath: string, options?: FarmOptions) { this._options = {...options}; this._ending = false; - this._inspector = this.setUpInspector(); + this._inspectorSession = this.setUpInspector(); const workerPoolOptions: WorkerPoolOptions = { - enableWorkerThreads: this._options.enableWorkerThreads || false, - forkOptions: this._options.forkOptions || {}, - inspector: this._inspector, - maxRetries: this._options.maxRetries || 3, - numWorkers: this._options.numWorkers || Math.max(cpus().length - 1, 1), + enableWorkerThreads: this._options.enableWorkerThreads ?? false, + forkOptions: this._options.forkOptions ?? {}, + inspector: this._inspectorSession, + maxRetries: this._options.maxRetries ?? 3, + numWorkers: this._options.numWorkers ?? Math.max(cpus().length - 1, 1), resourceLimits: this._options.resourceLimits ?? {}, - setupArgs: this._options.setupArgs || [], - workerHeartbeatTimeout: this._options.workerHeartbeatTimeout || 5000, + setupArgs: this._options.setupArgs ?? [], + workerHeartbeatTimeout: this._options.workerHeartbeatTimeout ?? 5000, }; if (this._options.WorkerPool) { @@ -175,7 +174,7 @@ export class Worker { throw new Error('Farm is ended, no more calls can be done to it'); } this._ending = true; - this._inspector?.disconnect(); + this._inspectorSession?.disconnect(); inspector.close(); return this._workerPool.end(); diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index e07290463bf5..6ac15b0f478a 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -68,7 +68,7 @@ export default class ChildProcessWorker implements WorkerInterface { private _exitPromise: Promise; private _resolveExitPromise!: () => void; - private _heartbeatTimeout: any; + private _heartbeatTimeout!: NodeJS.Timeout; constructor(options: WorkerOptions) { this._options = options; @@ -156,7 +156,7 @@ export default class ChildProcessWorker implements WorkerInterface { clearTimeout(this._heartbeatTimeout); this._heartbeatTimeout = setTimeout(() => { onExceeded(); - }, this._options.workerHeartbeatTimeout); + }, this._options.workerHeartbeatTimeout).unref(); } private _shutdown() { @@ -170,22 +170,24 @@ export default class ChildProcessWorker implements WorkerInterface { } private async monitorHeartbeatError() { - let error; + let error = new Error( + `Test worker was unresponsive for ${this._options.workerHeartbeatTimeout} milliseconds. There was no inspector connected so we were unable to capture stack frames before it was terminated.`, + ); - if (this._options.inspector !== undefined) { + if (this._options.inspector) { error = new Error( `Test worker was unresponsive for ${this._options.workerHeartbeatTimeout} milliseconds. There was an inspector connected so we were able to capture stack frames before it was terminated.`, ); this._options.inspector.on('Debugger.paused', (message: any) => { const callFrames = message.params.callFrames.slice(0, 20); for (const callFrame of callFrames) { - const loc = callFrame['location']; + const loc = callFrame.location; - const columnNumber = loc['columnNumber']; - const lineNumber = loc['lineNumber']; - const url = callFrame['url']; + const columnNumber = loc.columnNumber; + const lineNumber = loc.lineNumber; + const url = callFrame.url; - const name = callFrame['scopeChain'][0].name; + const name = callFrame.scopeChain[0].name; // TODO: process error with stack trace console.log({ @@ -206,16 +208,12 @@ export default class ChildProcessWorker implements WorkerInterface { } }); }); - } else { - error = new Error( - `Test worker was unresponsive for ${this._options.workerHeartbeatTimeout} milliseconds. There was no inspector connected so we were unable to capture stack frames before it was terminated.`, - ); } // @ts-expect-error: adding custom properties to errors. error.type = 1; error.stack = 'test stack'; - if (this._heartbeatTimeout !== undefined) { + if (this._heartbeatTimeout) { clearTimeout(this._heartbeatTimeout); } this._onProcessEnd(error, null); diff --git a/packages/jest-worker/src/workers/NodeThreadsWorker.ts b/packages/jest-worker/src/workers/NodeThreadsWorker.ts index e18cf7fb4bd4..cb497cd33ad1 100644 --- a/packages/jest-worker/src/workers/NodeThreadsWorker.ts +++ b/packages/jest-worker/src/workers/NodeThreadsWorker.ts @@ -44,7 +44,7 @@ export default class ExperimentalWorker implements WorkerInterface { private _resolveExitPromise!: () => void; private _forceExited: boolean; - private _heartbeatTimeout: any; + private _heartbeatTimeout!: NodeJS.Timeout; constructor(options: WorkerOptions) { this._options = options; @@ -136,7 +136,7 @@ export default class ExperimentalWorker implements WorkerInterface { clearTimeout(this._heartbeatTimeout); this._heartbeatTimeout = setTimeout(() => { onExceeded(); - }, this._options.workerHeartbeatTimeout); + }, this._options.workerHeartbeatTimeout).unref(); } private _shutdown() { @@ -150,21 +150,23 @@ export default class ExperimentalWorker implements WorkerInterface { } private async monitorHeartbeatError() { - let error; - if (this._options.inspector !== undefined) { + let error = new Error( + `Test worker was unresponsive for 10 seconds. There was no inspector connected so we were unable to capture stack frames before it was terminated.`, + ); + if (this._options.inspector) { error = new Error( `Test worker was unresponsive for 10 seconds. There was an inspector connected so we were able to capture stack frames before it was terminated.`, ); this._options.inspector.on('Debugger.paused', (message: any) => { const callFrames = message.params.callFrames.slice(0, 20); for (const callFrame of callFrames) { - const loc = callFrame['location']; + const loc = callFrame.location; - const columnNumber = loc['columnNumber']; - const lineNumber = loc['lineNumber']; - const url = callFrame['url']; + const columnNumber = loc.columnNumber; + const lineNumber = loc.lineNumber; + const url = callFrame.url; - const name = callFrame['scopeChain'][0].name; + const name = callFrame.scopeChain[0].name; // TODO: process error with stack trace console.log({ @@ -185,10 +187,6 @@ export default class ExperimentalWorker implements WorkerInterface { } }); }); - } else { - error = new Error( - `Test worker was unresponsive for 10 seconds. There was no inspector connected so we were unable to capture stack frames before it was terminated.`, - ); } // @ts-expect-error: adding custom properties to errors. error.type = 1; diff --git a/packages/jest-worker/src/workers/processChild.ts b/packages/jest-worker/src/workers/processChild.ts index 85524e25937f..a56179e7d418 100644 --- a/packages/jest-worker/src/workers/processChild.ts +++ b/packages/jest-worker/src/workers/processChild.ts @@ -46,7 +46,7 @@ const messageListener: NodeJS.MessageListener = request => { heartbeatIntervalValue = request[4]; monitorHeartbeat = setInterval(() => { sendParentMessageHeartbeat(); - }, heartbeatIntervalValue); + }, heartbeatIntervalValue).unref(); break; case CHILD_MESSAGE_CALL: @@ -67,7 +67,7 @@ const messageListener: NodeJS.MessageListener = request => { process.on('message', messageListener); function sendParentMessageHeartbeat() { - if (process && process.send) { + if (process?.send) { process.send([PARENT_MESSAGE_HEARTBEAT]); } } diff --git a/packages/jest-worker/src/workers/threadChild.ts b/packages/jest-worker/src/workers/threadChild.ts index 67a877de8f7a..0cd6251c8c58 100644 --- a/packages/jest-worker/src/workers/threadChild.ts +++ b/packages/jest-worker/src/workers/threadChild.ts @@ -47,7 +47,7 @@ const messageListener = (request: any) => { heartbeatIntervalValue = request[4]; monitorHeartbeat = setInterval(() => { sendParentMessageHeartbeat(); - }, heartbeatIntervalValue); + }, heartbeatIntervalValue).unref(); break; case CHILD_MESSAGE_CALL: @@ -76,7 +76,7 @@ function reportSuccess(result: unknown) { } function sendParentMessageHeartbeat() { - if (process && process.send) { + if (process?.send) { process.send([PARENT_MESSAGE_HEARTBEAT]); } } From e5d4564188776503d469e3e9c4acc4578517695a Mon Sep 17 00:00:00 2001 From: Remi Date: Wed, 28 Oct 2020 16:06:07 +0100 Subject: [PATCH 06/20] feat(jest-worker): add PARENT_MESSAGE_HEARTBEAT_ERROR --- packages/jest-worker/src/types.ts | 10 ++++++++-- packages/jest-worker/src/workers/ChildProcessWorker.ts | 5 +++-- packages/jest-worker/src/workers/NodeThreadsWorker.ts | 5 +++-- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/jest-worker/src/types.ts b/packages/jest-worker/src/types.ts index be6ec9ca7fc6..587324c018d5 100644 --- a/packages/jest-worker/src/types.ts +++ b/packages/jest-worker/src/types.ts @@ -30,11 +30,12 @@ export const PARENT_MESSAGE_CLIENT_ERROR: 1 = 1; export const PARENT_MESSAGE_SETUP_ERROR: 2 = 2; export const PARENT_MESSAGE_CUSTOM: 3 = 3; export const PARENT_MESSAGE_HEARTBEAT: 4 = 4; +export const PARENT_MESSAGE_HEARTBEAT_ERROR: 5 = 5; export type PARENT_MESSAGE_ERROR = | typeof PARENT_MESSAGE_CLIENT_ERROR | typeof PARENT_MESSAGE_SETUP_ERROR - | typeof PARENT_MESSAGE_HEARTBEAT; + | typeof PARENT_MESSAGE_HEARTBEAT_ERROR; export interface WorkerPoolInterface { getStderr(): NodeJS.ReadableStream; @@ -183,6 +184,10 @@ export type ParentMessageOk = [ unknown, // result ]; +export type ParentMessageHeartbeat = [ + typeof PARENT_MESSAGE_HEARTBEAT, // type +]; + export type ParentMessageError = [ PARENT_MESSAGE_ERROR, // type string, // constructor @@ -194,7 +199,8 @@ export type ParentMessageError = [ export type ParentMessage = | ParentMessageOk | ParentMessageError - | ParentMessageCustom; + | ParentMessageCustom + | ParentMessageHeartbeat; // Queue types. diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 6ac15b0f478a..abfb5f848dfa 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -18,6 +18,7 @@ import { PARENT_MESSAGE_CLIENT_ERROR, PARENT_MESSAGE_CUSTOM, PARENT_MESSAGE_HEARTBEAT, + PARENT_MESSAGE_HEARTBEAT_ERROR, PARENT_MESSAGE_OK, PARENT_MESSAGE_SETUP_ERROR, ParentMessage, @@ -209,8 +210,8 @@ export default class ChildProcessWorker implements WorkerInterface { }); }); } - // @ts-expect-error: adding custom properties to errors. - error.type = 1; + // @ts-expect-error: no index + error.type = PARENT_MESSAGE_HEARTBEAT_ERROR; error.stack = 'test stack'; if (this._heartbeatTimeout) { diff --git a/packages/jest-worker/src/workers/NodeThreadsWorker.ts b/packages/jest-worker/src/workers/NodeThreadsWorker.ts index cb497cd33ad1..ba8b009641fe 100644 --- a/packages/jest-worker/src/workers/NodeThreadsWorker.ts +++ b/packages/jest-worker/src/workers/NodeThreadsWorker.ts @@ -18,6 +18,7 @@ import { PARENT_MESSAGE_CLIENT_ERROR, PARENT_MESSAGE_CUSTOM, PARENT_MESSAGE_HEARTBEAT, + PARENT_MESSAGE_HEARTBEAT_ERROR, PARENT_MESSAGE_OK, PARENT_MESSAGE_SETUP_ERROR, ParentMessage, @@ -188,8 +189,8 @@ export default class ExperimentalWorker implements WorkerInterface { }); }); } - // @ts-expect-error: adding custom properties to errors. - error.type = 1; + // @ts-expect-error: no index + error.type = PARENT_MESSAGE_HEARTBEAT_ERROR; error.stack = 'test stack'; if (this._heartbeatTimeout !== undefined) { From 0cf91e22b737deb4b909522dd08e33a15486720c Mon Sep 17 00:00:00 2001 From: Remi Date: Wed, 28 Oct 2020 16:22:35 +0100 Subject: [PATCH 07/20] feat(jest-worker): factory pattern --- packages/jest-haste-map/src/index.ts | 44 ++++++++++-------- .../jest-reporters/src/CoverageReporter.ts | 2 +- packages/jest-runner/src/index.ts | 4 +- packages/jest-worker/README.md | 6 +-- .../src/__performance_tests__/test.js | 13 ++++-- packages/jest-worker/src/index.ts | 46 +++++++++++++------ 6 files changed, 71 insertions(+), 44 deletions(-) diff --git a/packages/jest-haste-map/src/index.ts b/packages/jest-haste-map/src/index.ts index 238e6032ec56..da1c5db944c1 100644 --- a/packages/jest-haste-map/src/index.ts +++ b/packages/jest-haste-map/src/index.ts @@ -546,14 +546,16 @@ export default class HasteMap extends EventEmitter { if (this._options.retainAllFiles && filePath.includes(NODE_MODULES)) { if (computeSha1) { return this._getWorker(workerOptions) - .getSha1({ - computeDependencies: this._options.computeDependencies, - computeSha1, - dependencyExtractor: this._options.dependencyExtractor, - filePath, - hasteImplModulePath: this._options.hasteImplModulePath, - rootDir, - }) + .then(workerInstance => + workerInstance.getSha1({ + computeDependencies: this._options.computeDependencies, + computeSha1, + dependencyExtractor: this._options.dependencyExtractor, + filePath, + hasteImplModulePath: this._options.hasteImplModulePath, + rootDir, + }), + ) .then(workerReply, workerError); } @@ -622,14 +624,16 @@ export default class HasteMap extends EventEmitter { } return this._getWorker(workerOptions) - .worker({ - computeDependencies: this._options.computeDependencies, - computeSha1, - dependencyExtractor: this._options.dependencyExtractor, - filePath, - hasteImplModulePath: this._options.hasteImplModulePath, - rootDir, - }) + .then(workerInstance => + workerInstance.worker({ + computeDependencies: this._options.computeDependencies, + computeSha1, + dependencyExtractor: this._options.dependencyExtractor, + filePath, + hasteImplModulePath: this._options.hasteImplModulePath, + rootDir, + }), + ) .then(workerReply, workerError); } @@ -714,17 +718,19 @@ export default class HasteMap extends EventEmitter { /** * Creates workers or parses files and extracts metadata in-process. */ - private _getWorker(options?: {forceInBand: boolean}): WorkerInterface { + private async _getWorker(options?: { + forceInBand: boolean; + }): Promise { if (!this._worker) { if ((options && options.forceInBand) || this._options.maxWorkers <= 1) { this._worker = {getSha1, worker}; } else { // @ts-expect-error: assignment of a worker with custom properties. - this._worker = new Worker(require.resolve('./worker'), { + this._worker = (await Worker.create(require.resolve('./worker'), { exposedMethods: ['getSha1', 'worker'], maxRetries: 3, numWorkers: this._options.maxWorkers, - }) as WorkerInterface; + })) as WorkerInterface; } } diff --git a/packages/jest-reporters/src/CoverageReporter.ts b/packages/jest-reporters/src/CoverageReporter.ts index 1883b39861e4..bd45667f03e0 100644 --- a/packages/jest-reporters/src/CoverageReporter.ts +++ b/packages/jest-reporters/src/CoverageReporter.ts @@ -153,7 +153,7 @@ export default class CoverageReporter extends BaseReporter { if (this._globalConfig.maxWorkers <= 1) { worker = require('./CoverageWorker'); } else { - worker = new Worker(require.resolve('./CoverageWorker'), { + worker = await Worker.create(require.resolve('./CoverageWorker'), { exposedMethods: ['worker'], maxRetries: 2, numWorkers: this._globalConfig.maxWorkers, diff --git a/packages/jest-runner/src/index.ts b/packages/jest-runner/src/index.ts index b58d881416f4..66081ed2423d 100644 --- a/packages/jest-runner/src/index.ts +++ b/packages/jest-runner/src/index.ts @@ -164,7 +164,7 @@ export default class TestRunner { } } - const worker = new Worker(TEST_WORKER_PATH, { + const worker = (await Worker.create(TEST_WORKER_PATH, { exposedMethods: ['worker'], forkOptions: {stdio: 'pipe'}, maxRetries: 3, @@ -174,7 +174,7 @@ export default class TestRunner { serializableResolvers: Array.from(resolvers.values()), }, ], - }) as WorkerInterface; + })) as WorkerInterface; if (worker.getStdout()) worker.getStdout().pipe(process.stdout); if (worker.getStderr()) worker.getStderr().pipe(process.stderr); diff --git a/packages/jest-worker/README.md b/packages/jest-worker/README.md index a97ab0ae4e18..69a9b0f70e5f 100644 --- a/packages/jest-worker/README.md +++ b/packages/jest-worker/README.md @@ -24,7 +24,7 @@ This example covers the minimal usage: import JestWorker from 'jest-worker'; async function main() { - const worker = new JestWorker(require.resolve('./Worker')); + const worker = await JestWorker.create(require.resolve('./Worker')); const result = await worker.hello('Alice'); // "Hello, Alice" } @@ -160,7 +160,7 @@ This example covers the standard usage: import JestWorker from 'jest-worker'; async function main() { - const myWorker = new JestWorker(require.resolve('./Worker'), { + const myWorker = await JestWorker.create(require.resolve('./Worker'), { exposedMethods: ['foo', 'bar', 'getWorkerId'], numWorkers: 4, }); @@ -204,7 +204,7 @@ This example covers the usage with a `computeWorkerKey` method: import {Worker as JestWorker} from 'jest-worker'; async function main() { - const myWorker = new JestWorker(require.resolve('./Worker'), { + const myWorker = await JestWorker.create(require.resolve('./Worker'), { computeWorkerKey: (method, filename) => filename, }); diff --git a/packages/jest-worker/src/__performance_tests__/test.js b/packages/jest-worker/src/__performance_tests__/test.js index 87bf958b670d..7bf8b5b15284 100644 --- a/packages/jest-worker/src/__performance_tests__/test.js +++ b/packages/jest-worker/src/__performance_tests__/test.js @@ -95,11 +95,14 @@ function testJestWorker() { } } - const farm = new JestWorker(require.resolve('./workers/jest_worker'), { - exposedMethods: [method], - forkOptions: {execArgv: []}, - numWorkers: threads, - }); + const farm = await JestWorker.create( + require.resolve('./workers/jest_worker'), + { + exposedMethods: [method], + forkOptions: {execArgv: []}, + workers: threads, + }, + ); farm.getStdout().pipe(process.stdout); farm.getStderr().pipe(process.stderr); diff --git a/packages/jest-worker/src/index.ts b/packages/jest-worker/src/index.ts index c6b20ba4fb0b..8d3d166eac7d 100644 --- a/packages/jest-worker/src/index.ts +++ b/packages/jest-worker/src/index.ts @@ -82,8 +82,6 @@ export class Worker { this._options = {...options}; this._ending = false; - this._inspectorSession = this.setUpInspector(); - const workerPoolOptions: WorkerPoolOptions = { enableWorkerThreads: this._options.enableWorkerThreads ?? false, forkOptions: this._options.forkOptions ?? {}, @@ -118,19 +116,39 @@ export class Worker { this._bindExposedWorkerMethods(workerPath, this._options); } - private setUpInspector() { - // Open V8 Inspector - inspector.open(); + public static create = async ( + workerPath: string, + options?: FarmOptions, + ): Promise => { + const setUpInspector = async () => { + // Open V8 Inspector + inspector.open(); + + const inspectorUrl = inspector.url(); + if (inspectorUrl) { + const session = new inspector.Session(); + session.connect(); + await new Promise((resolve, reject) => { + session.post('Debugger.enable', (err: Error) => { + if (err === null) { + resolve(); + } else { + reject(err); + } + }); + }); + return session; + } + return undefined; + }; + + const jestWorker = new JestWorker(workerPath, options); + const inspectorSession = await setUpInspector(); - const inspectorUrl = inspector.url(); - let session; - if (inspectorUrl !== undefined) { - session = new inspector.Session(); - session.connect(); - session.post('Debugger.enable'); - } - return session; - } + jestWorker._inspectorSession = inspectorSession; + + return jestWorker; + }; private _bindExposedWorkerMethods( workerPath: string, From ea4f6e2b5486087c9f53319fbde429356aecb32d Mon Sep 17 00:00:00 2001 From: Remi Date: Thu, 29 Oct 2020 20:13:46 +0100 Subject: [PATCH 08/20] feat(jest-worker): add default heartbeat interval value --- packages/jest-worker/src/workers/processChild.ts | 4 +++- packages/jest-worker/src/workers/threadChild.ts | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/packages/jest-worker/src/workers/processChild.ts b/packages/jest-worker/src/workers/processChild.ts index a56179e7d418..4b1c78a10314 100644 --- a/packages/jest-worker/src/workers/processChild.ts +++ b/packages/jest-worker/src/workers/processChild.ts @@ -18,6 +18,8 @@ import { PARENT_MESSAGE_SETUP_ERROR, } from '../types'; +const DEFAULT_HEARTBEAT_INTERVAL_VALUE = 1_000; + let file: string | null = null; let setupArgs: Array = []; let initialized = false; @@ -43,7 +45,7 @@ const messageListener: NodeJS.MessageListener = request => { const init: ChildMessageInitialize = request; file = init[2]; setupArgs = request[3]; - heartbeatIntervalValue = request[4]; + heartbeatIntervalValue = request[4] || DEFAULT_HEARTBEAT_INTERVAL_VALUE; monitorHeartbeat = setInterval(() => { sendParentMessageHeartbeat(); }, heartbeatIntervalValue).unref(); diff --git a/packages/jest-worker/src/workers/threadChild.ts b/packages/jest-worker/src/workers/threadChild.ts index 0cd6251c8c58..5077b94a337c 100644 --- a/packages/jest-worker/src/workers/threadChild.ts +++ b/packages/jest-worker/src/workers/threadChild.ts @@ -19,6 +19,8 @@ import { PARENT_MESSAGE_SETUP_ERROR, } from '../types'; +const DEFAULT_HEARTBEAT_INTERVAL_VALUE = 1_000; + let file: string | null = null; let setupArgs: Array = []; let initialized = false; @@ -44,7 +46,7 @@ const messageListener = (request: any) => { const init: ChildMessageInitialize = request; file = init[2]; setupArgs = request[3]; - heartbeatIntervalValue = request[4]; + heartbeatIntervalValue = request[4] || DEFAULT_HEARTBEAT_INTERVAL_VALUE; monitorHeartbeat = setInterval(() => { sendParentMessageHeartbeat(); }, heartbeatIntervalValue).unref(); From 7512463a10ea14b6ee83d9dc64481b8362b49b2c Mon Sep 17 00:00:00 2001 From: Remi Date: Thu, 29 Oct 2020 20:26:40 +0100 Subject: [PATCH 09/20] test(jest-worker): add heartbeat logic --- .../__tests__/ChildProcessWorker.test.js | 36 +++++++++++++++++++ .../__tests__/NodeThreadsWorker.test.js | 5 +++ .../workers/__tests__/processChild.test.js | 25 +++++++++++++ 3 files changed, 66 insertions(+) diff --git a/packages/jest-worker/src/workers/__tests__/ChildProcessWorker.test.js b/packages/jest-worker/src/workers/__tests__/ChildProcessWorker.test.js index e5e7e1e99694..22481cb876de 100644 --- a/packages/jest-worker/src/workers/__tests__/ChildProcessWorker.test.js +++ b/packages/jest-worker/src/workers/__tests__/ChildProcessWorker.test.js @@ -11,6 +11,7 @@ import getStream from 'get-stream'; import supportsColor from 'supports-color'; import { CHILD_MESSAGE_CALL, + PARENT_MESSAGE_HEARTBEAT, CHILD_MESSAGE_INITIALIZE, PARENT_MESSAGE_CLIENT_ERROR, PARENT_MESSAGE_CUSTOM, @@ -19,6 +20,9 @@ import { jest.useFakeTimers(); +const CHILD_HEARTBEAT_INTERVAL = 1_000; +const WORKER_HEARTBEAT_TIMEOUT = 5_000; + let Worker; let forkInterface; let childProcess; @@ -93,6 +97,7 @@ it('initializes the child process with the given workerPath', () => { maxRetries: 3, setupArgs: ['foo', 'bar'], workerPath: '/tmp/foo/bar/baz.js', + workerHeartbeatTimeout: WORKER_HEARTBEAT_TIMEOUT, }); expect(forkInterface.send.mock.calls[0][0]).toEqual([ @@ -100,6 +105,7 @@ it('initializes the child process with the given workerPath', () => { false, '/tmp/foo/bar/baz.js', ['foo', 'bar'], + CHILD_HEARTBEAT_INTERVAL, ]); }); @@ -411,3 +417,33 @@ it('does not send SIGKILL if SIGTERM exited the process', async () => { jest.runAllTimers(); expect(forkInterface.kill.mock.calls).toEqual([['SIGTERM']]); }); + +it('calls the onProcessEnd method when we have no heartbeat message from child during n-time', () => { + jest.useFakeTimers(); + + const worker = new Worker({ + forkOptions: {}, + maxRetries: 3, + workerPath: '/tmp/foo', + workerHeartbeatTimeout: WORKER_HEARTBEAT_TIMEOUT, + }); + + const onProcessStart = jest.fn(); + const onProcessEnd = jest.fn(); + + worker.send( + [CHILD_MESSAGE_INITIALIZE, false, 'foo', [], CHILD_HEARTBEAT_INTERVAL], + onProcessStart, + onProcessEnd, + ); + + expect(onProcessEnd).not.toHaveBeenCalled(); + + forkInterface.emit('message', [PARENT_MESSAGE_HEARTBEAT]); + + jest.advanceTimersByTime(WORKER_HEARTBEAT_TIMEOUT - 1); + expect(onProcessEnd).not.toHaveBeenCalled(); + + jest.advanceTimersByTime(1); + expect(onProcessEnd).toHaveBeenCalledTimes(1); +}); diff --git a/packages/jest-worker/src/workers/__tests__/NodeThreadsWorker.test.js b/packages/jest-worker/src/workers/__tests__/NodeThreadsWorker.test.js index 5bc2fe215aa1..a61d1feac234 100644 --- a/packages/jest-worker/src/workers/__tests__/NodeThreadsWorker.test.js +++ b/packages/jest-worker/src/workers/__tests__/NodeThreadsWorker.test.js @@ -16,6 +16,9 @@ import { PARENT_MESSAGE_OK, } from '../../types'; +const CHILD_HEARTBEAT_INTERVAL = 1_000; +const WORKER_HEARTBEAT_TIMEOUT = 5_000; + let Worker; let workerThreads; let originalExecArgv; @@ -102,6 +105,7 @@ it('initializes the thread with the given workerPath', () => { maxRetries: 3, setupArgs: ['foo', 'bar'], workerPath: '/tmp/foo/bar/baz.js', + workerHeartbeatTimeout: WORKER_HEARTBEAT_TIMEOUT, }); expect(worker._worker.postMessage.mock.calls[0][0]).toEqual([ @@ -109,6 +113,7 @@ it('initializes the thread with the given workerPath', () => { false, '/tmp/foo/bar/baz.js', ['foo', 'bar'], + CHILD_HEARTBEAT_INTERVAL, ]); }); diff --git a/packages/jest-worker/src/workers/__tests__/processChild.test.js b/packages/jest-worker/src/workers/__tests__/processChild.test.js index ba579e052efe..7bb8fc753c95 100644 --- a/packages/jest-worker/src/workers/__tests__/processChild.test.js +++ b/packages/jest-worker/src/workers/__tests__/processChild.test.js @@ -17,11 +17,14 @@ const sleep = ms => new Promise(resolve => setTimeout(resolve, ms)); import { CHILD_MESSAGE_CALL, CHILD_MESSAGE_END, + PARENT_MESSAGE_HEARTBEAT, CHILD_MESSAGE_INITIALIZE, PARENT_MESSAGE_CLIENT_ERROR, PARENT_MESSAGE_OK, } from '../../types'; +const CHILD_HEARTBEAT_INTERVAL = 1_000; + let ended; let mockCount; let initializeParm = uninitializedParam; @@ -376,3 +379,25 @@ it('throws if child is not forked', () => { ]); }).toThrow(); }); + +it('should emit an heartbeat message every time interval', () => { + jest.useFakeTimers(); + + const HEARTBEATS_NUM = 3; + + process.emit('message', [ + CHILD_MESSAGE_INITIALIZE, + false, // Not really used here, but for flow type purity. + './my-fancy-worker', + ['foo'], // Pass empty initialize params so the initialize method is called. + CHILD_HEARTBEAT_INTERVAL, + ]); + + jest.advanceTimersByTime(CHILD_HEARTBEAT_INTERVAL * HEARTBEATS_NUM); + + expect(process.send).toHaveBeenCalledTimes(HEARTBEATS_NUM); + + for (let i = 0; i < HEARTBEATS_NUM; i++) { + expect(process.send.mock.calls[i][0]).toEqual([PARENT_MESSAGE_HEARTBEAT]); + } +}); From 064c528bbb9eb1a53507c84d97d7dcdeef629de4 Mon Sep 17 00:00:00 2001 From: Remi Date: Thu, 29 Oct 2020 20:45:06 +0100 Subject: [PATCH 10/20] fix(jest-worker): linter --- .../src/workers/__tests__/ChildProcessWorker.test.js | 6 +++--- .../src/workers/__tests__/NodeThreadsWorker.test.js | 2 +- .../jest-worker/src/workers/__tests__/processChild.test.js | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/jest-worker/src/workers/__tests__/ChildProcessWorker.test.js b/packages/jest-worker/src/workers/__tests__/ChildProcessWorker.test.js index 22481cb876de..b6e45f4a64a3 100644 --- a/packages/jest-worker/src/workers/__tests__/ChildProcessWorker.test.js +++ b/packages/jest-worker/src/workers/__tests__/ChildProcessWorker.test.js @@ -11,10 +11,10 @@ import getStream from 'get-stream'; import supportsColor from 'supports-color'; import { CHILD_MESSAGE_CALL, - PARENT_MESSAGE_HEARTBEAT, CHILD_MESSAGE_INITIALIZE, PARENT_MESSAGE_CLIENT_ERROR, PARENT_MESSAGE_CUSTOM, + PARENT_MESSAGE_HEARTBEAT, PARENT_MESSAGE_OK, } from '../../types'; @@ -96,8 +96,8 @@ it('initializes the child process with the given workerPath', () => { forkOptions: {}, maxRetries: 3, setupArgs: ['foo', 'bar'], - workerPath: '/tmp/foo/bar/baz.js', workerHeartbeatTimeout: WORKER_HEARTBEAT_TIMEOUT, + workerPath: '/tmp/foo/bar/baz.js', }); expect(forkInterface.send.mock.calls[0][0]).toEqual([ @@ -424,8 +424,8 @@ it('calls the onProcessEnd method when we have no heartbeat message from child d const worker = new Worker({ forkOptions: {}, maxRetries: 3, - workerPath: '/tmp/foo', workerHeartbeatTimeout: WORKER_HEARTBEAT_TIMEOUT, + workerPath: '/tmp/foo', }); const onProcessStart = jest.fn(); diff --git a/packages/jest-worker/src/workers/__tests__/NodeThreadsWorker.test.js b/packages/jest-worker/src/workers/__tests__/NodeThreadsWorker.test.js index a61d1feac234..1df90202f1f2 100644 --- a/packages/jest-worker/src/workers/__tests__/NodeThreadsWorker.test.js +++ b/packages/jest-worker/src/workers/__tests__/NodeThreadsWorker.test.js @@ -104,8 +104,8 @@ it('initializes the thread with the given workerPath', () => { forkOptions: {}, maxRetries: 3, setupArgs: ['foo', 'bar'], - workerPath: '/tmp/foo/bar/baz.js', workerHeartbeatTimeout: WORKER_HEARTBEAT_TIMEOUT, + workerPath: '/tmp/foo/bar/baz.js', }); expect(worker._worker.postMessage.mock.calls[0][0]).toEqual([ diff --git a/packages/jest-worker/src/workers/__tests__/processChild.test.js b/packages/jest-worker/src/workers/__tests__/processChild.test.js index 7bb8fc753c95..3e70cb1ee667 100644 --- a/packages/jest-worker/src/workers/__tests__/processChild.test.js +++ b/packages/jest-worker/src/workers/__tests__/processChild.test.js @@ -17,9 +17,9 @@ const sleep = ms => new Promise(resolve => setTimeout(resolve, ms)); import { CHILD_MESSAGE_CALL, CHILD_MESSAGE_END, - PARENT_MESSAGE_HEARTBEAT, CHILD_MESSAGE_INITIALIZE, PARENT_MESSAGE_CLIENT_ERROR, + PARENT_MESSAGE_HEARTBEAT, PARENT_MESSAGE_OK, } from '../../types'; From 2d90d2d0ee54ecc926ee312a0ec21cd0f4f73402 Mon Sep 17 00:00:00 2001 From: Remi Date: Mon, 2 Nov 2020 16:26:57 +0100 Subject: [PATCH 11/20] feat(jest-haste-map): mock jest-worker --- .../src/__tests__/index.test.js | 68 ++++++++++++++++++- 1 file changed, 66 insertions(+), 2 deletions(-) diff --git a/packages/jest-haste-map/src/__tests__/index.test.js b/packages/jest-haste-map/src/__tests__/index.test.js index d400e73cbe68..bbd64e7a5769 100644 --- a/packages/jest-haste-map/src/__tests__/index.test.js +++ b/packages/jest-haste-map/src/__tests__/index.test.js @@ -20,7 +20,7 @@ jest.mock('child_process', () => ({ })); jest.mock('jest-worker', () => ({ - Worker: jest.fn(worker => { + create: jest.fn(worker => { mockWorker = jest.fn((...args) => require(worker).worker(...args)); mockEnd = jest.fn(); @@ -1225,7 +1225,71 @@ describe('HasteMap', () => { dependencyExtractor, hasteImplModulePath: undefined, maxWorkers: 4, - }).build(); + }) + .build() + .then(({__hasteMapForTest: data}) => { + expect(jestWorker.create.mock.calls.length).toBe(1); + + expect(mockWorker.mock.calls.length).toBe(5); + + expect(mockWorker.mock.calls).toEqual([ + [ + { + computeDependencies: true, + computeSha1: false, + dependencyExtractor, + filePath: path.join('/', 'project', 'fruits', 'Banana.js'), + hasteImplModulePath: undefined, + rootDir: path.join('/', 'project'), + }, + ], + [ + { + computeDependencies: true, + computeSha1: false, + dependencyExtractor, + filePath: path.join('/', 'project', 'fruits', 'Pear.js'), + hasteImplModulePath: undefined, + rootDir: path.join('/', 'project'), + }, + ], + [ + { + computeDependencies: true, + computeSha1: false, + dependencyExtractor, + filePath: path.join('/', 'project', 'fruits', 'Strawberry.js'), + hasteImplModulePath: undefined, + rootDir: path.join('/', 'project'), + }, + ], + [ + { + computeDependencies: true, + computeSha1: false, + dependencyExtractor, + filePath: path.join( + '/', + 'project', + 'fruits', + '__mocks__', + 'Pear.js', + ), + hasteImplModulePath: undefined, + rootDir: path.join('/', 'project'), + }, + ], + [ + { + computeDependencies: true, + computeSha1: false, + dependencyExtractor, + filePath: path.join('/', 'project', 'vegetables', 'Melon.js'), + hasteImplModulePath: undefined, + rootDir: path.join('/', 'project'), + }, + ], + ]); expect(jestWorker.mock.calls.length).toBe(1); From aaaf93a0f57b23a514a0b96194ab8b055c761a42 Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Tue, 3 Nov 2020 12:31:20 +0000 Subject: [PATCH 12/20] fix(jest-haste-map): worker creation race --- packages/jest-haste-map/src/index.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/jest-haste-map/src/index.ts b/packages/jest-haste-map/src/index.ts index da1c5db944c1..6897cb60869c 100644 --- a/packages/jest-haste-map/src/index.ts +++ b/packages/jest-haste-map/src/index.ts @@ -220,7 +220,7 @@ export default class HasteMap extends EventEmitter { private _console: Console; private _options: InternalOptions; private _watchers: Array; - private _worker: WorkerInterface | null; + private _worker: Promise | null; constructor(options: Options) { super(); @@ -723,14 +723,14 @@ export default class HasteMap extends EventEmitter { }): Promise { if (!this._worker) { if ((options && options.forceInBand) || this._options.maxWorkers <= 1) { - this._worker = {getSha1, worker}; + this._worker = Promise.resolve({getSha1, worker}); } else { // @ts-expect-error: assignment of a worker with custom properties. - this._worker = (await Worker.create(require.resolve('./worker'), { + this._worker = Worker.create(require.resolve('./worker'), { exposedMethods: ['getSha1', 'worker'], maxRetries: 3, numWorkers: this._options.maxWorkers, - })) as WorkerInterface; + }) as Promise; } } From c8c86f4a7a5dc18e3c19c65a5d7d6d2393ef179d Mon Sep 17 00:00:00 2001 From: Remi Date: Wed, 4 Nov 2020 17:57:03 +0100 Subject: [PATCH 13/20] feat(jest-runner): update jest-worker mock --- packages/jest-runner/src/__tests__/testRunner.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jest-runner/src/__tests__/testRunner.test.ts b/packages/jest-runner/src/__tests__/testRunner.test.ts index 16fd75e183ad..2a3583811132 100644 --- a/packages/jest-runner/src/__tests__/testRunner.test.ts +++ b/packages/jest-runner/src/__tests__/testRunner.test.ts @@ -12,7 +12,7 @@ import TestRunner from '../index'; let mockWorkerFarm; jest.mock('jest-worker', () => ({ - Worker: jest.fn( + create: jest.fn( worker => (mockWorkerFarm = { end: jest.fn().mockResolvedValue({forceExited: false}), From 4b8fd072dfb370da5fc202cfca77a7773c3a0d98 Mon Sep 17 00:00:00 2001 From: Remi Date: Wed, 4 Nov 2020 17:58:30 +0100 Subject: [PATCH 14/20] refactor(jest-worker): rename heartbeat message error --- packages/jest-worker/src/types.ts | 6 +++--- packages/jest-worker/src/workers/ChildProcessWorker.ts | 4 ++-- packages/jest-worker/src/workers/NodeThreadsWorker.ts | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/packages/jest-worker/src/types.ts b/packages/jest-worker/src/types.ts index 587324c018d5..f5c5c9322ff7 100644 --- a/packages/jest-worker/src/types.ts +++ b/packages/jest-worker/src/types.ts @@ -30,12 +30,12 @@ export const PARENT_MESSAGE_CLIENT_ERROR: 1 = 1; export const PARENT_MESSAGE_SETUP_ERROR: 2 = 2; export const PARENT_MESSAGE_CUSTOM: 3 = 3; export const PARENT_MESSAGE_HEARTBEAT: 4 = 4; -export const PARENT_MESSAGE_HEARTBEAT_ERROR: 5 = 5; + +export const HEARTBEAT_ERROR: 0 = 0; export type PARENT_MESSAGE_ERROR = | typeof PARENT_MESSAGE_CLIENT_ERROR - | typeof PARENT_MESSAGE_SETUP_ERROR - | typeof PARENT_MESSAGE_HEARTBEAT_ERROR; + | typeof PARENT_MESSAGE_SETUP_ERROR; export interface WorkerPoolInterface { getStderr(): NodeJS.ReadableStream; diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index abfb5f848dfa..e96c85b311e7 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -12,13 +12,13 @@ import {stdout as stdoutSupportsColor} from 'supports-color'; import { CHILD_MESSAGE_INITIALIZE, ChildMessage, + HEARTBEAT_ERROR, OnCustomMessage, OnEnd, OnStart, PARENT_MESSAGE_CLIENT_ERROR, PARENT_MESSAGE_CUSTOM, PARENT_MESSAGE_HEARTBEAT, - PARENT_MESSAGE_HEARTBEAT_ERROR, PARENT_MESSAGE_OK, PARENT_MESSAGE_SETUP_ERROR, ParentMessage, @@ -211,7 +211,7 @@ export default class ChildProcessWorker implements WorkerInterface { }); } // @ts-expect-error: no index - error.type = PARENT_MESSAGE_HEARTBEAT_ERROR; + error.type = HEARTBEAT_ERROR; error.stack = 'test stack'; if (this._heartbeatTimeout) { diff --git a/packages/jest-worker/src/workers/NodeThreadsWorker.ts b/packages/jest-worker/src/workers/NodeThreadsWorker.ts index ba8b009641fe..34fc68abaabe 100644 --- a/packages/jest-worker/src/workers/NodeThreadsWorker.ts +++ b/packages/jest-worker/src/workers/NodeThreadsWorker.ts @@ -12,13 +12,13 @@ import mergeStream = require('merge-stream'); import { CHILD_MESSAGE_INITIALIZE, ChildMessage, + HEARTBEAT_ERROR, OnCustomMessage, OnEnd, OnStart, PARENT_MESSAGE_CLIENT_ERROR, PARENT_MESSAGE_CUSTOM, PARENT_MESSAGE_HEARTBEAT, - PARENT_MESSAGE_HEARTBEAT_ERROR, PARENT_MESSAGE_OK, PARENT_MESSAGE_SETUP_ERROR, ParentMessage, @@ -190,7 +190,7 @@ export default class ExperimentalWorker implements WorkerInterface { }); } // @ts-expect-error: no index - error.type = PARENT_MESSAGE_HEARTBEAT_ERROR; + error.type = HEARTBEAT_ERROR; error.stack = 'test stack'; if (this._heartbeatTimeout !== undefined) { From 728389386f3120dd12571f585e4ada58bb67dff7 Mon Sep 17 00:00:00 2001 From: Remi Date: Wed, 4 Nov 2020 19:33:34 +0100 Subject: [PATCH 15/20] fix(jest-worker): linter + _inspectorSession test --- packages/jest-worker/src/__tests__/index.test.js | 8 ++++++++ packages/jest-worker/src/index.ts | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/packages/jest-worker/src/__tests__/index.test.js b/packages/jest-worker/src/__tests__/index.test.js index ec6f091289d5..5439ba4a1caf 100644 --- a/packages/jest-worker/src/__tests__/index.test.js +++ b/packages/jest-worker/src/__tests__/index.test.js @@ -7,6 +7,8 @@ 'use strict'; +import {Session} from 'inspector'; + let Farm; let WorkerPool; let Queue; @@ -170,3 +172,9 @@ it('calls getStderr and getStdout from worker', async () => { expect(farm.getStderr()('err')).toEqual('err'); expect(farm.getStdout()('out')).toEqual('out'); }); + +it('should create a worker with an inspector attached to it', async () => { + const farm = await Farm.create('/fake-worker.js'); + + expect(farm._inspectorSession instanceof Session).toBe(true); +}); diff --git a/packages/jest-worker/src/index.ts b/packages/jest-worker/src/index.ts index 8d3d166eac7d..c38387ffdb1e 100644 --- a/packages/jest-worker/src/index.ts +++ b/packages/jest-worker/src/index.ts @@ -7,8 +7,8 @@ /* eslint-disable local/ban-types-eventually */ -import {cpus} from 'os'; import * as inspector from 'inspector'; +import {cpus} from 'os'; import Farm from './Farm'; import WorkerPool from './WorkerPool'; import type { From 9aee53fcd5ab2961b313ea403fc56e6db4467887 Mon Sep 17 00:00:00 2001 From: Remi Date: Tue, 1 Dec 2020 23:00:33 +0100 Subject: [PATCH 16/20] fest(jest-worker): e2e + handle stack trace --- e2e/__tests__/inspector.test.ts | 46 ++++++++++++++++++ .../src/workers/ChildProcessWorker.ts | 43 ++++++++++++----- .../src/workers/NodeThreadsWorker.ts | 47 +++++++++++++------ 3 files changed, 109 insertions(+), 27 deletions(-) create mode 100644 e2e/__tests__/inspector.test.ts diff --git a/e2e/__tests__/inspector.test.ts b/e2e/__tests__/inspector.test.ts new file mode 100644 index 000000000000..448574dbd38c --- /dev/null +++ b/e2e/__tests__/inspector.test.ts @@ -0,0 +1,46 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ +import {tmpdir} from 'os'; +import * as path from 'path'; +import { + cleanup, + generateTestFilesToForceUsingWorkers, + writeFiles, +} from '../Utils'; +import runJest from '../runJest'; + +const DIR = path.resolve(tmpdir(), 'inspector'); + +beforeEach(() => cleanup(DIR)); +afterAll(() => cleanup(DIR)); + +test('fails a test which causes an infinite loop', () => { + const testFiles = { + ...generateTestFilesToForceUsingWorkers(), + 'package.json': `{ + "testEnvironment": "node" + }`, + }; + + writeFiles(DIR, { + ...testFiles, + '__tests__/inspector.test.js': ` + test('infinite loop error', () => { + while(true) {} + }); + `, + }); + + const {exitCode, stdout, stderr} = runJest(DIR, ['--maxWorkers=2']); + + console.log({exitCode, stdout, stderr}); + + const numberOfTestsPassed = (stderr.match(/\bPASS\b/g) || []).length; + + expect(numberOfTestsPassed).toBe(Object.keys(testFiles).length - 1); + expect(stderr).toContain('FAIL __tests__/inspector.test.js'); +}); diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index e96c85b311e7..efa9b24b972f 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -26,6 +26,13 @@ import { WorkerOptions, } from '../types'; +interface ErrorFrame { + columnNumber: number; + lineNumber: number; + name: string; + url: string; +} + const SIGNAL_BASE_EXIT_CODE = 128; const SIGKILL_EXIT_CODE = SIGNAL_BASE_EXIT_CODE + 9; const SIGTERM_EXIT_CODE = SIGNAL_BASE_EXIT_CODE + 15; @@ -171,15 +178,16 @@ export default class ChildProcessWorker implements WorkerInterface { } private async monitorHeartbeatError() { - let error = new Error( - `Test worker was unresponsive for ${this._options.workerHeartbeatTimeout} milliseconds. There was no inspector connected so we were unable to capture stack frames before it was terminated.`, - ); + if (this._heartbeatTimeout) { + clearTimeout(this._heartbeatTimeout); + } if (this._options.inspector) { - error = new Error( + const error = new Error( `Test worker was unresponsive for ${this._options.workerHeartbeatTimeout} milliseconds. There was an inspector connected so we were able to capture stack frames before it was terminated.`, ); this._options.inspector.on('Debugger.paused', (message: any) => { + const frames: ErrorFrame[] = []; const callFrames = message.params.callFrames.slice(0, 20); for (const callFrame of callFrames) { const loc = callFrame.location; @@ -190,14 +198,19 @@ export default class ChildProcessWorker implements WorkerInterface { const name = callFrame.scopeChain[0].name; - // TODO: process error with stack trace - console.log({ + frames.push({ columnNumber, lineNumber, name, url, }); } + + // @ts-expect-error: no index + error.type = HEARTBEAT_ERROR; + error.stack = JSON.stringify(frames); + + this._onProcessEnd(error, null); }); await new Promise((resolve, reject) => { @@ -209,15 +222,15 @@ export default class ChildProcessWorker implements WorkerInterface { } }); }); - } - // @ts-expect-error: no index - error.type = HEARTBEAT_ERROR; - error.stack = 'test stack'; + } else { + const error = new Error( + `Test worker was unresponsive for ${this._options.workerHeartbeatTimeout} milliseconds. There was no inspector connected so we were unable to capture stack frames before it was terminated.`, + ); + // @ts-expect-error: no index + error.type = HEARTBEAT_ERROR; - if (this._heartbeatTimeout) { - clearTimeout(this._heartbeatTimeout); + this._onProcessEnd(error, null); } - this._onProcessEnd(error, null); } private _onMessage(response: ParentMessage) { @@ -230,6 +243,10 @@ export default class ChildProcessWorker implements WorkerInterface { break; case PARENT_MESSAGE_CLIENT_ERROR: + if (this._heartbeatTimeout) { + clearTimeout(this._heartbeatTimeout); + this.forceExit(); + } error = response[4]; if (error != null && typeof error === 'object') { diff --git a/packages/jest-worker/src/workers/NodeThreadsWorker.ts b/packages/jest-worker/src/workers/NodeThreadsWorker.ts index 34fc68abaabe..232c3bb19279 100644 --- a/packages/jest-worker/src/workers/NodeThreadsWorker.ts +++ b/packages/jest-worker/src/workers/NodeThreadsWorker.ts @@ -26,6 +26,13 @@ import { WorkerOptions, } from '../types'; +interface ErrorFrame { + columnNumber: number; + lineNumber: number; + name: string; + url: string; +} + const CHILD_HEARTBEAT_INTERVAL = 1_000; export default class ExperimentalWorker implements WorkerInterface { @@ -151,14 +158,16 @@ export default class ExperimentalWorker implements WorkerInterface { } private async monitorHeartbeatError() { - let error = new Error( - `Test worker was unresponsive for 10 seconds. There was no inspector connected so we were unable to capture stack frames before it was terminated.`, - ); + if (this._heartbeatTimeout) { + clearTimeout(this._heartbeatTimeout); + } + if (this._options.inspector) { - error = new Error( - `Test worker was unresponsive for 10 seconds. There was an inspector connected so we were able to capture stack frames before it was terminated.`, + const error = new Error( + `Test worker was unresponsive for ${this._options.workerHeartbeatTimeout} milliseconds. There was an inspector connected so we were able to capture stack frames before it was terminated.`, ); this._options.inspector.on('Debugger.paused', (message: any) => { + const frames: ErrorFrame[] = []; const callFrames = message.params.callFrames.slice(0, 20); for (const callFrame of callFrames) { const loc = callFrame.location; @@ -169,14 +178,19 @@ export default class ExperimentalWorker implements WorkerInterface { const name = callFrame.scopeChain[0].name; - // TODO: process error with stack trace - console.log({ + frames.push({ columnNumber, lineNumber, name, url, }); } + + // @ts-expect-error: no index + error.type = HEARTBEAT_ERROR; + error.stack = JSON.stringify(frames); + + this._onProcessEnd(error, null); }); await new Promise((resolve, reject) => { @@ -188,15 +202,15 @@ export default class ExperimentalWorker implements WorkerInterface { } }); }); - } - // @ts-expect-error: no index - error.type = HEARTBEAT_ERROR; - error.stack = 'test stack'; + } else { + const error = new Error( + `Test worker was unresponsive for ${this._options.workerHeartbeatTimeout} milliseconds. There was no inspector connected so we were unable to capture stack frames before it was terminated.`, + ); + // @ts-expect-error: no index + error.type = HEARTBEAT_ERROR; - if (this._heartbeatTimeout !== undefined) { - clearTimeout(this._heartbeatTimeout); + this._onProcessEnd(error, null); } - this._onProcessEnd(error, null); } private _onMessage(response: ParentMessage) { @@ -208,6 +222,10 @@ export default class ExperimentalWorker implements WorkerInterface { break; case PARENT_MESSAGE_CLIENT_ERROR: + if (this._heartbeatTimeout) { + clearTimeout(this._heartbeatTimeout); + this.forceExit(); + } error = response[4]; if (error != null && typeof error === 'object') { @@ -244,6 +262,7 @@ export default class ExperimentalWorker implements WorkerInterface { this._onCustomMessage(response[1]); break; default: + clearTimeout(this._heartbeatTimeout); throw new TypeError('Unexpected response from worker: ' + response[0]); } } From c9e575425b76455b0a58698ce1a6a8ba3e875666 Mon Sep 17 00:00:00 2001 From: Remi Date: Thu, 28 Jan 2021 16:44:24 +0100 Subject: [PATCH 17/20] fix(jest-worker): type's fixes --- packages/jest-haste-map/src/index.ts | 3 --- packages/jest-worker/src/index.ts | 6 +++--- packages/jest-worker/src/workers/ChildProcessWorker.ts | 2 +- packages/jest-worker/src/workers/NodeThreadsWorker.ts | 2 +- 4 files changed, 5 insertions(+), 8 deletions(-) diff --git a/packages/jest-haste-map/src/index.ts b/packages/jest-haste-map/src/index.ts index 6897cb60869c..2e81a59c5635 100644 --- a/packages/jest-haste-map/src/index.ts +++ b/packages/jest-haste-map/src/index.ts @@ -50,9 +50,6 @@ import {getSha1, worker} from './worker'; // understand `require`. const {version: VERSION} = require('../package.json'); -import inspector = require('inspector'); -type HType = typeof H; - type Options = { cacheDirectory?: string; computeDependencies?: boolean; diff --git a/packages/jest-worker/src/index.ts b/packages/jest-worker/src/index.ts index c38387ffdb1e..1290b9abb8c1 100644 --- a/packages/jest-worker/src/index.ts +++ b/packages/jest-worker/src/index.ts @@ -119,7 +119,7 @@ export class Worker { public static create = async ( workerPath: string, options?: FarmOptions, - ): Promise => { + ): Promise => { const setUpInspector = async () => { // Open V8 Inspector inspector.open(); @@ -128,7 +128,7 @@ export class Worker { if (inspectorUrl) { const session = new inspector.Session(); session.connect(); - await new Promise((resolve, reject) => { + await new Promise((resolve, reject) => { session.post('Debugger.enable', (err: Error) => { if (err === null) { resolve(); @@ -142,7 +142,7 @@ export class Worker { return undefined; }; - const jestWorker = new JestWorker(workerPath, options); + const jestWorker = new Worker(workerPath, options); const inspectorSession = await setUpInspector(); jestWorker._inspectorSession = inspectorSession; diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index efa9b24b972f..9776fc3ca29e 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -213,7 +213,7 @@ export default class ChildProcessWorker implements WorkerInterface { this._onProcessEnd(error, null); }); - await new Promise((resolve, reject) => { + await new Promise((resolve, reject) => { this._options.inspector?.post('Debugger.pause', (err: Error) => { if (err === null) { resolve(); diff --git a/packages/jest-worker/src/workers/NodeThreadsWorker.ts b/packages/jest-worker/src/workers/NodeThreadsWorker.ts index 232c3bb19279..1e8aa52ac4e6 100644 --- a/packages/jest-worker/src/workers/NodeThreadsWorker.ts +++ b/packages/jest-worker/src/workers/NodeThreadsWorker.ts @@ -193,7 +193,7 @@ export default class ExperimentalWorker implements WorkerInterface { this._onProcessEnd(error, null); }); - await new Promise((resolve, reject) => { + await new Promise((resolve, reject) => { this._options.inspector?.post('Debugger.pause', (err: Error) => { if (err === null) { resolve(); From 391e8c0851c1ab4b687bff730b435d9163511ad7 Mon Sep 17 00:00:00 2001 From: Remi Date: Sat, 13 Feb 2021 01:58:01 +0100 Subject: [PATCH 18/20] fix(jest-worker): lint + increase tiemout --- e2e/__tests__/inspector.test.ts | 10 +-- .../src/__tests__/index.test.js | 66 +------------------ packages/jest-worker/README.md | 2 +- packages/jest-worker/src/index.ts | 2 +- .../src/workers/ChildProcessWorker.ts | 12 ++-- .../src/workers/NodeThreadsWorker.ts | 2 +- .../jest-worker/src/workers/processChild.ts | 1 + 7 files changed, 13 insertions(+), 82 deletions(-) diff --git a/e2e/__tests__/inspector.test.ts b/e2e/__tests__/inspector.test.ts index 448574dbd38c..c80c9c5e1721 100644 --- a/e2e/__tests__/inspector.test.ts +++ b/e2e/__tests__/inspector.test.ts @@ -35,12 +35,8 @@ test('fails a test which causes an infinite loop', () => { `, }); - const {exitCode, stdout, stderr} = runJest(DIR, ['--maxWorkers=2']); + const {exitCode, stderr} = runJest(DIR, ['--maxWorkers=2']); - console.log({exitCode, stdout, stderr}); - - const numberOfTestsPassed = (stderr.match(/\bPASS\b/g) || []).length; - - expect(numberOfTestsPassed).toBe(Object.keys(testFiles).length - 1); - expect(stderr).toContain('FAIL __tests__/inspector.test.js'); + expect(exitCode).toBe(1); + expect(stderr).toContain('Error: Test worker was unresponsive'); }); diff --git a/packages/jest-haste-map/src/__tests__/index.test.js b/packages/jest-haste-map/src/__tests__/index.test.js index bbd64e7a5769..7170aae37238 100644 --- a/packages/jest-haste-map/src/__tests__/index.test.js +++ b/packages/jest-haste-map/src/__tests__/index.test.js @@ -1225,71 +1225,7 @@ describe('HasteMap', () => { dependencyExtractor, hasteImplModulePath: undefined, maxWorkers: 4, - }) - .build() - .then(({__hasteMapForTest: data}) => { - expect(jestWorker.create.mock.calls.length).toBe(1); - - expect(mockWorker.mock.calls.length).toBe(5); - - expect(mockWorker.mock.calls).toEqual([ - [ - { - computeDependencies: true, - computeSha1: false, - dependencyExtractor, - filePath: path.join('/', 'project', 'fruits', 'Banana.js'), - hasteImplModulePath: undefined, - rootDir: path.join('/', 'project'), - }, - ], - [ - { - computeDependencies: true, - computeSha1: false, - dependencyExtractor, - filePath: path.join('/', 'project', 'fruits', 'Pear.js'), - hasteImplModulePath: undefined, - rootDir: path.join('/', 'project'), - }, - ], - [ - { - computeDependencies: true, - computeSha1: false, - dependencyExtractor, - filePath: path.join('/', 'project', 'fruits', 'Strawberry.js'), - hasteImplModulePath: undefined, - rootDir: path.join('/', 'project'), - }, - ], - [ - { - computeDependencies: true, - computeSha1: false, - dependencyExtractor, - filePath: path.join( - '/', - 'project', - 'fruits', - '__mocks__', - 'Pear.js', - ), - hasteImplModulePath: undefined, - rootDir: path.join('/', 'project'), - }, - ], - [ - { - computeDependencies: true, - computeSha1: false, - dependencyExtractor, - filePath: path.join('/', 'project', 'vegetables', 'Melon.js'), - hasteImplModulePath: undefined, - rootDir: path.join('/', 'project'), - }, - ], - ]); + }).build(); expect(jestWorker.mock.calls.length).toBe(1); diff --git a/packages/jest-worker/README.md b/packages/jest-worker/README.md index 69a9b0f70e5f..1e1cf6f7e65d 100644 --- a/packages/jest-worker/README.md +++ b/packages/jest-worker/README.md @@ -65,7 +65,7 @@ Amount of workers to spawn. Defaults to the number of CPUs minus 1. #### `workerHeartbeatTimeout: number` (optional) -Heartbeat timeout used to ping the parent process when child workers are alive. Defaults to 5000 ms. +Heartbeat timeout used to ping the parent process when child workers are alive. Defaults to 10000 ms. #### `maxRetries: number` (optional) diff --git a/packages/jest-worker/src/index.ts b/packages/jest-worker/src/index.ts index 1290b9abb8c1..6b152118f69b 100644 --- a/packages/jest-worker/src/index.ts +++ b/packages/jest-worker/src/index.ts @@ -90,7 +90,7 @@ export class Worker { numWorkers: this._options.numWorkers ?? Math.max(cpus().length - 1, 1), resourceLimits: this._options.resourceLimits ?? {}, setupArgs: this._options.setupArgs ?? [], - workerHeartbeatTimeout: this._options.workerHeartbeatTimeout ?? 5000, + workerHeartbeatTimeout: this._options.workerHeartbeatTimeout ?? 10000, }; if (this._options.WorkerPool) { diff --git a/packages/jest-worker/src/workers/ChildProcessWorker.ts b/packages/jest-worker/src/workers/ChildProcessWorker.ts index 9776fc3ca29e..008d1e62fcde 100644 --- a/packages/jest-worker/src/workers/ChildProcessWorker.ts +++ b/packages/jest-worker/src/workers/ChildProcessWorker.ts @@ -187,7 +187,7 @@ export default class ChildProcessWorker implements WorkerInterface { `Test worker was unresponsive for ${this._options.workerHeartbeatTimeout} milliseconds. There was an inspector connected so we were able to capture stack frames before it was terminated.`, ); this._options.inspector.on('Debugger.paused', (message: any) => { - const frames: ErrorFrame[] = []; + const frames: Array = []; const callFrames = message.params.callFrames.slice(0, 20); for (const callFrame of callFrames) { const loc = callFrame.location; @@ -243,10 +243,6 @@ export default class ChildProcessWorker implements WorkerInterface { break; case PARENT_MESSAGE_CLIENT_ERROR: - if (this._heartbeatTimeout) { - clearTimeout(this._heartbeatTimeout); - this.forceExit(); - } error = response[4]; if (error != null && typeof error === 'object') { @@ -336,8 +332,10 @@ export default class ChildProcessWorker implements WorkerInterface { () => this._child.kill('SIGKILL'), SIGKILL_DELAY, ); - this._exitPromise.then(() => clearTimeout(sigkillTimeout)); - clearTimeout(this._heartbeatTimeout); + this._exitPromise.then(() => { + clearTimeout(sigkillTimeout); + clearTimeout(this._heartbeatTimeout); + }); } getWorkerId(): number { diff --git a/packages/jest-worker/src/workers/NodeThreadsWorker.ts b/packages/jest-worker/src/workers/NodeThreadsWorker.ts index 1e8aa52ac4e6..39af57da37e2 100644 --- a/packages/jest-worker/src/workers/NodeThreadsWorker.ts +++ b/packages/jest-worker/src/workers/NodeThreadsWorker.ts @@ -167,7 +167,7 @@ export default class ExperimentalWorker implements WorkerInterface { `Test worker was unresponsive for ${this._options.workerHeartbeatTimeout} milliseconds. There was an inspector connected so we were able to capture stack frames before it was terminated.`, ); this._options.inspector.on('Debugger.paused', (message: any) => { - const frames: ErrorFrame[] = []; + const frames: Array = []; const callFrames = message.params.callFrames.slice(0, 20); for (const callFrame of callFrames) { const loc = callFrame.location; diff --git a/packages/jest-worker/src/workers/processChild.ts b/packages/jest-worker/src/workers/processChild.ts index 4b1c78a10314..d64f8d03cc3c 100644 --- a/packages/jest-worker/src/workers/processChild.ts +++ b/packages/jest-worker/src/workers/processChild.ts @@ -91,6 +91,7 @@ function reportInitializeError(error: Error) { } function reportError(error: Error, type: PARENT_MESSAGE_ERROR) { + clearInterval(monitorHeartbeat); if (!process || !process.send) { throw new Error('Child can only be used on a forked process'); } From baa0b3495c4dd247ae16f8954b4d7315ff8d5dda Mon Sep 17 00:00:00 2001 From: Remi Date: Tue, 16 Feb 2021 19:22:04 +0100 Subject: [PATCH 19/20] fix(jest-worker): inspector initialization --- packages/jest-haste-map/src/index.ts | 2 +- packages/jest-worker/src/index.ts | 12 +++++++----- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/jest-haste-map/src/index.ts b/packages/jest-haste-map/src/index.ts index 2e81a59c5635..91b189680af1 100644 --- a/packages/jest-haste-map/src/index.ts +++ b/packages/jest-haste-map/src/index.ts @@ -723,7 +723,7 @@ export default class HasteMap extends EventEmitter { this._worker = Promise.resolve({getSha1, worker}); } else { // @ts-expect-error: assignment of a worker with custom properties. - this._worker = Worker.create(require.resolve('./worker'), { + this._worker = await Worker.create(require.resolve('./worker'), { exposedMethods: ['getSha1', 'worker'], maxRetries: 3, numWorkers: this._options.maxWorkers, diff --git a/packages/jest-worker/src/index.ts b/packages/jest-worker/src/index.ts index 6b152118f69b..9dccfc93b782 100644 --- a/packages/jest-worker/src/index.ts +++ b/packages/jest-worker/src/index.ts @@ -78,14 +78,18 @@ export class Worker { private _workerPool: WorkerPoolInterface; private _inspectorSession: inspector.Session | undefined; - constructor(workerPath: string, options?: FarmOptions) { + constructor( + workerPath: string, + inspectorSession?: inspector.Session, + options?: FarmOptions, + ) { this._options = {...options}; this._ending = false; const workerPoolOptions: WorkerPoolOptions = { enableWorkerThreads: this._options.enableWorkerThreads ?? false, forkOptions: this._options.forkOptions ?? {}, - inspector: this._inspectorSession, + inspector: inspectorSession, maxRetries: this._options.maxRetries ?? 3, numWorkers: this._options.numWorkers ?? Math.max(cpus().length - 1, 1), resourceLimits: this._options.resourceLimits ?? {}, @@ -142,10 +146,8 @@ export class Worker { return undefined; }; - const jestWorker = new Worker(workerPath, options); const inspectorSession = await setUpInspector(); - - jestWorker._inspectorSession = inspectorSession; + const jestWorker = new Worker(workerPath, inspectorSession, options); return jestWorker; }; From 0c8f15868fd2e45a23b1d150845af75c1d3f6a0e Mon Sep 17 00:00:00 2001 From: Tim Seckinger Date: Mon, 5 Apr 2021 20:16:52 +0200 Subject: [PATCH 20/20] fix lint and cleanup --- ...nspector.test.ts => infinite-loop.test.ts} | 23 ++++++++----------- packages/jest-haste-map/src/index.ts | 4 ++-- 2 files changed, 11 insertions(+), 16 deletions(-) rename e2e/__tests__/{inspector.test.ts => infinite-loop.test.ts} (60%) diff --git a/e2e/__tests__/inspector.test.ts b/e2e/__tests__/infinite-loop.test.ts similarity index 60% rename from e2e/__tests__/inspector.test.ts rename to e2e/__tests__/infinite-loop.test.ts index c80c9c5e1721..a0b84db3fd77 100644 --- a/e2e/__tests__/inspector.test.ts +++ b/e2e/__tests__/infinite-loop.test.ts @@ -15,28 +15,23 @@ import runJest from '../runJest'; const DIR = path.resolve(tmpdir(), 'inspector'); -beforeEach(() => cleanup(DIR)); -afterAll(() => cleanup(DIR)); +afterEach(() => cleanup(DIR)); -test('fails a test which causes an infinite loop', () => { - const testFiles = { - ...generateTestFilesToForceUsingWorkers(), - 'package.json': `{ - "testEnvironment": "node" - }`, - }; +it('fails a test which causes an infinite loop', () => { + const testFiles = generateTestFilesToForceUsingWorkers(); writeFiles(DIR, { ...testFiles, '__tests__/inspector.test.js': ` - test('infinite loop error', () => { - while(true) {} - }); - `, + test('infinite loop error', () => { + while(true) {} + }); + `, + 'package.json': '{}', }); const {exitCode, stderr} = runJest(DIR, ['--maxWorkers=2']); expect(exitCode).toBe(1); - expect(stderr).toContain('Error: Test worker was unresponsive'); + expect(stderr).toMatch(/worker.+unresponsive/); }); diff --git a/packages/jest-haste-map/src/index.ts b/packages/jest-haste-map/src/index.ts index 91b189680af1..f8826999e287 100644 --- a/packages/jest-haste-map/src/index.ts +++ b/packages/jest-haste-map/src/index.ts @@ -727,11 +727,11 @@ export default class HasteMap extends EventEmitter { exposedMethods: ['getSha1', 'worker'], maxRetries: 3, numWorkers: this._options.maxWorkers, - }) as Promise; + }); } } - return this._worker; + return this._worker!; } private _crawl(hasteMap: InternalHasteMap) {