Merge fixes from core monorepo (#810)

Author: d-gubert

.github/workflows/build_and_test.yml

@@ -65,7 +65,7 @@ jobs:
               run: |
                   tar czf /tmp/workspace.tar.gz .
 
-            - uses: actions/upload-artifact@v2
+            - uses: actions/upload-artifact@v4
               with:
                   name: workspace
                   path: /tmp/workspace.tar.gz
@@ -80,7 +80,7 @@ jobs:
               with:
                   node-version: '14.19.3'
 
-            - uses: actions/download-artifact@v2
+            - uses: actions/download-artifact@v4
               with:
                   name: workspace
                   path: /tmp
@@ -102,7 +102,7 @@ jobs:
               with:
                   node-version: '14.19.3'
 
-            - uses: actions/download-artifact@v2
+            - uses: actions/download-artifact@v4
               with:
                   name: workspace
                   path: /tmp
@@ -124,7 +124,7 @@ jobs:
               with:
                   node-version: '14.19.3'
 
-            - uses: actions/download-artifact@v2
+            - uses: actions/download-artifact@v4
               with:
                   name: workspace
                   path: /tmp
@@ -140,10 +140,11 @@ jobs:
               run: |
                   tar czf /tmp/workspace.tar.gz .
 
-            - uses: actions/upload-artifact@v2
+            - uses: actions/upload-artifact@v4
               with:
                   name: workspace
                   path: /tmp/workspace.tar.gz
+                  overwrite: true
 
     publish:
         runs-on: ubuntu-latest
@@ -156,7 +157,7 @@ jobs:
               with:
                   node-version: '14.19.3'
 
-            - uses: actions/download-artifact@v2
+            - uses: actions/download-artifact@v4
               with:
                   name: workspace
                   path: /tmp

src/definition/metadata/AppMethod.ts

@@ -98,4 +98,6 @@ export enum AppMethod {
     EXECUTE_POST_USER_LOGGED_IN = 'executePostUserLoggedIn',
     EXECUTE_POST_USER_LOGGED_OUT = 'executePostUserLoggedOut',
     EXECUTE_POST_USER_STATUS_CHANGED = 'executePostUserStatusChanged',
+    // Runtime specific methods
+    RUNTIME_RESTART = 'runtime:restart',
 }

src/server/compiler/AppCompiler.ts

@@ -21,7 +21,7 @@ export class AppCompiler {
             throw new Error(`Invalid App package for "${storage.info.name}". Could not find the classFile (${storage.info.classFile}) file.`);
         }
 
-        const runtime = await manager.getRuntime().startRuntimeForApp(packageResult);
+        const runtime = await manager.getRuntime().startRuntimeForApp(packageResult, storage);
 
         const app = new ProxiedApp(manager, storage, runtime);
 

src/server/managers/AppRuntimeManager.ts

@@ -1,6 +1,7 @@
 import type { AppManager } from '../AppManager';
 import type { IParseAppPackageResult } from '../compiler';
 import { DenoRuntimeSubprocessController } from '../runtime/deno/AppsEngineDenoRuntime';
+import type { IAppStorageItem } from '../storage';
 
 export type AppRuntimeParams = {
     appId: string;
@@ -21,14 +22,18 @@ export class AppRuntimeManager {
 
     constructor(private readonly manager: AppManager) {}
 
-    public async startRuntimeForApp(appPackage: IParseAppPackageResult, options = { force: false }): Promise<DenoRuntimeSubprocessController> {
+    public async startRuntimeForApp(
+        appPackage: IParseAppPackageResult,
+        storageItem: IAppStorageItem,
+        options = { force: false },
+    ): Promise<DenoRuntimeSubprocessController> {
         const { id: appId } = appPackage.info;
 
         if (appId in this.subprocesses && !options.force) {
             throw new Error('App already has an associated runtime');
         }
 
-        this.subprocesses[appId] = new DenoRuntimeSubprocessController(this.manager, appPackage);
+        this.subprocesses[appId] = new DenoRuntimeSubprocessController(this.manager, appPackage, storageItem);
 
         await this.subprocesses[appId].setupApp();
 

src/server/runtime/deno/AppsEngineDenoRuntime.ts

@@ -7,11 +7,11 @@ import debugFactory from 'debug';
 
 import { decoder } from './codec';
 import type { AppManager } from '../../AppManager';
-import type { AppLogStorage } from '../../storage';
+import type { AppLogStorage, IAppStorageItem } from '../../storage';
 import type { AppBridges } from '../../bridges';
 import type { IParseAppPackageResult } from '../../compiler';
+import { AppConsole, type ILoggerStorageEntry } from '../../logging';
 import type { AppAccessorManager, AppApiManager } from '../../managers';
-import type { ILoggerStorageEntry } from '../../logging';
 import { AppStatus } from '../../../definition/AppStatus';
 import { bundleLegacyApp } from './bundler';
 import { ProcessMessenger } from './ProcessMessenger';
@@ -103,7 +103,7 @@ export class DenoRuntimeSubprocessController extends EventEmitter {
     private readonly livenessManager: LivenessManager;
 
     // We need to keep the appSource around in case the Deno process needs to be restarted
-    constructor(manager: AppManager, private readonly appPackage: IParseAppPackageResult) {
+    constructor(manager: AppManager, private readonly appPackage: IParseAppPackageResult, private readonly storageItem: IAppStorageItem) {
         super();
 
         this.debug = baseDebug.extend(appPackage.info.id);
@@ -164,23 +164,38 @@ export class DenoRuntimeSubprocessController extends EventEmitter {
         }
     }
 
-    public async killProcess(): Promise<void> {
+    /**
+     * Attempts to kill the process currently controlled by this.deno
+     *
+     * @returns boolean - if a process has been killed or not
+     */
+    public async killProcess(): Promise<boolean> {
+        if (!this.deno) {
+            this.debug('No child process reference');
+            return false;
+        }
+
+        let { killed } = this.deno;
+
         // This field is not populated if the process is killed by the OS
-        if (this.deno.killed) {
+        if (killed) {
             this.debug('App process was already killed');
-            return;
+            return killed;
         }
 
         // What else should we do?
         if (this.deno.kill('SIGKILL')) {
             // Let's wait until we get confirmation the process exited
             await new Promise<void>((r) => this.deno.on('exit', r));
+            killed = true;
         } else {
             this.debug('Tried killing the process but failed. Was it already dead?');
+            killed = false;
         }
 
         delete this.deno;
         this.messenger.clearReceiver();
+        return killed;
     }
 
     // Debug purposes, could be deleted later
@@ -200,7 +215,7 @@ export class DenoRuntimeSubprocessController extends EventEmitter {
 
     public async getStatus(): Promise<AppStatus> {
         // If the process has been terminated, we can't get the status
-        if (this.deno.exitCode !== null) {
+        if (!this.deno || this.deno.exitCode !== null) {
             return AppStatus.UNKNOWN;
         }
 
@@ -231,19 +246,49 @@ export class DenoRuntimeSubprocessController extends EventEmitter {
 
     public async restartApp() {
         this.debug('Restarting app subprocess');
+        const logger = new AppConsole('runtime:restart');
+
+        logger.info('Starting restart procedure for app subprocess...', this.livenessManager.getRuntimeData());
 
         this.state = 'restarting';
 
-        await this.killProcess();
+        try {
+            const pid = this.deno?.pid;
 
-        await this.setupApp();
+            const hasKilled = await this.killProcess();
 
-        // setupApp() changes the state to 'ready' - we'll need to workaround that for now
-        this.state = 'restarting';
+            if (hasKilled) {
+                logger.debug('Process successfully terminated', { pid });
+            } else {
+                logger.warn('Could not terminate process. Maybe it was already dead?', { pid });
+            }
 
-        await this.sendRequest({ method: 'app:initialize' });
+            await this.setupApp();
+            logger.info('New subprocess successfully spawned', { pid: this.deno.pid });
 
-        this.state = 'ready';
+            // setupApp() changes the state to 'ready' - we'll need to workaround that for now
+            this.state = 'restarting';
+
+            // When an app has just been installed, the status in the storageItem passed to this controller will be "initialized"
+            // So, whenever we get that value here, let's just make it 'auto_enabled'
+            let { status } = this.storageItem;
+
+            if (status === AppStatus.INITIALIZED) {
+                logger.info('Stored status was "initialized". Changing to "auto_enabled"');
+                status = AppStatus.AUTO_ENABLED;
+            }
+
+            await this.sendRequest({ method: 'app:initialize' });
+            await this.sendRequest({ method: 'app:setStatus', params: [status] });
+
+            this.state = 'ready';
+
+            logger.info('Successfully restarted app subprocess');
+        } catch (e) {
+            logger.error("Failed to restart app's subprocess", { error: e });
+        } finally {
+            await this.logStorage.storeEntries(AppConsole.toStorageEntry(this.getAppId(), logger));
+        }
     }
 
     public getAppId(): string {

src/server/runtime/deno/LivenessManager.ts

@@ -10,7 +10,7 @@ const defaultOptions: LivenessManager['options'] = {
     pingRequestTimeout: 10000,
     pingFrequencyInMS: 10000,
     consecutiveTimeoutLimit: 4,
-    maxRestarts: 3,
+    maxRestarts: Infinity,
 };
 
 /**
@@ -65,6 +65,16 @@ export class LivenessManager {
         this.options = Object.assign({}, defaultOptions, options);
     }
 
+    public getRuntimeData() {
+        const { restartCount, pingTimeoutConsecutiveCount, restartLog } = this;
+
+        return {
+            restartCount,
+            pingTimeoutConsecutiveCount,
+            restartLog,
+        };
+    }
+
     public attach(deno: ChildProcess) {
         this.subprocess = deno;
 
@@ -122,7 +132,7 @@ export class LivenessManager {
 
                 if (reason === 'timeout' && this.pingTimeoutConsecutiveCount >= this.options.consecutiveTimeoutLimit) {
                     this.debug('Subprocess failed to respond to pings %d consecutive times. Attempting restart...', this.options.consecutiveTimeoutLimit);
-                    this.restartProcess();
+                    this.restartProcess('Too many pings timed out');
                     return false;
                 }
 
@@ -154,17 +164,21 @@ export class LivenessManager {
             return;
         }
 
+        let reason: string;
+
         // Otherwise we try to restart the subprocess, if possible
         if (signal) {
             this.debug('App has been killed (%s). Attempting restart #%d...', signal, this.restartCount + 1);
+            reason = `App has been killed with signal ${signal}`;
         } else {
             this.debug('App has exited with code %d. Attempting restart #%d...', exitCode, this.restartCount + 1);
+            reason = `App has exited with code ${exitCode}`;
         }
 
-        this.restartProcess();
+        this.restartProcess(reason);
     }
 
-    private restartProcess() {
+    private restartProcess(reason: string) {
         if (this.restartCount >= this.options.maxRestarts) {
             this.debug('Limit of restarts reached (%d). Aborting restart...', this.options.maxRestarts);
             this.controller.stopApp();
@@ -174,6 +188,7 @@ export class LivenessManager {
         this.pingTimeoutConsecutiveCount = 0;
         this.restartCount++;
         this.restartLog.push({
+            reason,
             restartedAt: new Date(),
             source: 'liveness-manager',
             pid: this.subprocess.pid,

tests/server/runtime/DenoRuntimeSubprocessController.spec.ts

@@ -1,24 +1,28 @@
-import * as path from 'path';
 import * as fs from 'fs/promises';
+import * as path from 'path';
 
 import { TestFixture, Setup, Expect, AsyncTest, SpyOn, Any, AsyncSetupFixture, Teardown } from 'alsatian';
 import type { SuccessObject } from 'jsonrpc-lite';
 
-import { AppAccessorManager, AppApiManager } from '../../../src/server/managers';
-import { TestInfastructureSetup } from '../../test-data/utilities';
-import { DenoRuntimeSubprocessController } from '../../../src/server/runtime/deno/AppsEngineDenoRuntime';
-import type { AppManager } from '../../../src/server/AppManager';
+import { AppStatus } from '../../../src/definition/AppStatus';
 import { UserStatusConnection, UserType } from '../../../src/definition/users';
+import type { AppManager } from '../../../src/server/AppManager';
 import type { IParseAppPackageResult } from '../../../src/server/compiler';
+import { AppAccessorManager, AppApiManager } from '../../../src/server/managers';
+import { DenoRuntimeSubprocessController } from '../../../src/server/runtime/deno/AppsEngineDenoRuntime';
+import type { IAppStorageItem } from '../../../src/server/storage';
+import { TestInfastructureSetup } from '../../test-data/utilities';
 
-@TestFixture('DenoRuntimeSubprocessController')
+@TestFixture()
 export class DenuRuntimeSubprocessControllerTestFixture {
     private manager: AppManager;
 
     private controller: DenoRuntimeSubprocessController;
 
     private appPackage: IParseAppPackageResult;
 
+    private appStorageItem: IAppStorageItem;
+
     @AsyncSetupFixture
     public async fixture() {
         const infrastructure = new TestInfastructureSetup();
@@ -35,11 +39,16 @@ export class DenuRuntimeSubprocessControllerTestFixture {
         const appPackage = await fs.readFile(path.join(__dirname, '../../test-data/apps/hello-world-test_0.0.1.zip'));
 
         this.appPackage = await this.manager.getParser().unpackageApp(appPackage);
+
+        this.appStorageItem = {
+            id: 'hello-world-test',
+            status: AppStatus.MANUALLY_ENABLED,
+        } as IAppStorageItem;
     }
 
     @Setup
     public setup() {
-        this.controller = new DenoRuntimeSubprocessController(this.manager, this.appPackage);
+        this.controller = new DenoRuntimeSubprocessController(this.manager, this.appPackage, this.appStorageItem);
         this.controller.setupApp();
     }