Skip to content

Commit c0c7b53

Browse files
Copilothotlong
andcommitted
Implement reviewer suggestion: reject timeout configs upfront instead of post-execution check
Co-authored-by: hotlong <[email protected]>
1 parent 78b44c2 commit c0c7b53

File tree

1 file changed

+13
-18
lines changed

1 file changed

+13
-18
lines changed

packages/foundation/core/src/formula-engine.ts

Lines changed: 13 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ export class FormulaEngine {
4040
this.config = {
4141
enable_cache: config.enable_cache ?? false,
4242
cache_ttl: config.cache_ttl ?? 300,
43-
max_execution_time: config.max_execution_time ?? 1000,
43+
max_execution_time: config.max_execution_time ?? 0, // 0 means no timeout enforcement
4444
enable_monitoring: config.enable_monitoring ?? false,
4545
custom_functions: config.custom_functions ?? {},
4646
sandbox: {
@@ -68,7 +68,7 @@ export class FormulaEngine {
6868
options: FormulaEvaluationOptions = {}
6969
): FormulaEvaluationResult {
7070
const startTime = Date.now();
71-
const timeout = options.timeout ?? this.config.max_execution_time ?? 1000;
71+
const timeout = options.timeout ?? this.config.max_execution_time ?? 0;
7272

7373
try {
7474
// Validate expression
@@ -276,34 +276,29 @@ export class FormulaEngine {
276276
* Execute function with timeout protection
277277
*
278278
* NOTE: This synchronous implementation **cannot** pre-emptively interrupt execution.
279-
* The timeout check occurs AFTER the formula completes. Long-running formulas will
280-
* still block execution. This is a limitation of synchronous JavaScript.
281-
*
282-
* For true interruption, use platform-specific mechanisms like Worker threads or
283-
* migrate to an async/isolated runtime. Formulas should be written to be fast.
279+
* To avoid giving a false sense of safety, any positive finite timeout configuration
280+
* is rejected up-front. Callers must not rely on timeout-based protection in this
281+
* runtime; instead, formulas must be written to be fast and side-effect free.
284282
*/
285283
private executeWithTimeout(
286284
func: Function,
287285
args: any[],
288286
timeout: number
289287
): unknown {
290-
// Execute the formula synchronously
291-
const startTime = Date.now();
292-
const result = func(...args);
293-
const elapsed = Date.now() - startTime;
294-
295-
// Check timeout AFTER execution (cannot interrupt synchronous code)
296-
if (elapsed > timeout) {
288+
// Reject any positive finite timeout to avoid misleading "protection" semantics.
289+
if (Number.isFinite(timeout) && timeout > 0) {
297290
throw new FormulaError(
298291
FormulaErrorType.TIMEOUT,
299-
`Formula execution exceeded timeout of ${timeout}ms (actual: ${elapsed}ms). ` +
300-
`Note: Synchronous formulas cannot be interrupted. Write fast formulas or use async runtime.`,
292+
'Formula timeout enforcement is not supported for synchronous execution. ' +
293+
'Remove the timeout configuration or migrate to an async/isolated runtime ' +
294+
'that can safely interrupt long-running formulas.',
301295
'',
302-
{ elapsed, timeout }
296+
{ requestedTimeoutMs: timeout }
303297
);
304298
}
305299

306-
return result;
300+
// No timeout configured (or non-positive/invalid value): execute directly.
301+
return func(...args);
307302
}
308303

309304
/**

0 commit comments

Comments
 (0)