fix(mcp): check tracing state before stopping in browser_stop_tracing#41040
Open
SebTardif wants to merge 1 commit into
Open
fix(mcp): check tracing state before stopping in browser_stop_tracing#41040SebTardif wants to merge 1 commit into
SebTardif wants to merge 1 commit into
Conversation
browser_stop_tracing called tracing.stop() unconditionally before checking the traceLegendSymbol. If tracing was started via config (saveTrace: true) rather than via browser_start_tracing, the tool killed the config-initiated trace, then threw 'Tracing is not started'. The destructive side effect happened before the guard. Move the traceLegendSymbol check before tracing.stop() so the guard is evaluated first and the active trace is preserved on the error path. Bug introduced in microsoft#39646 (2026-03-12). Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>
pavelfeldman
approved these changes
May 29, 2026
Contributor
Test results for "MCP"7220 passed, 1113 skipped Merge workflow run. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
browser_stop_tracingcallsbrowserContext.tracing.stop()unconditionally before checking thetraceLegendSymbolguard. When tracing was started via config (saveTrace: true) rather than via thebrowser_start_tracingtool, the symbol is not set. The tool kills the config-initiated trace recording, then throws "Tracing is not started." The destructive side effect fires before the guard check.Fix
Move the
traceLegendSymbolcheck beforetracing.stop()so the guard is evaluated first and the active trace is preserved on the error path.Trigger scenario
saveTrace = truein configcontext.ts:329-335(notraceLegendSymbolset)browser_stop_tracingtracing.stop()kills the config trace, then the symbol check throws "Tracing is not started"The config-initiated trace has its own cleanup via the disposable registered at
context.ts:336-340, so the tool should not interfere with it.Test
Added a test that starts with
saveTrace: true, callsbrowser_stop_tracing(expects error), then verifies the config trace is still active by confirmingbrowser_start_tracingfails with "Tracing has been already started."Bug introduced in #39646 (2026-03-12).