Skip to content

Fix Chrome kill failure on Windows 11 by adding graceful error handling#111

Merged
lirantal merged 3 commits intomainfrom
copilot/fix-110
Sep 4, 2025
Merged

Fix Chrome kill failure on Windows 11 by adding graceful error handling#111
lirantal merged 3 commits intomainfrom
copilot/fix-110

Conversation

Copy link
Contributor

Copilot AI commented Sep 2, 2025

User description

Fixed an issue where the application would crash on Windows 11 when trying to kill the Chrome process during cleanup. The error occurred when Chrome had already exited before the kill command was executed, causing taskkill to fail with:

Error: Chrome could not be killed Command failed: taskkill /pid 15132 /T /F
错误: 没有找到进程 "15132"。

Root Cause

The chrome-launcher dependency uses taskkill /pid ${pid} /T /F on Windows to terminate Chrome processes. However, when Chrome exits on its own (due to crashes, user intervention, or normal completion), the PID becomes invalid, causing the kill command to fail and bubble up as an unhandled error.

Solution

Added graceful error handling around chromeInstance.kill() in src/Audit.js. When the kill operation fails, the error is caught and logged as a debug message instead of crashing the application. This approach recognizes that if Chrome is already dead, that's actually the desired outcome.

Changes Made

  • src/Audit.js: Wrapped chromeInstance.kill() in a try-catch block with debug logging
  • __tests__/Audit.test.js: Added comprehensive test coverage for the error handling scenario and improved existing test mocks

The fix ensures that website vulnerability scans complete successfully even when Chrome cleanup encounters issues on Windows 11, while maintaining full backward compatibility and test coverage.

Fixes #110.


💡 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.


PR Type

Bug fix


Description

  • Add graceful error handling for Chrome process termination failures

  • Prevent application crashes when Chrome exits before kill command

  • Improve test coverage with Chrome kill error scenarios

  • Fix Windows 11 specific taskkill command failures


Diagram Walkthrough

flowchart LR
  A["Chrome Launch"] --> B["Lighthouse Audit"]
  B --> C["Chrome Kill Attempt"]
  C --> D{"Kill Success?"}
  D -->|Yes| E["Clean Exit"]
  D -->|No| F["Log Debug Message"]
  F --> E
Loading

File Walkthrough

Relevant files
Bug fix
Audit.js
Add graceful Chrome kill error handling                                   

src/Audit.js

  • Wrap chromeInstance.kill() in try-catch block
  • Add debug logging for Chrome kill failures
  • Prevent crashes when Chrome process already exited
+6/-1     
Tests
Audit.test.js
Enhance test coverage for Chrome kill errors                         

tests/Audit.test.js

  • Extract mockKill function for better test control
  • Add beforeEach setup with mock resets
  • Create test case for Chrome kill error scenarios
  • Fix lighthouse mock call assertion index
+30/-4   
Dependencies
npm-shrinkwrap.json
Update package lock file metadata                                               

npm-shrinkwrap.json

  • Add package name field to root package definition
  • Add binary entries for sshpk package
+6/-0     

Copy link
Contributor Author

Copilot AI commented Sep 2, 2025

@lirantal 👋 This repository doesn't have Copilot instructions. With Copilot instructions, I can understand the repository better, work faster and produce higher quality PRs.

I can generate a .github/copilot-instructions.md file for you automatically. Click here to open a pre-filled issue and assign it to me. I'll write the instructions, and then tag you for review.

Copilot AI and others added 2 commits September 2, 2025 12:27
Co-authored-by: lirantal <316371+lirantal@users.noreply.github.com>
Co-authored-by: lirantal <316371+lirantal@users.noreply.github.com>
Copilot AI changed the title [WIP] fail to kill chrome task Fix Chrome kill failure on Windows 11 by adding graceful error handling Sep 2, 2025
Copilot AI requested a review from lirantal September 2, 2025 12:31
@lirantal lirantal marked this pull request as ready for review September 4, 2025 06:48
@qodo-free-for-open-source-projects

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ✅

110 - PR Code Verified

Compliant requirements:

  • Prevent crashes when Chrome kill command fails on Windows (e.g., PID not found).
  • Gracefully handle cases where Chrome has already exited and taskkill returns an error.
  • Ensure normal scan flow completes despite kill failures.
  • Add or adjust tests to cover the error scenario.

Requires further human verification:

  • Manual validation on Windows 11 environment to confirm behavior with real Chrome processes and taskkill behavior.
⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Logging Level

Consider whether debug logging is sufficient for visibility when Chrome kill fails, or if a warn-level log is preferable behind a verbose flag to aid users diagnosing cleanup issues.

try {
  await chromeInstance.kill()
} catch (error) {
  // Chrome process may have already exited, which is the desired outcome
  debug(`Chrome kill warning: ${error.message}`)
}
Test Isolation

Ensure lighthouse mock and chrome-launcher mock are fully reset between tests; using jest.resetModules() might be necessary if module state leakage occurs beyond function mocks.

beforeEach(() => {
  jest.clearAllMocks()
  mockKill.mockResolvedValue(null)
  lighthouse.mockResolvedValue({
    lhr: {
      audits: {}
    }
  })
})

@lirantal lirantal merged commit 9c2cf88 into main Sep 4, 2025
23 checks passed
@qodo-free-for-open-source-projects

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
High-level
Ensure robust Chrome cleanup

Move chromeInstance.kill() into a finally block so it always runs whether
Lighthouse succeeds or throws, preventing orphaned Chrome processes on failures.
In the catch, only treat "process not found" (already exited) as benign and
surface a warning (or retry) for other errors instead of swallowing all failures
behind a debug log. This keeps the Windows 11 fix while avoiding hidden
termination issues across platforms.

Examples:

src/Audit.js [95-108]
      spinner2.succeed(
        `${chalk.green(
          `Auditing completed in ${((new Date() - time) / 1000).toFixed(2)} seconds!`
        )}`
      )

    try {
      await chromeInstance.kill()
    } catch (error) {
      // Chrome process may have already exited, which is the desired outcome

 ... (clipped 4 lines)

Solution Walkthrough:

Before:

async function scanUrl(url, options) {
  const chromeInstance = await chromeLauncher.launch(...);
  let scanResult;
  try {
    scanResult = await lighthouse(...);
  } catch (error) {
    // ... spinner failure
    return null; // <- chromeInstance.kill() is never called
  }

  try {
    await chromeInstance.kill();
  } catch (error) {
    // Swallows any error, not just "process not found"
    debug(`Chrome kill warning: ${error.message}`);
  }
  return scanResult;
}

After:

async function scanUrl(url, options) {
  const chromeInstance = await chromeLauncher.launch(...);
  try {
    const scanResult = await lighthouse(...);
    return scanResult;
  } catch (error) {
    // ... spinner failure
    return null;
  } finally {
    try {
      await chromeInstance.kill();
    } catch (error) {
      // Only ignore the specific error for a non-existent process
      if (error.message.includes('not find task') || error.message.includes('No such process')) {
        debug(`Chrome process already exited: ${error.message}`);
      } else {
        // Log other kill errors more seriously or re-throw
        console.warn(`Failed to kill Chrome: ${error.message}`);
      }
    }
  }
}
Suggestion importance[1-10]: 9

__

Why: This suggestion correctly identifies a potential resource leak where a failed audit would skip chromeInstance.kill(), and it improves the PR's error handling to be more precise, thus increasing the overall robustness of the application.

High
  • More

@github-actions
Copy link

github-actions bot commented Sep 4, 2025

🎉 This PR is included in version 1.14.11 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fail to kill chrome task

2 participants