Skip to content

fix(oidc-signin-tool): replace deprecated Playwright APIs to fix Chromium 146+ sign-in failures#346

Merged
hl662 merged 5 commits intomainfrom
nam/fix-playwright
Apr 24, 2026
Merged

fix(oidc-signin-tool): replace deprecated Playwright APIs to fix Chromium 146+ sign-in failures#346
hl662 merged 5 commits intomainfrom
nam/fix-playwright

Conversation

@hl662
Copy link
Copy Markdown
Contributor

@hl662 hl662 commented Apr 23, 2026

Why

Chromium 146 (shipped in Electron 41) changed the timing of OAuth redirects — they now complete before in-flight CDP commands return. The OIDC sign-in automation in this package uses Playwright's deprecated page.$() API, which sends eager CDP queries. When a redirect races ahead of the response, the execution context is already gone and the query throws "Execution context was destroyed", failing every E2E test that signs in through a separate Electron window.

What

Replaced all deprecated page.$() and page.waitForSelector() calls in SignInAutomation.ts with Playwright's locator API, which binds lazily and tolerates navigations. Error-checking helpers (checkSelectorExists, checkErrorOnPage) now catch context-destroyed failures gracefully, and the federated sign-in flow branches on the Promise.race winner directly instead of re-querying the DOM.

hl662 and others added 3 commits April 23, 2026 17:05
Replace deprecated Playwright page.$() and page.waitForSelector() APIs
with modern locator-based equivalents in SignInAutomation.ts. The
deprecated APIs throw "Execution context was destroyed" on Chromium 146+
(Electron 41) because OAuth redirects complete faster than the CDP
response.

Changes:
- checkSelectorExists: use page.locator().count() with try/catch for
  navigation resilience
- checkErrorOnPage: use page.locator() with try/catch, throwing error
  text outside the catch to avoid swallowing sign-in errors
- handleFederatedSignin: branch on Promise.race winner ("ms" vs "fed")
  instead of re-querying the DOM after the race
- Replace all page.waitForSelector() → locator auto-wait / .click()
- Suppress unhandled rejections from losing Promise.race participants

Fixes: iTwin/itwinjs-backlog#2030

Co-authored-by: Copilot <[email protected]>
- Update path-to-regexp override from pinned 0.1.12 (vulnerable) to >=0.1.13
- Add lodash override >=4.18.0 to fix code injection vulnerability
- Update vite to >=6.4.2 to fix arbitrary file read vulnerability

Co-authored-by: Copilot <[email protected]>
@hl662 hl662 force-pushed the nam/fix-playwright branch from 77e6ca6 to d2d9ab3 Compare April 23, 2026 21:33
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Updates the OIDC sign-in automation to avoid Playwright APIs that are prone to “execution context destroyed” failures during OAuth redirect races (Chromium 146+/Electron 41), and bumps a couple of dependencies/overrides.

Changes:

  • Replace deprecated page.$() / page.waitForSelector() usage with locator-based Playwright APIs in SignInAutomation.ts.
  • Add/adjust pnpm overrides (notably path-to-regexp, lodash) and update Vite to ^6.4.2 in browser/electron packages.
  • Add Beachball change files for the affected packages.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
pnpm-lock.yaml Captures dependency resolution changes from new overrides and Vite bump.
packages/oidc-signin-tool/src/SignInAutomation.ts Migrates selector operations to locator API and hardens helpers against navigation/context-destroyed errors.
packages/electron/package.json Bumps Vite devDependency to ^6.4.2.
packages/browser/package.json Bumps Vite devDependency to ^6.4.2.
package.json Updates pnpm overrides for path-to-regexp and lodash.
change/@itwin-oidc-signin-tool-*.json Patch change entry for the locator migration.
change/@itwin-electron-authorization-*.json Records Vite devDependency bump (no release).
change/@itwin-browser-authorization-*.json Records Vite devDependency bump (no release).
Files not reviewed (1)
  • pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pnpm-lock.yaml
Comment thread package.json Outdated
Comment thread package.json
Comment thread packages/oidc-signin-tool/src/SignInAutomation.ts Outdated
- Pin path-to-regexp override to 0.1.13 (was >=0.1.13 which resolved to 8.x inside Express 4)
- Pin lodash override to 4.18.1 (was >=4.18.0)
- Add void prefix to .catch() calls to satisfy no-floating-promises

Co-authored-by: Copilot <[email protected]>
@hl662 hl662 enabled auto-merge (squash) April 24, 2026 16:35
@hl662 hl662 merged commit f7e34ac into main Apr 24, 2026
13 checks passed
@hl662 hl662 deleted the nam/fix-playwright branch April 24, 2026 16:35
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.

3 participants