Skip to content

Improve doctor.mjs Playwright browser check with distinct failure modes#9

Closed
RajjjAryan with Copilot wants to merge 2 commits into
mainfrom
copilot/improve-doctor-check-playwright-installation
Closed

Improve doctor.mjs Playwright browser check with distinct failure modes#9
RajjjAryan with Copilot wants to merge 2 commits into
mainfrom
copilot/improve-doctor-check-playwright-installation

Conversation

Copilot AI commented Apr 9, 2026

Copy link
Copy Markdown

checkPlaywright() treated "playwright package missing" and "chromium binary missing" as the same failure with the same fix instruction, which is misleading.

  • Separate failure detection: Split the single try/catch into two stages — package import vs binary existence — with targeted fix instructions (npm install vs npx playwright install chromium)
  • Show binary path on success: Include executablePath() in the pass label for debuggability
✓ Playwright Chromium browser installed (/home/runner/.cache/ms-playwright/chromium-1217/chrome-linux64/chrome)

✗ Playwright Chromium browser not installed
  → Run: npx playwright install chromium

✗ Playwright package not available
  → Run: npm install

Copilot AI linked an issue Apr 9, 2026 that may be closed by this pull request
2 tasks
Copilot AI changed the title [WIP] Improve doctor.mjs to check Playwright browser installation Improve doctor.mjs Playwright browser check with distinct failure modes Apr 9, 2026
Copilot AI requested a review from RajjjAryan April 9, 2026 20:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refines doctor.mjs’s Playwright prerequisite check so it distinguishes between a missing Playwright package and a missing Chromium browser binary, and improves debuggability by printing the resolved Chromium executable path on success.

Changes:

  • Split checkPlaywright() into two stages: Playwright package import vs Chromium binary presence.
  • Add targeted fix instructions for each failure mode (npm install vs npx playwright install chromium).
  • Include chromium.executablePath() in the success label.

Comment thread doctor.mjs
Comment on lines +50 to +54
} catch {
return {
pass: false,
label: 'Playwright package not available',
fix: 'Run: npm install',

Copilot AI Apr 10, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import stage uses a bare catch and always reports "Playwright package not available" with the generic npm install fix. This will misdiagnose other import-time failures (e.g., corrupted install, unsupported platform/architecture, or other runtime errors). Capture the thrown error and only treat it as "package not available" when it’s a module-not-found case (e.g., err.code === 'ERR_MODULE_NOT_FOUND' / message indicates missing playwright); otherwise surface a distinct failure label (ideally including err.message) so users aren’t sent to the wrong fix.

Suggested change
} catch {
return {
pass: false,
label: 'Playwright package not available',
fix: 'Run: npm install',
} catch (err) {
const errCode = err && typeof err === 'object' ? err.code : undefined;
const errMessage = err instanceof Error ? err.message : String(err);
const isModuleNotFound =
errCode === 'ERR_MODULE_NOT_FOUND' ||
/cannot find package ['"]playwright['"]/i.test(errMessage) ||
/cannot find module ['"]playwright['"]/i.test(errMessage) ||
/module ['"]playwright['"] was not found/i.test(errMessage);
if (isModuleNotFound) {
return {
pass: false,
label: 'Playwright package not available',
fix: 'Run: npm install',
};
}
return {
pass: false,
label: `Playwright import failed: ${errMessage}`,

Copilot uses AI. Check for mistakes.
Comment thread doctor.mjs
Comment on lines 59 to 74
try {
const { chromium } = await import('playwright');
const execPath = chromium.executablePath();
if (existsSync(execPath)) {
return { pass: true, label: 'Playwright chromium installed' };
return { pass: true, label: `Playwright Chromium browser installed (${execPath})` };
}
return {
pass: false,
label: 'Playwright chromium not installed',
label: 'Playwright Chromium browser not installed',
fix: 'Run: npx playwright install chromium',
};
} catch {
return {
pass: false,
label: 'Playwright chromium not installed',
label: 'Playwright Chromium browser not installed',
fix: 'Run: npx playwright install chromium',
};

Copilot AI Apr 10, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Chromium check’s catch treats any exception as "browser not installed". If chromium.executablePath() throws for reasons other than a missing browser (e.g., Playwright internal error), the output will point to npx playwright install chromium even though that may not resolve it. Capture the error and report a separate failure mode (or include err.message) so this stage only claims "not installed" when the binary is actually absent.

Copilot uses AI. Check for mistakes.
@RajjjAryan

Copy link
Copy Markdown
Owner

Closing — superseded by #31 which was merged to main and includes a more comprehensive fix (Playwright browser launch test + YAML validation for profile.yml and portals.yml).

Thank you for the contribution! 🙏

@RajjjAryan RajjjAryan closed this Apr 10, 2026
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.

Improve doctor.mjs to check Playwright browser installation

3 participants