fix(webapp): clamp run priority so a large value can't fail run creation (#4512)
## Summary Triggering a run with a very large `priority` could fail run creation outright with an opaque database error. `priority` is multiplied by 1000 and stored in a 32-bit integer column, with nothing bounding it, so a big enough value overflowed the column and the create failed. The trigger now caps the value to the highest supported priority instead of erroring, so the run is still created. ## Fix `priorityMs` (the stored `priority * 1000`) now goes through a `clampPriorityMs` helper before the write. It rounds to a whole number and clamps into the column range at both ends, so only a valid integer ever reaches the column and an out-of-range priority caps rather than failing. Single and batch triggers share the write path, so both are covered.
This commit is contained in:
@@ -0,0 +1,6 @@
|
||||
---
|
||||
area: webapp
|
||||
type: fix
|
||||
---
|
||||
|
||||
Triggering a run with a very large `priority` no longer fails. The priority is now capped to the highest supported value instead of erroring out.
|
||||
@@ -33,6 +33,7 @@ import type {
|
||||
TriggerTaskServiceResult,
|
||||
} from "../../v3/services/triggerTask.server";
|
||||
import { clampMaxDuration } from "../../v3/utils/maxDuration";
|
||||
import { clampPriorityMs } from "../../v3/utils/priority";
|
||||
import {
|
||||
type IdempotencyKeyConcern,
|
||||
type ClaimedIdempotency,
|
||||
@@ -887,7 +888,9 @@ export class RunEngineTriggerTaskService {
|
||||
? clampMaxDuration(args.body.options.maxDuration)
|
||||
: undefined,
|
||||
machine: args.body.options?.machine,
|
||||
priorityMs: args.body.options?.priority ? args.body.options.priority * 1_000 : undefined,
|
||||
priorityMs: args.body.options?.priority
|
||||
? clampPriorityMs(args.body.options.priority)
|
||||
: undefined,
|
||||
queueTimestamp:
|
||||
args.options.queueTimestamp ??
|
||||
(args.parentRun && args.body.options?.resumeParentOnCompletion
|
||||
|
||||
@@ -0,0 +1,41 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { clampPriorityMs } from "./priority";
|
||||
|
||||
const INT4_MIN = -2_147_483_648;
|
||||
const INT4_MAX = 2_147_483_647;
|
||||
|
||||
describe("clampPriorityMs", () => {
|
||||
it("converts seconds to milliseconds for in-range values", () => {
|
||||
expect(clampPriorityMs(10)).toBe(10_000);
|
||||
expect(clampPriorityMs(0.5)).toBe(500);
|
||||
});
|
||||
|
||||
it("rounds a sub-millisecond fractional priority to an integer", () => {
|
||||
expect(clampPriorityMs(0.0005)).toBe(1);
|
||||
expect(clampPriorityMs(0.00049)).toBe(0);
|
||||
expect(Number.isInteger(clampPriorityMs(0.0005))).toBe(true);
|
||||
});
|
||||
|
||||
it("clamps a value that would overflow INT4 down to the column max", () => {
|
||||
const priority = 31_536_000;
|
||||
expect(priority * 1_000).toBeGreaterThan(INT4_MAX);
|
||||
expect(clampPriorityMs(priority)).toBe(INT4_MAX);
|
||||
});
|
||||
|
||||
it("leaves the largest safe priority untouched", () => {
|
||||
expect(clampPriorityMs(2_147_483)).toBe(2_147_483_000);
|
||||
});
|
||||
|
||||
it("clamps a large negative priority to the column min", () => {
|
||||
expect(clampPriorityMs(-3_000_000)).toBe(INT4_MIN);
|
||||
});
|
||||
|
||||
it("keeps every result inside the INT4 range", () => {
|
||||
for (const priority of [-1e12, -5, -0.3, 0, 0.7, 5, 1234.5678, 1e12]) {
|
||||
const result = clampPriorityMs(priority);
|
||||
expect(Number.isInteger(result)).toBe(true);
|
||||
expect(result).toBeGreaterThanOrEqual(INT4_MIN);
|
||||
expect(result).toBeLessThanOrEqual(INT4_MAX);
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,6 @@
|
||||
const INT4_MIN = -2_147_483_648;
|
||||
const INT4_MAX = 2_147_483_647;
|
||||
|
||||
export function clampPriorityMs(priority: number): number {
|
||||
return Math.min(Math.max(Math.round(priority * 1_000), INT4_MIN), INT4_MAX);
|
||||
}
|
||||
Reference in New Issue
Block a user