Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failed IPC communication with playwright smoke tests from preload script #146785

Closed
rebornix opened this issue Apr 5, 2022 · 8 comments
Closed
Assignees
Labels
linux Issues with VS Code on Linux smoke-test-failure windows VS Code on Windows issues
Milestone

Comments

@rebornix
Copy link
Member

rebornix commented Apr 5, 2022

1) VSCode Smoke Tests (Electron)
       Quick Open
         "before all" hook for "quick open search produces correct result":
     locator.waitFor: Timeout 40000ms exceeded.
=========================== logs ===========================
waiting for selector ".monaco-workbench" to be visible
============================================================
      at Application.checkWindowReady (D:\a\_work\1\s\test\automation\src\application.ts:149:56)
      at Application._start (D:\a\_work\1\s\test\automation\src\application.ts:98:4)
      at Application.start (D:\a\_work\1\s\test\automation\src\application.ts:75:3)
      at Context.<anonymous> (src\utils.ts:92:3)

https://dev.azure.com/monacotools/Monaco/_build/results?buildId=165034&view=logs&j=672276a2-8d3a-5fab-615d-090c51352f92&t=9302e8c4-de85-5274-34fa-935ce79d4e8d

@bpasero
Copy link
Member

bpasero commented Apr 5, 2022

Yes this is from the new smoke tests with playwright, they are marked to not fail the build though.

@bpasero bpasero added the windows VS Code on Windows issues label Apr 5, 2022
@bpasero bpasero changed the title Flaky remote smoke test Windows: playwright smoke tests often timeout on window Apr 5, 2022
@bpasero
Copy link
Member

bpasero commented Apr 5, 2022

This is our master issue for tracking the Windows issues for using playwright.

/cc @pavelfeldman

@bpasero
Copy link
Member

bpasero commented Apr 5, 2022

More logging reveals that the window awaits a IPC reply from the main process to continue loading the workbench but the response is never reaching the window.

Here is the flow:

  • electron main process opens a ipcMain.handle (from here before the window loadUrl is called)
  • window loads preload.js first which will immediately call on the ipcRenderer.invoke (from here)
  • we await the config object to be resolved

Unclear to me what could trigger this in Electron, maybe it is related to the extra inspect/debugging related properties playwright passes on:

D:\a\_work\1\VSCode-win32-x64\Code - Insiders.exe --inspect=0 --remote-debugging-port=0 C:\Users\CLOUDT~1\AppData\Local\Temp\vscsmoke\vscode-smoketest-express --skip-release-notes --skip-welcome --disable-telemetry --no-cached-data --disable-updates --disable-keytar --disable-crash-reporter --disable-workspace-trust --extensions-dir=C:\Users\CLOUDT~1\AppData\Local\Temp\vscsmoke\extensions-dir --user-data-dir=C:\Users\CLOUDT~1\AppData\Local\Temp\vscsmoke\d-zfp3rzpv --logsPath=D:\a\_work\1\s\.build\logs\smoke-tests-electron --enable-proposed-api=vscode.vscode-notebook-tests --enable-smoke-test-driver true

bpasero added a commit that referenced this issue Apr 5, 2022
@bpasero
Copy link
Member

bpasero commented Apr 5, 2022

Mitigated by doing a window reload after timeout of 10 seconds. This change only impacts smoke tests, I am hesitant to change something in our preload or bootstrap files until real users report this.

Still would be interesting to figure out if this is a Electron regression.

@bpasero bpasero added this to ✨ New in Electron Integration Apr 5, 2022
@bpasero bpasero added the mitigated Issue has workaround in place label Apr 6, 2022
@bpasero bpasero moved this from ✨ New to 🗂 Backlog in Electron Integration Apr 8, 2022
@bpasero bpasero changed the title Windows: playwright smoke tests often timeout on window Windows: failed IPC communication with playwright smoke tests from preload script Apr 13, 2022
bpasero added a commit that referenced this issue Apr 13, 2022
@bpasero bpasero removed the mitigated Issue has workaround in place label Apr 14, 2022
@bpasero
Copy link
Member

bpasero commented Apr 14, 2022

@deepak1556 while I can mitigate the hang in the renderer by doing a window reload, I cannot mitigate it for the shared process where this is also happening and is most likely the cause of #146800

Would it be possible to do an internal build where electron/electron#33323 is reverted to see if that is the cause?

@bpasero bpasero changed the title Windows: failed IPC communication with playwright smoke tests from preload script Failed IPC communication with playwright smoke tests from preload script Apr 14, 2022
@bpasero bpasero added the linux Issues with VS Code on Linux label Apr 14, 2022
@deepak1556
Copy link
Contributor

deepak1556 commented Apr 20, 2022

@bpasero turns out the above change and electron/electron@3001b69 was reverted internally by Teams since it caused a regression on their end for their custom ipc implementations and never made it into our product builds. This could also explain why it was only seen with CI. The changes have now been added back in and I am currently creating newer builds. Pretty sure this is very likely related to the root issue sharing the cause sender.url being an empty string when using playwright for testing in #147301. Playwright is causing some sort of frame swap on initial load causing the empty url as well as https://github.com/electron/electron/blob/31c2b5703a75a5ff2bf0e68f472d9002fe866bfb/shell/browser/electron_browser_client.cc#L1558-L1559 condition to fail. I will look into this further once we have newer builds.

@bpasero
Copy link
Member

bpasero commented Apr 20, 2022

Thanks ❤️ , I forgot that we use different builds in our CI for smoke tests, so that is a good thing to keep in mind going forward.

Curious what kind of issue Teams folks saw with IPC, maybe we can connect the dots with our issue.

@bpasero bpasero moved this from 🗂 Backlog to 🏃 In Progress in Electron Integration Apr 21, 2022
@bpasero
Copy link
Member

bpasero commented Apr 22, 2022

Optimistically closing, will reopen when this happens again, I have removed all our workarounds.

@bpasero bpasero closed this as completed Apr 22, 2022
@bpasero bpasero moved this from 🏃 In Progress to ✔️ Done / Deferred in Electron Integration Apr 29, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Jun 6, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
linux Issues with VS Code on Linux smoke-test-failure windows VS Code on Windows issues
Projects
Archived in project
Electron Integration
  
✔️ Done / Deferred
Development

No branches or pull requests

4 participants