Skip to content

Commit

Permalink
Merge fixes from core monorepo (#810)
Browse files Browse the repository at this point in the history
  • Loading branch information
d-gubert authored Nov 10, 2024
2 parents e4bd643 + 7042b29 commit 1f2b310
Show file tree
Hide file tree
Showing 7 changed files with 110 additions and 33 deletions.
13 changes: 7 additions & 6 deletions .github/workflows/build_and_test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down
2 changes: 2 additions & 0 deletions src/definition/metadata/AppMethod.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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',
}
2 changes: 1 addition & 1 deletion src/server/compiler/AppCompiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
9 changes: 7 additions & 2 deletions src/server/managers/AppRuntimeManager.ts
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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();

Expand Down
71 changes: 58 additions & 13 deletions src/server/runtime/deno/AppsEngineDenoRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand All @@ -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;
}

Expand Down Expand Up @@ -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 {
Expand Down
23 changes: 19 additions & 4 deletions src/server/runtime/deno/LivenessManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const defaultOptions: LivenessManager['options'] = {
pingRequestTimeout: 10000,
pingFrequencyInMS: 10000,
consecutiveTimeoutLimit: 4,
maxRestarts: 3,
maxRestarts: Infinity,
};

/**
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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();
Expand All @@ -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,
Expand Down
23 changes: 16 additions & 7 deletions tests/server/runtime/DenoRuntimeSubprocessController.spec.ts
Original file line number Diff line number Diff line change
@@ -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();
Expand All @@ -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();
}

Expand Down

0 comments on commit 1f2b310

Please sign in to comment.