From 1057712025b50daac2f55554e3a11bb919a0ca3a Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Wed, 23 Oct 2024 17:21:25 +0200 Subject: [PATCH 1/9] cri - use `--headless=old` as default. Chrome v128 changed the default from --headless=old to --headless=new in 2024-08. Old headless mode was effectively a separate browser render, and while more performant did not share the same browser implementation as headful Chrome. New headless mode will likely be useful to some, but in Quarto use cases like printing to PDF or screenshoting, we need more work to move to the new mode. We'll use `--headless=old` as the default for now until the new mode is more stable, or until we really pin a version as default to be used. This is also impacting in chromote and pagedown R packages and we could keep syncing with them. --- src/core/cri/cri.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/core/cri/cri.ts b/src/core/cri/cri.ts index e35b375f0c3..2b7b4bd15df 100644 --- a/src/core/cri/cri.ts +++ b/src/core/cri/cri.ts @@ -80,7 +80,15 @@ export async function criClient(appPath?: string, port?: number) { const cmd = [ app, - "--headless", + // TODO: Chrome v128 changed the default from --headless=old to --headless=new + // in 2024-08. Old headless mode was effectively a separate browser render, + // and while more performant did not share the same browser implementation as + // headful Chrome. New headless mode will likely be useful to some, but in Quarto use cases + // like printing to PDF or screenshoting, we need more work to + // move to the new mode. We'll use `--headless=old` as the default for now + // until the new mode is more stable, or until we really pin a version as default to be used. + // This is also impacting in chromote and pagedown R packages and we could keep syncing with them. + "--headless=old", "--no-sandbox", "--disable-gpu", "--renderer-process-limit=1", From f0403e1e5ef0571c5f8a358a95d45911a6191911 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Wed, 23 Oct 2024 18:15:10 +0200 Subject: [PATCH 2/9] cri - Allow user to set the headless through an env var --- src/core/cri/cri.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/core/cri/cri.ts b/src/core/cri/cri.ts index 2b7b4bd15df..2d0858d5b1d 100644 --- a/src/core/cri/cri.ts +++ b/src/core/cri/cri.ts @@ -14,6 +14,7 @@ import { findOpenPort } from "../port.ts"; import { getNamedLifetime, ObjectWithLifetime } from "../lifetimes.ts"; import { sleep } from "../async.ts"; import { InternalError } from "../lib/error.ts"; +import { getenv } from "../env.ts"; import { kRenderFileLifetime } from "../../config/constants.ts"; async function waitForServer(port: number, timeout = 3000) { @@ -78,6 +79,9 @@ export async function criClient(appPath?: string, port?: number) { } const app: string = appPath || await getBrowserExecutablePath(); + // Allow to adapt the headless mode depending on the Chrome version + const headlessMode = getenv("QUARTO_CHROME_HEADLESS_MODE", "old"); + const cmd = [ app, // TODO: Chrome v128 changed the default from --headless=old to --headless=new @@ -88,7 +92,7 @@ export async function criClient(appPath?: string, port?: number) { // move to the new mode. We'll use `--headless=old` as the default for now // until the new mode is more stable, or until we really pin a version as default to be used. // This is also impacting in chromote and pagedown R packages and we could keep syncing with them. - "--headless=old", + `--headless${headlessMode ? "=" + headlessMode : ""}`, "--no-sandbox", "--disable-gpu", "--renderer-process-limit=1", From 513f37151286fd7fa12149a555bd7463df6f8840 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Wed, 23 Oct 2024 22:37:06 +0200 Subject: [PATCH 3/9] cri - Allow to specify a chrome to use with Quarto using an env var `QUARTO_CHROMIUM` closes #10170 --- src/core/puppeteer.ts | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/core/puppeteer.ts b/src/core/puppeteer.ts index 6a1934ba89a..3b2625ac608 100644 --- a/src/core/puppeteer.ts +++ b/src/core/puppeteer.ts @@ -201,6 +201,12 @@ export async function withHeadlessBrowser( async function findChrome(): Promise { let path; + // First check env var and use this path if specified + const envPath = Deno.env.get("QUARTO_CHROMIUM"); + if (envPath && safeExistsSync(envPath)) { + return envPath; + } + // Otherwise, try to find the path based on OS. if (Deno.build.os === "darwin") { const programs = [ "/Applications/Google Chrome.app/Contents/MacOS/Google Chrome", From 1045f42aac3edf8a2d78ffa8752c6e28faf52764 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Thu, 24 Oct 2024 15:27:32 +0200 Subject: [PATCH 4/9] cri - Add debug info if error --- src/core/cri/cri.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/core/cri/cri.ts b/src/core/cri/cri.ts index 2d0858d5b1d..16cfae06ede 100644 --- a/src/core/cri/cri.ts +++ b/src/core/cri/cri.ts @@ -16,6 +16,7 @@ import { sleep } from "../async.ts"; import { InternalError } from "../lib/error.ts"; import { getenv } from "../env.ts"; import { kRenderFileLifetime } from "../../config/constants.ts"; +import { debug } from "../../deno_ral/log.ts"; async function waitForServer(port: number, timeout = 3000) { const interval = 50; @@ -104,6 +105,8 @@ export async function criClient(appPath?: string, port?: number) { let msg = "Couldn't find open server."; // Printing more error information if chrome process errored if (!(await browser.status()).success) { + debug(`[CHROMIUM path] : ${app}`); + debug(`[CHROMIUM cmd] : ${cmd}`); const rawError = await browser.stderrOutput(); const errorString = new TextDecoder().decode(rawError); msg = msg + "\n" + `Chrome process error: ${errorString}`; From ae33f4b4789a4a9aa67eac940168f93fb605afa7 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Thu, 24 Oct 2024 15:27:55 +0200 Subject: [PATCH 5/9] cri - Use CHROMIUM for env var also for headless setting --- src/core/cri/cri.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/cri/cri.ts b/src/core/cri/cri.ts index 16cfae06ede..0e5957b5ed7 100644 --- a/src/core/cri/cri.ts +++ b/src/core/cri/cri.ts @@ -81,7 +81,7 @@ export async function criClient(appPath?: string, port?: number) { const app: string = appPath || await getBrowserExecutablePath(); // Allow to adapt the headless mode depending on the Chrome version - const headlessMode = getenv("QUARTO_CHROME_HEADLESS_MODE", "old"); + const headlessMode = getenv("QUARTO_CHROMIUM_HEADLESS_MODE", "old"); const cmd = [ app, From a2806f561fb3f27d9c44e3f33f568ed447e63b33 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Thu, 24 Oct 2024 17:42:20 +0200 Subject: [PATCH 6/9] Add to changelog [skip ci] --- news/changelog-1.6.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/news/changelog-1.6.md b/news/changelog-1.6.md index bf90919d293..65a5998864a 100644 --- a/news/changelog-1.6.md +++ b/news/changelog-1.6.md @@ -101,6 +101,11 @@ All changes included in 1.6: - ([#9134](https://github.com/quarto-dev/quarto-cli/issues/9134)): Add proper fix for `multiprocessing` in notebooks with the Python kernel. +## Chromium support + +- ([#11135](https://github.com/quarto-dev/quarto-cli/issues/11135)): Use `--headless=old` mode for Chromium to avoid recent issues with the new `--headless` mode. Setting `--headless=new` can be configured with `QUARTO_CHROMIUM_HEADLESS_MODE=new` environment variable, however it is not recommended new headless mode seems to be unstable. Only use to be unblocked of a situation (like `QUARTO_CHROMIUM_HEADLESS_MODE=""` if you use a old chrome version somehow). +- ([#10170](https://github.com/quarto-dev/quarto-cli/issues/10170)): Quarto should find chrome executable automatically on most OS. If this is does not find it, or a specific version is needed, set `QUARTO_CHROMIUM` environment variable to the executable path. + ## Other Fixes and Improvements - Upgrade `mermaidjs` to 11.2.0. From babee58ca3d563fb3bb715e5c553ef341bb491e9 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Thu, 24 Oct 2024 17:53:14 +0200 Subject: [PATCH 7/9] cri - use `none` for no flag value as `""` will be undefined in Deno at least for Windows --- news/changelog-1.6.md | 2 +- src/core/cri/cri.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/news/changelog-1.6.md b/news/changelog-1.6.md index 65a5998864a..053dbca6c31 100644 --- a/news/changelog-1.6.md +++ b/news/changelog-1.6.md @@ -103,7 +103,7 @@ All changes included in 1.6: ## Chromium support -- ([#11135](https://github.com/quarto-dev/quarto-cli/issues/11135)): Use `--headless=old` mode for Chromium to avoid recent issues with the new `--headless` mode. Setting `--headless=new` can be configured with `QUARTO_CHROMIUM_HEADLESS_MODE=new` environment variable, however it is not recommended new headless mode seems to be unstable. Only use to be unblocked of a situation (like `QUARTO_CHROMIUM_HEADLESS_MODE=""` if you use a old chrome version somehow). +- ([#11135](https://github.com/quarto-dev/quarto-cli/issues/11135)): Use `--headless=old` mode for Chromium to avoid recent issues with the new `--headless` mode. Setting `--headless=new` can be configured with `QUARTO_CHROMIUM_HEADLESS_MODE=new` environment variable, however it is not recommended new headless mode seems to be unstable. Only use to be unblocked of a situation (like `QUARTO_CHROMIUM_HEADLESS_MODE="none"` if you use a old chrome version somehow that don't support `--headless=old`). - ([#10170](https://github.com/quarto-dev/quarto-cli/issues/10170)): Quarto should find chrome executable automatically on most OS. If this is does not find it, or a specific version is needed, set `QUARTO_CHROMIUM` environment variable to the executable path. ## Other Fixes and Improvements diff --git a/src/core/cri/cri.ts b/src/core/cri/cri.ts index 0e5957b5ed7..478fe9224ff 100644 --- a/src/core/cri/cri.ts +++ b/src/core/cri/cri.ts @@ -93,7 +93,7 @@ export async function criClient(appPath?: string, port?: number) { // move to the new mode. We'll use `--headless=old` as the default for now // until the new mode is more stable, or until we really pin a version as default to be used. // This is also impacting in chromote and pagedown R packages and we could keep syncing with them. - `--headless${headlessMode ? "=" + headlessMode : ""}`, + `--headless${headlessMode == "none" ? "" : "=" + headlessMode}`, "--no-sandbox", "--disable-gpu", "--renderer-process-limit=1", From d60c4870513e3959061ecd4fa02f293015ce4759 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Thu, 24 Oct 2024 18:20:17 +0200 Subject: [PATCH 8/9] fix typo in changelog [skip ci] --- news/changelog-1.6.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/news/changelog-1.6.md b/news/changelog-1.6.md index 053dbca6c31..404633a135d 100644 --- a/news/changelog-1.6.md +++ b/news/changelog-1.6.md @@ -103,7 +103,7 @@ All changes included in 1.6: ## Chromium support -- ([#11135](https://github.com/quarto-dev/quarto-cli/issues/11135)): Use `--headless=old` mode for Chromium to avoid recent issues with the new `--headless` mode. Setting `--headless=new` can be configured with `QUARTO_CHROMIUM_HEADLESS_MODE=new` environment variable, however it is not recommended new headless mode seems to be unstable. Only use to be unblocked of a situation (like `QUARTO_CHROMIUM_HEADLESS_MODE="none"` if you use a old chrome version somehow that don't support `--headless=old`). +- ([#11135](https://github.com/quarto-dev/quarto-cli/issues/11135)): Use `--headless=old` mode for Chromium to avoid recent issues with the new `--headless` mode. Setting `--headless=new` can be configured with `QUARTO_CHROMIUM_HEADLESS_MODE=new` environment variable, however it is not recommended new headless mode seems to be unstable. Only use to be unblocked of a situation (like `QUARTO_CHROMIUM_HEADLESS_MODE="none"` if you use an old chrome version somehow that don't support `--headless=old`). - ([#10170](https://github.com/quarto-dev/quarto-cli/issues/10170)): Quarto should find chrome executable automatically on most OS. If this is does not find it, or a specific version is needed, set `QUARTO_CHROMIUM` environment variable to the executable path. ## Other Fixes and Improvements From d629028564d854444803ba6587b438e4294a37a0 Mon Sep 17 00:00:00 2001 From: Christophe Dervieux Date: Thu, 24 Oct 2024 18:28:30 +0200 Subject: [PATCH 9/9] cri - add debug() log message to help know how chromium is detected --- src/core/puppeteer.ts | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/core/puppeteer.ts b/src/core/puppeteer.ts index 3b2625ac608..f39b212443d 100644 --- a/src/core/puppeteer.ts +++ b/src/core/puppeteer.ts @@ -6,7 +6,7 @@ import { readRegistryKey } from "./windows.ts"; import { safeExistsSync, which } from "./path.ts"; -import { error, info } from "../deno_ral/log.ts"; +import { debug, error, info } from "../deno_ral/log.ts"; import { existsSync } from "../deno_ral/fs.ts"; import { UnreachableError } from "./lib/error.ts"; import { quartoDataDir } from "./appdirs.ts"; @@ -204,6 +204,8 @@ async function findChrome(): Promise { // First check env var and use this path if specified const envPath = Deno.env.get("QUARTO_CHROMIUM"); if (envPath && safeExistsSync(envPath)) { + debug("[CHROMIUM] Using path specified in QUARTO_CHROMIUM"); + debug(`[CHROMIUM] Path: ${envPath}`); return envPath; } // Otherwise, try to find the path based on OS. @@ -212,7 +214,7 @@ async function findChrome(): Promise { "/Applications/Google Chrome.app/Contents/MacOS/Google Chrome", "/Applications/Microsoft Edge.app/Contents/MacOS/Microsoft Edge", ]; - return programs.find(safeExistsSync); + path = programs.find(safeExistsSync); } else if (Deno.build.os === "windows") { // Try the HKLM key const programs = ["chrome.exe", "msedge.exe"]; @@ -222,7 +224,7 @@ async function findChrome(): Promise { programs[i], "(Default)", ); - if (path && existsSync(path)) break; + if (path && safeExistsSync(path)) break; } // Try the HKCR key @@ -246,6 +248,12 @@ async function findChrome(): Promise { path = await which("chromium-browser"); } } + if (path) { + debug("[CHROMIUM] Found Chromium on OS known location"); + debug(`[CHROMIUM] Path: ${path}`); + } else { + debug("[CHROMIUM] Chromium not found on OS known location"); + } return path; }