Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Jan 16, 2026

Description

Addresses feedback from regarding missing error handling when initializing the run-level profiler via RunOptions.

Changes:

  • Wrapped profiler initialization in try-catch to handle exceptions from Initialize() and StartProfiling()
  • Added validation after StartProfiling() to verify IsEnabled() returns true
  • Reset run_profiler and log warnings on failure, allowing execution to continue without profiling

Before:

if (run_options.enable_profiling && !session_profiler_.IsEnabled()) {
  run_profiler.emplace();
  run_profiler->Initialize(session_logger_);  // Can throw
  // ... setup code ...
  run_profiler->StartProfiling(profile_file);  // Can fail silently
}
// Later: assumes run_profiler is valid if set

After:

if (run_options.enable_profiling && !session_profiler_.IsEnabled()) {
  try {
    run_profiler.emplace();
    run_profiler->Initialize(session_logger_);
    // ... setup code ...
    run_profiler->StartProfiling(profile_file);
    
    if (!run_profiler->IsEnabled()) {
      LOGS(*session_logger_, WARNING) << "Failed to enable run-level profiler...";
      run_profiler.reset();
    }
  } catch (const std::exception& ex) {
    LOGS(*session_logger_, WARNING) << "Failed to initialize: " << ex.what();
    run_profiler.reset();
  }
}

Motivation and Context

Without error handling, profiler initialization failures (e.g., null logger, file open failures) would result in undefined behavior when dereferencing run_profiler later in the execution path. The fix ensures graceful degradation when profiling cannot be enabled.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI changed the title [WIP] Address feedback on enable profiling implementation Add error handling for run-level profiler initialization Jan 16, 2026
Copilot AI requested a review from xiaofeihan1 January 16, 2026 02:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants