From 91a2afb098ad8c3c11302d52cce91dc9f1b3b331 Mon Sep 17 00:00:00 2001 From: Sergei Garin Date: Wed, 22 May 2024 16:56:42 +0300 Subject: [PATCH] Fix broken login flow (#9965) * kill the electron process instead of restarting limit the port range with single one Make ENSO_CLOUD_REDIRECT optional, use window.location.origin by default * Revert server changes * limit the amount of ports in server --- app/gui2/vite.config.ts | 3 +- app/ide-desktop/.example.env | 1 - app/ide-desktop/lib/client/src/bin/server.ts | 2 +- app/ide-desktop/lib/client/src/index.ts | 34 ++++++++++++------- app/ide-desktop/lib/client/watch.ts | 26 ++++++++++---- app/ide-desktop/lib/common/src/appConfig.js | 6 ++-- .../dashboard/src/authentication/service.ts | 7 ++-- app/ide-desktop/lib/dashboard/vite.config.ts | 2 +- app/ide-desktop/lib/types/globals.d.ts | 2 +- 9 files changed, 51 insertions(+), 32 deletions(-) diff --git a/app/gui2/vite.config.ts b/app/gui2/vite.config.ts index ec3cb0b1ad3..dd342fbaaf8 100644 --- a/app/gui2/vite.config.ts +++ b/app/gui2/vite.config.ts @@ -12,7 +12,6 @@ import { defineConfig, type Plugin } from 'vite' // @ts-expect-error import * as tailwindConfig from 'enso-dashboard/tailwind.config' import { createGatewayServer } from './ydoc-server' -const localServerPort = 8080 const projectManagerUrl = 'ws://127.0.0.1:30535' const IS_CLOUD_BUILD = process.env.CLOUD_BUILD === 'true' @@ -60,7 +59,7 @@ export default defineConfig({ }, }, define: { - ...getDefines(localServerPort), + ...getDefines(), IS_CLOUD_BUILD: JSON.stringify(IS_CLOUD_BUILD), PROJECT_MANAGER_URL: JSON.stringify(projectManagerUrl), YDOC_SERVER_URL: IS_POLYGLOT_YDOC_SERVER ? JSON.stringify('defined') : undefined, diff --git a/app/ide-desktop/.example.env b/app/ide-desktop/.example.env index b4ff39c0410..ba37cdfe520 100644 --- a/app/ide-desktop/.example.env +++ b/app/ide-desktop/.example.env @@ -1,4 +1,3 @@ -ENSO_CLOUD_REDIRECT=http://localhost:8080 ENSO_CLOUD_ENVIRONMENT=production ENSO_CLOUD_API_URL=https://aaaaaaaaaa.execute-api.mars.amazonaws.com ENSO_CLOUD_CHAT_URL=wss://chat.example.com diff --git a/app/ide-desktop/lib/client/src/bin/server.ts b/app/ide-desktop/lib/client/src/bin/server.ts index 1587a99eaec..2a61e55958f 100644 --- a/app/ide-desktop/lib/client/src/bin/server.ts +++ b/app/ide-desktop/lib/client/src/bin/server.ts @@ -75,7 +75,7 @@ export class Config { /** Determine the initial available communication endpoint, starting from the specified port, * to provide file hosting services. */ async function findPort(port: number): Promise { - return await portfinder.getPortPromise({ port, startPort: port }) + return await portfinder.getPortPromise({ port, startPort: port, stopPort: port + 4 }) } // ============== diff --git a/app/ide-desktop/lib/client/src/index.ts b/app/ide-desktop/lib/client/src/index.ts index 14c02dd1979..096ad77acae 100644 --- a/app/ide-desktop/lib/client/src/index.ts +++ b/app/ide-desktop/lib/client/src/index.ts @@ -223,8 +223,8 @@ class App { await logger.asyncGroupMeasured('Starting the application', async () => { // Note that we want to do all the actions synchronously, so when the window // appears, it serves the website immediately. - await this.startBackendIfEnabled() await this.startContentServerIfEnabled() + await this.startBackendIfEnabled() await this.createWindowIfEnabled(windowSize) this.initIpc() await this.loadWindowContent() @@ -236,8 +236,10 @@ class App { authentication.initModule(() => this.window!) }) } catch (err) { - console.error('Failed to initialize the application, shutting down. Error:', err) + logger.error('Failed to initialize the application, shutting down. Error: ', err) electron.app.quit() + } finally { + logger.groupEnd() } } @@ -281,18 +283,24 @@ class App { /** Start the content server, which will serve the application content (HTML) to the window. */ async startContentServerIfEnabled() { await this.runIfEnabled(this.args.options.server, async () => { - await logger.asyncGroupMeasured('Starting the content server.', async () => { - const serverCfg = new server.Config({ - dir: paths.ASSETS_PATH, - port: this.args.groups.server.options.port.value, - externalFunctions: { - uploadProjectBundle: projectManagement.uploadBundle, - runProjectManagerCommand: (cliArguments, body?: NodeJS.ReadableStream) => - projectManager.runCommand(this.args, cliArguments, body), - }, + await logger + .asyncGroupMeasured('Starting the content server.', async () => { + const serverCfg = new server.Config({ + dir: paths.ASSETS_PATH, + port: this.args.groups.server.options.port.value, + externalFunctions: { + uploadProjectBundle: projectManagement.uploadBundle, + runProjectManagerCommand: ( + cliArguments, + body?: NodeJS.ReadableStream + ) => projectManager.runCommand(this.args, cliArguments, body), + }, + }) + this.server = await server.Server.create(serverCfg) + }) + .finally(() => { + logger.groupEnd() }) - this.server = await server.Server.create(serverCfg) - }) }) } diff --git a/app/ide-desktop/lib/client/watch.ts b/app/ide-desktop/lib/client/watch.ts index bf068a287c5..fb9c816abee 100644 --- a/app/ide-desktop/lib/client/watch.ts +++ b/app/ide-desktop/lib/client/watch.ts @@ -109,18 +109,32 @@ process.on('SIGINT', () => { }) }) -while (true) { +/** + * Starts the electron process with the IDE. + */ +function startElectronProcess() { console.log('Spawning Electron process.') + const electronProcess = childProcess.spawn('electron', ELECTRON_ARGS, { stdio: 'inherit', shell: true, // eslint-disable-next-line @typescript-eslint/naming-convention env: Object.assign({ NODE_MODULES_PATH }, process.env), }) - console.log('Waiting for Electron process to finish.') - const result = await new Promise((resolve, reject) => { - electronProcess.on('close', resolve) - electronProcess.on('error', reject) + + electronProcess.on('close', code => { + if (code === 0) { + electronProcess.removeAllListeners() + process.exit(0) + } + }) + electronProcess.on('error', error => { + console.error('Electron process failed:', error) + console.error('Killing electron process.') + electronProcess.removeAllListeners() + electronProcess.kill() + process.exit(1) }) - console.log('Electron process finished. Exit code: ', result) } + +startElectronProcess() diff --git a/app/ide-desktop/lib/common/src/appConfig.js b/app/ide-desktop/lib/common/src/appConfig.js index ca164dffdee..617ae816907 100644 --- a/app/ide-desktop/lib/common/src/appConfig.js +++ b/app/ide-desktop/lib/common/src/appConfig.js @@ -86,13 +86,13 @@ function stringify(value) { * - the unique identifier for the cloud environment, for use in Sentry logs * - Stripe, Sentry and Amplify public keys */ // eslint-disable-next-line @typescript-eslint/no-magic-numbers -export function getDefines(serverPort = 8080) { +export function getDefines() { return { /* eslint-disable @typescript-eslint/naming-convention */ 'process.env.ENSO_CLOUD_REDIRECT': stringify( // The actual environment variable does not necessarily exist. // eslint-disable-next-line @typescript-eslint/no-unnecessary-condition - process.env.ENSO_CLOUD_REDIRECT ?? `http://localhost:${serverPort}` + process.env.ENSO_CLOUD_REDIRECT ), 'process.env.ENSO_CLOUD_ENVIRONMENT': stringify( // The actual environment variable does not necessarily exist. @@ -124,11 +124,9 @@ export function getDefines(serverPort = 8080) { } } -const SERVER_PORT = 8080 const DUMMY_DEFINES = { /* eslint-disable @typescript-eslint/naming-convention */ 'process.env.NODE_ENV': 'production', - 'process.env.ENSO_CLOUD_REDIRECT': `http://localhost:${SERVER_PORT}`, 'process.env.ENSO_CLOUD_ENVIRONMENT': 'production', 'process.env.ENSO_CLOUD_API_URL': 'https://mock', 'process.env.ENSO_CLOUD_SENTRY_DSN': diff --git a/app/ide-desktop/lib/dashboard/src/authentication/service.ts b/app/ide-desktop/lib/dashboard/src/authentication/service.ts index 198f306b5f6..73bc9bde196 100644 --- a/app/ide-desktop/lib/dashboard/src/authentication/service.ts +++ b/app/ide-desktop/lib/dashboard/src/authentication/service.ts @@ -167,10 +167,11 @@ function loadAmplifyConfig( // needs to be registered. setDeepLinkHandler(logger, navigate) } + + const redirectUrl = process.env.ENSO_CLOUD_REDIRECT ?? window.location.origin + /** Load the platform-specific Amplify configuration. */ - const signInOutRedirect = supportsDeepLinks - ? `${common.DEEP_LINK_SCHEME}://auth` - : process.env.ENSO_CLOUD_REDIRECT + const signInOutRedirect = supportsDeepLinks ? `${common.DEEP_LINK_SCHEME}://auth` : redirectUrl return process.env.ENSO_CLOUD_COGNITO_USER_POOL_ID == null || process.env.ENSO_CLOUD_COGNITO_USER_POOL_WEB_CLIENT_ID == null || process.env.ENSO_CLOUD_COGNITO_DOMAIN == null || diff --git a/app/ide-desktop/lib/dashboard/vite.config.ts b/app/ide-desktop/lib/dashboard/vite.config.ts index d0b23e4ac44..00521c0b473 100644 --- a/app/ide-desktop/lib/dashboard/vite.config.ts +++ b/app/ide-desktop/lib/dashboard/vite.config.ts @@ -45,7 +45,7 @@ export default vite.defineConfig({ IS_VITE: JSON.stringify(true), // The sole hardcoded usage of `global` in aws-amplify. 'global.TYPED_ARRAY_SUPPORT': JSON.stringify(true), - ...appConfig.getDefines(SERVER_PORT), + ...appConfig.getDefines(), }, }) diff --git a/app/ide-desktop/lib/types/globals.d.ts b/app/ide-desktop/lib/types/globals.d.ts index 80747890942..8951c754150 100644 --- a/app/ide-desktop/lib/types/globals.d.ts +++ b/app/ide-desktop/lib/types/globals.d.ts @@ -147,7 +147,7 @@ declare global { // === Cloud environment variables === // @ts-expect-error The index signature is intentional to disallow unknown env vars. - readonly ENSO_CLOUD_REDIRECT: string + readonly ENSO_CLOUD_REDIRECT?: string // When unset, the `.env` loader tries to load `.env` rather than `..env`. // Set to the empty string to load `.env`. // @ts-expect-error The index signature is intentional to disallow unknown env vars.