Skip to content

Commit

Permalink
fix(codegen): SIGINT handling was leading to zombie processes (#33269)
Browse files Browse the repository at this point in the history
  • Loading branch information
mxschmitt authored Oct 24, 2024
1 parent 0509eca commit a2dec8d
Show file tree
Hide file tree
Showing 7 changed files with 10 additions and 1 deletion.
2 changes: 2 additions & 0 deletions packages/playwright-core/src/cli/program.ts
Original file line number Diff line number Diff line change
Expand Up @@ -554,6 +554,7 @@ async function open(options: Options, url: string | undefined, language: string)
contextOptions,
device: options.device,
saveStorage: options.saveStorage,
handleSIGINT: false,
});
await openPage(context, url);
}
Expand All @@ -577,6 +578,7 @@ async function codegen(options: Options & { target: string, output?: string, tes
codegenMode: process.env.PW_RECORDER_IS_TRACE_VIEWER ? 'trace-events' : 'actions',
testIdAttributeName,
outputFile: outputFile ? path.resolve(outputFile) : undefined,
handleSIGINT: false,
});
await openPage(context, url);
}
Expand Down
1 change: 1 addition & 0 deletions packages/playwright-core/src/protocol/validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -976,6 +976,7 @@ scheme.BrowserContextEnableRecorderParams = tObject({
device: tOptional(tString),
saveStorage: tOptional(tString),
outputFile: tOptional(tString),
handleSIGINT: tOptional(tBoolean),
omitCallTracking: tOptional(tBoolean),
});
scheme.BrowserContextEnableRecorderResult = tOptional(tObject({}));
Expand Down
2 changes: 2 additions & 0 deletions packages/playwright-core/src/server/recorder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import { buildFullSelector } from '../utils/isomorphic/recorderUtils';
const recorderSymbol = Symbol('recorderSymbol');

export class Recorder implements InstrumentationListener, IRecorder {
readonly handleSIGINT: boolean | undefined;
private _context: BrowserContext;
private _mode: Mode;
private _highlightedSelector = '';
Expand Down Expand Up @@ -75,6 +76,7 @@ export class Recorder implements InstrumentationListener, IRecorder {

constructor(codegenMode: 'actions' | 'trace-events', context: BrowserContext, params: channels.BrowserContextEnableRecorderParams) {
this._mode = params.mode || 'none';
this.handleSIGINT = params.handleSIGINT;
this._contextRecorder = new ContextRecorder(codegenMode, context, params, {});
this._context = context;
this._omitCallTracking = !!params.omitCallTracking;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ export class RecorderApp extends EventEmitter implements IRecorderApp {
noDefaultViewport: true,
headless: !!process.env.PWTEST_CLI_HEADLESS || (isUnderTest() && !headed),
useWebSocket: isUnderTest(),
handleSIGINT: false,
handleSIGINT: recorder.handleSIGINT,
executablePath: inspectedContext._browser.options.isChromium ? inspectedContext._browser.options.customExecutablePath : undefined,
}
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import type { EventEmitter } from 'events';
export interface IRecorder {
setMode(mode: Mode): void;
mode(): Mode;
readonly handleSIGINT: boolean | undefined;
}

export interface IRecorderApp extends EventEmitter {
Expand Down
2 changes: 2 additions & 0 deletions packages/protocol/src/channels.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1777,6 +1777,7 @@ export type BrowserContextEnableRecorderParams = {
device?: string,
saveStorage?: string,
outputFile?: string,
handleSIGINT?: boolean,
omitCallTracking?: boolean,
};
export type BrowserContextEnableRecorderOptions = {
Expand All @@ -1790,6 +1791,7 @@ export type BrowserContextEnableRecorderOptions = {
device?: string,
saveStorage?: string,
outputFile?: string,
handleSIGINT?: boolean,
omitCallTracking?: boolean,
};
export type BrowserContextEnableRecorderResult = void;
Expand Down
1 change: 1 addition & 0 deletions packages/protocol/src/protocol.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1208,6 +1208,7 @@ BrowserContext:
device: string?
saveStorage: string?
outputFile: string?
handleSIGINT: boolean?
omitCallTracking: boolean?

newCDPSession:
Expand Down

0 comments on commit a2dec8d

Please sign in to comment.