Skip to content

Commit 2eddda1

Browse files
authored
fix(webapp): worker actions now catch service validation errors and respond properly (#2481)
This also stops all the unnecessary error logging when throwing ServiceValidationErrors
1 parent 49f2c54 commit 2eddda1

File tree

10 files changed

+41
-31
lines changed

10 files changed

+41
-31
lines changed

apps/webapp/app/routes/api.v1.tasks.$taskId.trigger.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { EngineServiceValidationError } from "@internal/run-engine";
12
import { json } from "@remix-run/server-runtime";
23
import {
34
generateJWT as internal_generateJWT,
@@ -8,7 +9,6 @@ import { TaskRun } from "@trigger.dev/database";
89
import { z } from "zod";
910
import { prisma } from "~/db.server";
1011
import { env } from "~/env.server";
11-
import { EngineServiceValidationError } from "~/runEngine/concerns/errors";
1212
import { ApiAuthenticationResultSuccess, getOneTimeUseToken } from "~/services/apiAuth.server";
1313
import { logger } from "~/services/logger.server";
1414
import { createActionApiRoute } from "~/services/routeBuilders/apiBuilder.server";

apps/webapp/app/runEngine/concerns/errors.ts

Lines changed: 0 additions & 6 deletions
This file was deleted.

apps/webapp/app/runEngine/concerns/payloads.server.ts

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { PayloadProcessor, TriggerTaskRequest } from "../types";
33
import { env } from "~/env.server";
44
import { startActiveSpan } from "~/v3/tracer.server";
55
import { uploadPacketToObjectStore } from "~/v3/r2.server";
6-
import { EngineServiceValidationError } from "./errors";
6+
import { ServiceValidationError } from "~/v3/services/common.server";
77

88
export class DefaultPayloadProcessor implements PayloadProcessor {
99
async process(request: TriggerTaskRequest): Promise<IOPacket> {
@@ -36,10 +36,7 @@ export class DefaultPayloadProcessor implements PayloadProcessor {
3636
);
3737

3838
if (uploadError) {
39-
throw new EngineServiceValidationError(
40-
"Failed to upload large payload to object store",
41-
500
42-
); // This is retryable
39+
throw new ServiceValidationError("Failed to upload large payload to object store", 500); // This is retryable
4340
}
4441

4542
return {

apps/webapp/app/runEngine/concerns/queues.server.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,8 @@ import {
1313
import { WorkerGroupService } from "~/v3/services/worker/workerGroupService.server";
1414
import type { RunEngine } from "~/v3/runEngine.server";
1515
import { env } from "~/env.server";
16-
import { EngineServiceValidationError } from "./errors";
1716
import { tryCatch } from "@trigger.dev/core/v3";
17+
import { ServiceValidationError } from "~/v3/services/common.server";
1818

1919
export class DefaultQueueManager implements QueueManager {
2020
constructor(
@@ -45,7 +45,7 @@ export class DefaultQueueManager implements QueueManager {
4545
});
4646

4747
if (!specifiedQueue) {
48-
throw new EngineServiceValidationError(
48+
throw new ServiceValidationError(
4949
`Specified queue '${specifiedQueueName}' not found or not associated with locked version '${
5050
lockedBackgroundWorker.version ?? "<unknown>"
5151
}'.`
@@ -68,7 +68,7 @@ export class DefaultQueueManager implements QueueManager {
6868
});
6969

7070
if (!lockedTask) {
71-
throw new EngineServiceValidationError(
71+
throw new ServiceValidationError(
7272
`Task '${request.taskId}' not found on locked version '${
7373
lockedBackgroundWorker.version ?? "<unknown>"
7474
}'.`
@@ -83,7 +83,7 @@ export class DefaultQueueManager implements QueueManager {
8383
workerId: lockedBackgroundWorker.id,
8484
version: lockedBackgroundWorker.version,
8585
});
86-
throw new EngineServiceValidationError(
86+
throw new ServiceValidationError(
8787
`Default queue configuration for task '${request.taskId}' missing on locked version '${
8888
lockedBackgroundWorker.version ?? "<unknown>"
8989
}'.`
@@ -97,7 +97,7 @@ export class DefaultQueueManager implements QueueManager {
9797
// Task is not locked to a specific version, use regular logic
9898
if (request.body.options?.lockToVersion) {
9999
// This should only happen if the findFirst failed, indicating the version doesn't exist
100-
throw new EngineServiceValidationError(
100+
throw new ServiceValidationError(
101101
`Task locked to version '${request.body.options.lockToVersion}', but no worker found with that version.`
102102
);
103103
}
@@ -221,11 +221,11 @@ export class DefaultQueueManager implements QueueManager {
221221
);
222222

223223
if (error) {
224-
throw new EngineServiceValidationError(error.message);
224+
throw new ServiceValidationError(error.message);
225225
}
226226

227227
if (!workerGroup) {
228-
throw new EngineServiceValidationError("No worker group found");
228+
throw new ServiceValidationError("No worker group found");
229229
}
230230

231231
return workerGroup.masterQueue;

apps/webapp/app/runEngine/services/triggerTask.server.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,6 @@ import type {
3131
} from "../../v3/services/triggerTask.server";
3232
import { getTaskEventStore } from "../../v3/taskEventStore.server";
3333
import { clampMaxDuration } from "../../v3/utils/maxDuration";
34-
import { EngineServiceValidationError } from "../concerns/errors";
3534
import { IdempotencyKeyConcern } from "../concerns/idempotencyKeys.server";
3635
import type {
3736
PayloadProcessor,
@@ -41,6 +40,7 @@ import type {
4140
TriggerTaskRequest,
4241
TriggerTaskValidator,
4342
} from "../types";
43+
import { ServiceValidationError } from "~/v3/services/common.server";
4444

4545
export class RunEngineTriggerTaskService {
4646
private readonly queueConcern: QueueManager;
@@ -157,7 +157,7 @@ export class RunEngineTriggerTaskService {
157157
const [parseDelayError, delayUntil] = await tryCatch(parseDelay(body.options?.delay));
158158

159159
if (parseDelayError) {
160-
throw new EngineServiceValidationError(`Invalid delay ${body.options?.delay}`);
160+
throw new ServiceValidationError(`Invalid delay ${body.options?.delay}`);
161161
}
162162

163163
const ttl =
@@ -210,7 +210,7 @@ export class RunEngineTriggerTaskService {
210210
});
211211

212212
if (!queueSizeGuard.ok) {
213-
throw new EngineServiceValidationError(
213+
throw new ServiceValidationError(
214214
`Cannot trigger ${taskId} as the queue size limit for this environment has been reached. The maximum size is ${queueSizeGuard.maximumSize}`
215215
);
216216
}
@@ -351,7 +351,7 @@ export class RunEngineTriggerTaskService {
351351
);
352352

353353
if (result?.error) {
354-
throw new EngineServiceValidationError(
354+
throw new ServiceValidationError(
355355
taskRunErrorToString(taskRunErrorEnhancer(result.error))
356356
);
357357
}
@@ -365,7 +365,7 @@ export class RunEngineTriggerTaskService {
365365
}
366366

367367
if (error instanceof RunOneTimeUseTokenError) {
368-
throw new EngineServiceValidationError(
368+
throw new ServiceValidationError(
369369
`Cannot trigger ${taskId} with a one-time use token as it has already been used.`
370370
);
371371
}

apps/webapp/app/runEngine/validators/triggerTaskValidator.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ import { logger } from "~/services/logger.server";
33
import { getEntitlement } from "~/services/platform.v3.server";
44
import { MAX_ATTEMPTS, OutOfEntitlementError } from "~/v3/services/triggerTask.server";
55
import { isFinalRunStatus } from "~/v3/taskStatus";
6-
import { EngineServiceValidationError } from "../concerns/errors";
76
import type {
87
EntitlementValidationParams,
98
EntitlementValidationResult,
@@ -13,6 +12,7 @@ import type {
1312
TriggerTaskValidator,
1413
ValidationResult,
1514
} from "../types";
15+
import { ServiceValidationError } from "~/v3/services/common.server";
1616

1717
export class DefaultTriggerTaskValidator implements TriggerTaskValidator {
1818
validateTags(params: TagValidationParams): ValidationResult {
@@ -29,7 +29,7 @@ export class DefaultTriggerTaskValidator implements TriggerTaskValidator {
2929
if (tags.length > MAX_TAGS_PER_RUN) {
3030
return {
3131
ok: false,
32-
error: new EngineServiceValidationError(
32+
error: new ServiceValidationError(
3333
`Runs can only have ${MAX_TAGS_PER_RUN} tags, you're trying to set ${tags.length}.`
3434
),
3535
};
@@ -65,7 +65,7 @@ export class DefaultTriggerTaskValidator implements TriggerTaskValidator {
6565
if (attempt > MAX_ATTEMPTS) {
6666
return {
6767
ok: false,
68-
error: new EngineServiceValidationError(
68+
error: new ServiceValidationError(
6969
`Failed to trigger ${taskId} after ${MAX_ATTEMPTS} attempts.`
7070
),
7171
};
@@ -95,7 +95,7 @@ export class DefaultTriggerTaskValidator implements TriggerTaskValidator {
9595

9696
return {
9797
ok: false,
98-
error: new EngineServiceValidationError(
98+
error: new ServiceValidationError(
9999
`Cannot trigger ${taskId} as the parent run has a status of ${parentRun.status}`
100100
),
101101
};

apps/webapp/app/services/routeBuilders/apiBuilder.server.ts

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ import {
2323
} from "~/v3/services/worker/workerGroupTokenService.server";
2424
import { API_VERSIONS, getApiVersion } from "~/api/versions";
2525
import { WORKER_HEADERS } from "@trigger.dev/core/v3/runEngineWorker";
26+
import { ServiceValidationError } from "~/v3/services/common.server";
27+
import { EngineServiceValidationError } from "@internal/run-engine";
2628

2729
type AnyZodSchema = z.ZodFirstPartySchemaTypes | z.ZodDiscriminatedUnion<any, any>;
2830

@@ -1040,11 +1042,18 @@ export function createActionWorkerApiRoute<
10401042
});
10411043
return result;
10421044
} catch (error) {
1043-
console.error("Error in API route:", error);
10441045
if (error instanceof Response) {
10451046
return error;
10461047
}
10471048

1049+
if (error instanceof EngineServiceValidationError) {
1050+
return json({ error: error.message }, { status: error.status ?? 422 });
1051+
}
1052+
1053+
if (error instanceof ServiceValidationError) {
1054+
return json({ error: error.message }, { status: error.status ?? 422 });
1055+
}
1056+
10481057
logger.error("Error in action", {
10491058
error:
10501059
error instanceof Error

internal-packages/run-engine/src/engine/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { BillingCache } from "./billingCache.js";
21
import { createRedisClient, Redis } from "@internal/redis";
32
import { getMeter, Meter, startSpan, trace, Tracer } from "@internal/tracing";
43
import { Logger } from "@trigger.dev/core/logger";
@@ -32,6 +31,7 @@ import { FairQueueSelectionStrategy } from "../run-queue/fairQueueSelectionStrat
3231
import { RunQueue } from "../run-queue/index.js";
3332
import { RunQueueFullKeyProducer } from "../run-queue/keyProducer.js";
3433
import { MinimalAuthenticatedEnvironment } from "../shared/index.js";
34+
import { BillingCache } from "./billingCache.js";
3535
import { NotImplementedError, RunDuplicateIdempotencyKeyError } from "./errors.js";
3636
import { EventBus, EventBusEvents } from "./eventBus.js";
3737
import { RunLocker } from "./locking.js";

internal-packages/run-engine/src/engine/locking.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
Attributes,
1616
Histogram,
1717
} from "@internal/tracing";
18+
import { ServiceValidationError } from "./errors.js";
1819

1920
const SemanticAttributes = {
2021
LOCK_TYPE: "run_engine.lock.type",
@@ -174,6 +175,10 @@ export class RunLocker {
174175
);
175176

176177
if (error) {
178+
if (error instanceof ServiceValidationError) {
179+
throw error;
180+
}
181+
177182
// Record failed lock acquisition
178183
const lockDuration = performance.now() - lockStartTime;
179184
this.lockDurationHistogram.record(lockDuration, {
@@ -186,6 +191,7 @@ export class RunLocker {
186191
resources,
187192
duration: this.duration,
188193
});
194+
189195
throw error;
190196
}
191197

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
11
export { RunEngine } from "./engine/index.js";
2-
export { RunDuplicateIdempotencyKeyError, RunOneTimeUseTokenError } from "./engine/errors.js";
2+
export {
3+
RunDuplicateIdempotencyKeyError,
4+
RunOneTimeUseTokenError,
5+
ServiceValidationError as EngineServiceValidationError,
6+
} from "./engine/errors.js";
37
export type { EventBusEventArgs, EventBusEvents } from "./engine/eventBus.js";
48
export type { AuthenticatedEnvironment } from "./shared/index.js";

0 commit comments

Comments
 (0)