From 68468fadb349dd54f64f0cccca2e3b091c6374a6 Mon Sep 17 00:00:00 2001 From: Sam Rose Date: Thu, 4 Jan 2024 10:10:00 +0000 Subject: [PATCH] Add an extra JS execution time limit check to prevent creating unnecesary VM context. --- packages/backend-core/src/timers/timers.ts | 2 +- packages/server/src/jsRunner.ts | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/packages/backend-core/src/timers/timers.ts b/packages/backend-core/src/timers/timers.ts index 9de57af7f1..9121c576cd 100644 --- a/packages/backend-core/src/timers/timers.ts +++ b/packages/backend-core/src/timers/timers.ts @@ -50,7 +50,7 @@ export class ExecutionTimeTracker { return this.totalTimeMs } - private checkLimit() { + checkLimit() { if (this.totalTimeMs > this.limitMs) { throw new ExecutionTimeoutError( `Execution time limit of ${this.limitMs}ms exceeded: ${this.totalTimeMs}ms` diff --git a/packages/server/src/jsRunner.ts b/packages/server/src/jsRunner.ts index ab0381a399..a9dcd506d7 100644 --- a/packages/server/src/jsRunner.ts +++ b/packages/server/src/jsRunner.ts @@ -18,13 +18,16 @@ export function init() { bbCtx.jsExecutionTracker = timers.ExecutionTimeTracker.withLimit(perRequestLimit) } - track = bbCtx.jsExecutionTracker.track.bind(bbCtx.jsExecutionTracker) span?.addTags({ js: { limitMS: bbCtx.jsExecutionTracker.limitMs, elapsedMS: bbCtx.jsExecutionTracker.elapsedMS, }, }) + // We call checkLimit() here to prevent paying the cost of creating + // a new VM context below when we don't need to. + bbCtx.jsExecutionTracker.checkLimit() + track = bbCtx.jsExecutionTracker.track.bind(bbCtx.jsExecutionTracker) } }