Skip to content

Commit

Permalink
feat(deno-runtime): limit timeout of "Pre" listener events to 1s (#775)
Browse files Browse the repository at this point in the history
  • Loading branch information
d-gubert authored Jul 1, 2024
1 parent a1000dd commit c3b4d2e
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 5 deletions.
10 changes: 9 additions & 1 deletion src/server/ProxiedApp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,17 @@ export class ProxiedApp {
return logger;
}

// We'll need to refactor this method to remove the rest parameters so we can pass an options parameter
public async call(method: `${AppMethod}`, ...args: Array<any>): Promise<any> {
let options;

// Pre events need to be fast as they block the user
if (method.startsWith('checkPre') || method.startsWith('executePre')) {
options = { timeout: 1000 };
}

try {
return await this.appRuntime.sendRequest({ method: `app:${method}`, params: args });
return await this.appRuntime.sendRequest({ method: `app:${method}`, params: args }, options);
} catch (e) {
if (e.code === AppsEngineException.JSONRPC_ERROR_CODE) {
throw new AppsEngineException(e.message);
Expand Down
12 changes: 8 additions & 4 deletions src/server/runtime/deno/AppsEngineDenoRuntime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ export type DenoSystemUsageRecord = {
external: number;
};

export type DenoRuntimeOptions = {
timeout: number;
};

export class DenoRuntimeSubprocessController extends EventEmitter {
private readonly deno: child_process.ChildProcess;

Expand Down Expand Up @@ -259,14 +263,14 @@ export class DenoRuntimeSubprocessController extends EventEmitter {
this.deno.stdin.write(encoder.encode(message));
}

public async sendRequest(message: Pick<jsonrpc.RequestObject, 'method' | 'params'>): Promise<unknown> {
public async sendRequest(message: Pick<jsonrpc.RequestObject, 'method' | 'params'>, options = this.options): Promise<unknown> {
const id = String(Math.random().toString(36)).substring(2);

const start = Date.now();

const request = jsonrpc.request(id, message.method, message.params);

const promise = this.waitForResponse(request).finally(() => {
const promise = this.waitForResponse(request, options).finally(() => {
this.debug('Request %s for method %s took %dms', id, message.method, Date.now() - start);
});

Expand All @@ -291,7 +295,7 @@ export class DenoRuntimeSubprocessController extends EventEmitter {
});
}

private waitForResponse(req: jsonrpc.RequestObject): Promise<unknown> {
private waitForResponse(req: jsonrpc.RequestObject, options = this.options): Promise<unknown> {
return new Promise((resolve, reject) => {
const responseCallback = (result: unknown, error: jsonrpc.IParsedObjectError['payload']['error']) => {
clearTimeout(timeoutId);
Expand All @@ -308,7 +312,7 @@ export class DenoRuntimeSubprocessController extends EventEmitter {
const timeoutId = setTimeout(() => {
this.off(eventName, responseCallback);
reject(new Error(`[${this.getAppId()}] Request "${req.id}" for method "${req.method}" timed out`));
}, this.options.timeout);
}, options.timeout);

this.once(eventName, responseCallback);
});
Expand Down

0 comments on commit c3b4d2e

Please sign in to comment.