Filip/playwright
1
0
Fork 0
Code Issues Pull requests Actions Packages Projects Releases Wiki Activity

fix(tracing): clear recording state at the end of tracing.stop (#8120)

Browse source 
This ensures that any tracing operations can access the recording state.
When stopping, we await for all operations to finish and then clear the state.
To avoid reentrancy, a new flag `isStopping` is introduced.
This commit is contained in:
Dmitry Gozman 2021-08-10 17:36:06 -07:00 committed by GitHub
parent 4975f4179e
commit 708fa43f03
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
1 changed files with 17 additions and 17 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

34
src/server/trace/recorder/tracing.ts
View file

@ -55,6 +55,7 @@ export class Tracing implements InstrumentationListener {
private _context: BrowserContext; private _context: BrowserContext;
private _resourcesDir: string; private _resourcesDir: string;
private _recording: RecordingState | undefined; private _recording: RecordingState | undefined;
private _isStopping = false;
private _tracesDir: string; private _tracesDir: string;
constructor(context: BrowserContext) { constructor(context: BrowserContext) {
@ -65,6 +66,8 @@ export class Tracing implements InstrumentationListener {
} }
async start(options: TracerOptions): Promise<void> { async start(options: TracerOptions): Promise<void> {
if (this._isStopping)
throw new Error('Cannot start tracing while stopping');
// context + page must be the first events added, this method can't have awaits before them. // context + page must be the first events added, this method can't have awaits before them.
const state = this._recording; const state = this._recording;
@ -130,14 +133,16 @@ export class Tracing implements InstrumentationListener {
} }
async stop(): Promise<void> { async stop(): Promise<void> {
if (!this._recording) if (!this._recording || this._isStopping)
return; return;
this._recording = undefined; this._isStopping = true;
this._context.instrumentation.removeListener(this); this._context.instrumentation.removeListener(this);
this._stopScreencast(); this._stopScreencast();
await this._snapshotter.stop(); await this._snapshotter.stop();
// Ensure all writes are finished. // Ensure all writes are finished.
await this._appendEventChain; await this._appendEventChain;
this._recording = undefined;
this._isStopping = false;
} }
async dispose() { async dispose() {
@ -145,9 +150,6 @@ export class Tracing implements InstrumentationListener {
} }
async export(): Promise<Artifact> { async export(): Promise<Artifact> {
if (!this._recording)
throw new Error('Must start tracing before exporting');
for (const { sdkObject, metadata, beforeSnapshot, actionSnapshot, afterSnapshot } of this._pendingCalls.values()) { for (const { sdkObject, metadata, beforeSnapshot, actionSnapshot, afterSnapshot } of this._pendingCalls.values()) {
await Promise.all([beforeSnapshot, actionSnapshot, afterSnapshot]); await Promise.all([beforeSnapshot, actionSnapshot, afterSnapshot]);
let callMetadata = metadata; let callMetadata = metadata;
@ -161,19 +163,19 @@ export class Tracing implements InstrumentationListener {
await this.onAfterCall(sdkObject, callMetadata); await this.onAfterCall(sdkObject, callMetadata);
} }
if (!this._recording)
throw new Error('Must start tracing before exporting');
// Chain the export operation against write operations, // Chain the export operation against write operations,
// so that neither trace file nor sha1s change during the export. // so that neither trace file nor sha1s change during the export.
return await this._appendTraceOperation(async () => { return await this._appendTraceOperation(async () => {
if (!this._recording)
throw new Error('Must start tracing before exporting');
await this._snapshotter.checkpoint(); await this._snapshotter.checkpoint();
const resetIndex = this._recording.lastReset; const recording = this._recording!;
let state = this._recording; let state = recording;
// Make a filtered trace if needed. // Make a filtered trace if needed.
if (resetIndex) if (recording.lastReset)
state = await this._filterTrace(this._recording, resetIndex); state = await this._filterTrace(recording, recording.lastReset);
const zipFile = new yazl.ZipFile(); const zipFile = new yazl.ZipFile();
const failedPromise = new Promise<Artifact>((_, reject) => (zipFile as any as EventEmitter).on('error', reject)); const failedPromise = new Promise<Artifact>((_, reject) => (zipFile as any as EventEmitter).on('error', reject));
@ -192,7 +194,7 @@ export class Tracing implements InstrumentationListener {
}); });
return Promise.race([failedPromise, succeededPromise]).finally(async () => { return Promise.race([failedPromise, succeededPromise]).finally(async () => {
// Remove the filtered trace. // Remove the filtered trace.
if (resetIndex) if (recording.lastReset)
await fs.promises.unlink(state.traceFile).catch(() => {}); await fs.promises.unlink(state.traceFile).catch(() => {});
}); });
}); });
@ -313,10 +315,8 @@ export class Tracing implements InstrumentationListener {
private _appendTraceEvent(event: any) { private _appendTraceEvent(event: any) {
// Serialize all writes to the trace file. // Serialize all writes to the trace file.
this._appendTraceOperation(async () => { this._appendTraceOperation(async () => {
if (!this._recording) visitSha1s(event, this._recording!.sha1s);
return; await fs.promises.appendFile(this._recording!.traceFile, JSON.stringify(event) + '\n');
visitSha1s(event, this._recording.sha1s);
await fs.promises.appendFile(this._recording.traceFile, JSON.stringify(event) + '\n');
}); });
} }