Conversation
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Bumps [electron](https://github.com/electron/electron) from 38.2.1 to 39.0.0. - [Release notes](https://github.com/electron/electron/releases) - [Changelog](https://github.com/electron/electron/blob/main/docs/breaking-changes.md) - [Commits](electron/electron@v38.2.1...v39.0.0) --- updated-dependencies: - dependency-name: electron dependency-version: 39.0.0 dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
b3175b0 to
080415b
Compare
|
39.1.0 came out today, probably worth bumping to it instead of shipping 39.0.0. |
|
I will try to test the update sometime this or next week. |
|
We can also try upgrading node-pty here, as Dependabot closed the PR that updated that package lol. |
|
I've tested electron 39.2.2 and node-pty 1.0.0-beta.39. @ravicious did you test the v38 upgrade in a VM? I'm wondering if the 38.2.1 version that we have one master is broken too. |
|
OK, I won't notify you again about this release, but will get in touch when a new version is available. If you'd rather skip all updates until the next major or minor version, let me know by commenting If you change your mind, just re-open this PR and I'll resolve any conflicts on it. |
| @@ -44,7 +44,7 @@ | |||
| "@types/whatwg-url": "^13.0.0", | |||
| "@xterm/addon-fit": "^0.10.0", | |||
| "@xterm/xterm": "^5.5.0", | |||
| "electron": "38.2.1", | |||
| "electron": "39.0.0", | |||
There was a problem hiding this comment.
@ravicious did you test the v38 upgrade in a VM? I'm wondering if the 38.2.1 version that we have one master is broken too.
Yeah I did test it in a VM, see here: #59812 (comment). Too bad that the other issue you linked to seems to show up even outside of a VM. electron/electron#48859 (comment)
We could wait for v40 I guess and in the meantime update v37 on Teleport v18 and v17.
There was a problem hiding this comment.
Oh I saw the comment, but I don't know how I missed that you ran into the same bug in the VM.
We could wait for v40 I guess and in the meantime update v37 on Teleport v18 and v17.
I think we could merge this PR, as it won't introduce an new bug and I already went through the test plan on all platforms :p
I will work on updating the Electron version on v18/v17.
There was a problem hiding this comment.
Cool, so I guess you just need to push commits from your computer to make this branch update Electron to 39.2.2 right?
| @@ -26,7 +26,7 @@ | |||
| "@grpc/grpc-js": "1.14.0", | |||
| "@types/which": "^3.0.4", | |||
| "node-forge": "^1.3.1", | |||
| "node-pty": "1.1.0-beta35", | |||
| "node-pty": "1.1.0-beta39", | |||
There was a problem hiding this comment.
I cannot start a local terminal. The UI shows "Cannot execute /opt/homebrew/bin/zsh : posix_spawnp failed.". In the logs I see this:
SHARED [PtyHostService] info: created PTY process for id ET2_gzLSWIx297BwZUf9Q
SHARED [PtyProcess (id: ET2_gzLSWIx297BwZUf9Q /opt/homebrew/bin/zsh )] error: Error: posix_spawnp failed.
SHARED at new UnixTerminal (/Users/rav/src/teleport/node_modules/.pnpm/node-pty@1.1.0-beta39/node_modules/node-pty/lib/unixTerminal.js:86:24)
SHARED at Module.spawn (/Users/rav/src/teleport/node_modules/.pnpm/node-pty@1.1.0-beta39/node_modules/node-pty/lib/index.js:30:12)
SHARED at PtyProcess.start (/Users/rav/src/teleport/web/packages/teleterm/build/app/main/sharedProcess.js:1269:42)
SHARED [PtyEventsStreamHandler (id: ET2_gzLSWIx297BwZUf9Q)] info: stream has started
Additionally, our PTY process handling appears to not handle it super well, because when I then close this tab, I see the following errors in the logs. Maybe it's the result of my gRPC changes from #59832.
SHARED [PtyEventsStreamHandler (id: ET2_gzLSWIx297BwZUf9Q)] info: stream has ended
SHARED [uncaught exception] error: unhandledRejection TypeError: Cannot read properties of undefined (reading 'onExit')
SHARED at /Users/rav/src/teleport/web/packages/teleterm/build/app/main/sharedProcess.js:1430:48
SHARED at new Promise (<anonymous>)
SHARED at promisifyProcessExit (/Users/rav/src/teleport/web/packages/teleterm/build/app/main/sharedProcess.js:1430:10)
SHARED at PtyProcess.dispose (/Users/rav/src/teleport/web/packages/teleterm/build/app/main/sharedProcess.js:1331:25)
SHARED at PtyEventsStreamHandler.cleanResources (/Users/rav/src/teleport/web/packages/teleterm/build/app/main/sharedProcess.js:1227:27)
SHARED at PtyEventsStreamHandler.handleStreamEnd (/Users/rav/src/teleport/web/packages/teleterm/build/app/main/sharedProcess.js:1224:15)
SHARED at ServerDuplexStreamImpl.<anonymous> (/Users/rav/src/teleport/web/packages/teleterm/build/app/main/sharedProcess.js:1159:42)
SHARED at ServerDuplexStreamImpl.emit (node:events:519:28)
SHARED at endReadableNT (node:internal/streams/readable:1698:12)
SHARED at process.processTicksAndRejections (node:internal/process/task_queues:90:21)
SHARED [uncaught exception] error: unhandledRejection TypeError: Cannot read properties of undefined (reading 'kill')
SHARED at PtyProcess.dispose (/Users/rav/src/teleport/web/packages/teleterm/build/app/main/sharedProcess.js:1333:19)
SHARED at PtyEventsStreamHandler.cleanResources (/Users/rav/src/teleport/web/packages/teleterm/build/app/main/sharedProcess.js:1227:27)
SHARED at PtyEventsStreamHandler.handleStreamEnd (/Users/rav/src/teleport/web/packages/teleterm/build/app/main/sharedProcess.js:1224:15)
SHARED at ServerDuplexStreamImpl.<anonymous> (/Users/rav/src/teleport/web/packages/teleterm/build/app/main/sharedProcess.js:1159:42)
SHARED at ServerDuplexStreamImpl.emit (node:events:519:28)
SHARED at endReadableNT (node:internal/streams/readable:1698:12)
SHARED at process.processTicksAndRejections (node:internal/process/task_queues:90:21)
There was a problem hiding this comment.
I think the error from the second code block happens because the PtyProcess failed to start but we still call dispose which tries to wait for its exit. In the catch block where we call this._process = nodePTY.spawn, we should probably set this._disposed to true.
There was a problem hiding this comment.
Interesting, I didn't see it while performing the checklist, it seems that it doesn't happen in a packaged app.
I see there are some issue about it microsoft/node-pty#789, microsoft/node-pty#670, but without any resolution.
I will revert the update.
In the catch block where we call this._process = nodePTY.spawn, we should probably set this._disposed to true.
Yeah, I think so.
This reverts commit 260869b.
* Bump electron from 38.2.1 to 39.0.0 Bumps [electron](https://github.com/electron/electron) from 38.2.1 to 39.0.0. - [Release notes](https://github.com/electron/electron/releases) - [Changelog](https://github.com/electron/electron/blob/main/docs/breaking-changes.md) - [Commits](electron/electron@v38.2.1...v39.0.0) --- updated-dependencies: - dependency-name: electron dependency-version: 39.0.0 dependency-type: direct:development update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com> * Revert unrelated updates * Update electron to 39.2.2 * Update node-pty to 1.1.0-beta39 * Revert "Update node-pty to 1.1.0-beta39" This reverts commit 260869b. --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Grzegorz Zdunek <grzegorz.zdunek@goteleport.com>
Bumps electron from 38.2.1 to 39.0.0.
Release notes
Sourced from electron's releases.
... (truncated)
Commits
0abda74docs: modify the thickFrame doc (#48677)4e8a552fix: logical bug in install.js env var handling (#48673)d83383bdocs: fix Ubuntu version used to build Electron (#48643)496db94chore: bump chromium to 142.0.7444.52 (39-x-y) (#48641)00627c6fix: crash on empty dialog extensions array on Windows (#48658)7319e5cdocs: security.md mark 'Enable process sandboxing' as active by defau… (#48647)1056280feat: enable more granular a11y feature management (#48625)4fda94bfeat: AddgetAccentColoron Linux (#48628)e3715b0fix:systemPreferences.getAccentColorinverted color (#48624)90674e0fix: icon in Windows toast notification (#48629)Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting
@dependabot rebase.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebasewill rebase this PR@dependabot recreatewill recreate this PR, overwriting any edits that have been made to it@dependabot mergewill merge this PR after your CI passes on it@dependabot squash and mergewill squash and merge this PR after your CI passes on it@dependabot cancel mergewill cancel a previously requested merge and block automerging@dependabot reopenwill reopen this PR if it is closed@dependabot closewill close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot show <dependency name> ignore conditionswill show all of the ignore conditions of the specified dependency@dependabot ignore this major versionwill close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor versionwill close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependencywill close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)