Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .server-changes/sentry-wrapper-bypass-fix.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
---
area: webapp
type: fix
---

Stop nine catch sites in the webapp from escalating expected user-input
failures (`ServiceValidationError`, `OutOfEntitlementError`,
`CreateDeclarativeScheduleError`, `QueryError`) as `error`-level events.
Type-discriminate before logging; downgrade the user-facing branches to
`warn` while keeping unknown-error fall-throughs at `error`.
Original file line number Diff line number Diff line change
Expand Up @@ -60,14 +60,20 @@ export async function action({ request, params }: ActionFunctionArgs) {
{ status: 200 }
);
} catch (e) {
logger.error("Failed to create background worker", { error: e });

// Customer-facing validation failures (invalid task config, customer cron
// expression, etc.). The handler returns 4xx with the message; system
// handles it gracefully, no alert needed.
if (e instanceof ServiceValidationError) {
logger.warn("Failed to create background worker", { error: e.message });
return json({ error: e.message }, { status: e.status ?? 400 });
} else if (e instanceof CreateDeclarativeScheduleError) {
}
if (e instanceof CreateDeclarativeScheduleError) {
logger.warn("Failed to create background worker", { error: e.message });
return json({ error: e.message }, { status: 400 });
}

logger.error("Failed to create background worker", { error: e });

return json({ error: "Failed to create background worker" }, { status: 500 });
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,20 @@ export async function action({ request, params }: ActionFunctionArgs) {
{ status: 200 }
);
} catch (e) {
logger.error("Failed to create background worker", { error: JSON.stringify(e) });

// Customer-facing validation failures (invalid task config, customer cron
// expression, etc.). The handler returns 4xx with the message; system
// handles it gracefully, no alert needed.
if (e instanceof ServiceValidationError) {
logger.warn("Failed to create background worker", { error: e.message });
return json({ error: e.message }, { status: 400 });
} else if (e instanceof CreateDeclarativeScheduleError) {
}
if (e instanceof CreateDeclarativeScheduleError) {
logger.warn("Failed to create background worker", { error: e.message });
return json({ error: e.message }, { status: 400 });
}

logger.error("Failed to create background worker", { error: e });

return json({ error: "Failed to create background worker" }, { status: 500 });
}
}
18 changes: 12 additions & 6 deletions apps/webapp/app/routes/api.v1.query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,19 +61,25 @@ const { action, loader } = createActionApiRoute(
});

if (!queryResult.success) {
const message =
queryResult.error instanceof QueryError
? queryResult.error.message
: "An unexpected error occurred while executing the query.";
// QueryError surfaces customer SQL problems (invalid syntax,
// unsupported construct). Returned to the caller as 400; system
// handles it gracefully, no alert needed.
if (queryResult.error instanceof QueryError) {
logger.warn("Query API error", {
error: queryResult.error.message,
query,
});
return json({ error: queryResult.error.message }, { status: 400 });
}

logger.error("Query API error", {
error: queryResult.error,
query,
});

return json(
{ error: message },
{ status: queryResult.error instanceof QueryError ? 400 : 500 }
{ error: "An unexpected error occurred while executing the query." },
{ status: 500 }
);
}

Expand Down
18 changes: 13 additions & 5 deletions apps/webapp/app/routes/api.v1.tasks.batch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,18 +127,26 @@ const { action, loader } = createActionApiRoute(

return json(batch, { status: 202, headers: $responseHeaders });
} catch (error) {
// Customer-facing validation/quota failures (invalid batch shape,
// entitlements exhausted). The handler returns 422 with the message;
// system handles it gracefully, no alert needed.
if (error instanceof ServiceValidationError) {
logger.warn("Batch trigger error", { error: error.message });
return json({ error: error.message }, { status: 422 });
}
if (error instanceof OutOfEntitlementError) {
logger.warn("Batch trigger error", { error: error.message });
return json({ error: error.message }, { status: 422 });
}

logger.error("Batch trigger error", {
error: {
message: (error as Error).message,
stack: (error as Error).stack,
},
});

if (error instanceof ServiceValidationError) {
return json({ error: error.message }, { status: 422 });
} else if (error instanceof OutOfEntitlementError) {
return json({ error: error.message }, { status: 422 });
} else if (error instanceof Error) {
if (error instanceof Error) {
return json(
{ error: "Something went wrong" },
{ status: 500, headers: { "x-should-retry": "false" } }
Expand Down
18 changes: 13 additions & 5 deletions apps/webapp/app/routes/api.v2.tasks.batch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,18 +144,26 @@ const { action, loader } = createActionApiRoute(
headers: $responseHeaders,
});
} catch (error) {
// Customer-facing validation/quota failures (invalid batch shape,
// entitlements exhausted). The handler returns 422 with the message;
// system handles it gracefully, no alert needed.
if (error instanceof ServiceValidationError) {
logger.warn("Batch trigger error", { error: error.message });
return json({ error: error.message }, { status: 422 });
}
if (error instanceof OutOfEntitlementError) {
logger.warn("Batch trigger error", { error: error.message });
return json({ error: error.message }, { status: 422 });
}

logger.error("Batch trigger error", {
error: {
message: (error as Error).message,
stack: (error as Error).stack,
},
});

if (error instanceof ServiceValidationError) {
return json({ error: error.message }, { status: 422 });
} else if (error instanceof OutOfEntitlementError) {
return json({ error: error.message }, { status: 422 });
} else if (error instanceof Error) {
if (error instanceof Error) {
return json(
{ error: error.message },
{ status: 500, headers: { "x-should-retry": "false" } }
Expand Down
25 changes: 17 additions & 8 deletions apps/webapp/app/routes/api.v3.batches.$batchId.items.ts
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,22 @@ export async function action({ request, params }: ActionFunctionArgs) {

return json(result, { status: 200 });
} catch (error) {
// Customer-facing validation failures (invalid item shape, invalid JSON
// in the streamed body). The handler returns 4xx with the message;
// system handles it gracefully, no alert needed.
if (error instanceof ServiceValidationError) {
logger.warn("Stream batch items error", { batchId, error: error.message });
return json({ error: error.message }, { status: 422 });
}

if (error instanceof Error && error.message.includes("Invalid JSON")) {
logger.warn("Stream batch items error: invalid JSON", {
batchId,
error: error.message,
});
return json({ error: error.message }, { status: 400 });
}

logger.error("Stream batch items error", {
batchId,
error: {
Expand All @@ -96,14 +112,7 @@ export async function action({ request, params }: ActionFunctionArgs) {
},
});

if (error instanceof ServiceValidationError) {
return json({ error: error.message }, { status: 422 });
} else if (error instanceof Error) {
// Check for stream parsing errors (e.g. invalid JSON)
if (error.message.includes("Invalid JSON")) {
return json({ error: error.message }, { status: 400 });
}

if (error instanceof Error) {
return json({ error: error.message }, { status: 500 });
}

Expand Down
18 changes: 13 additions & 5 deletions apps/webapp/app/routes/api.v3.batches.ts
Original file line number Diff line number Diff line change
Expand Up @@ -172,18 +172,26 @@ const { action, loader } = createActionApiRoute(
);
}

// Customer-facing validation/quota failures (invalid batch shape,
// entitlements exhausted). The handler returns 422 with the message;
// system handles it gracefully, no alert needed.
if (error instanceof ServiceValidationError) {
logger.warn("Create batch error", { error: error.message });
return json({ error: error.message }, { status: error.status ?? 422 });
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
if (error instanceof OutOfEntitlementError) {
logger.warn("Create batch error", { error: error.message });
return json({ error: error.message }, { status: 422 });
}

logger.error("Create batch error", {
error: {
message: (error as Error).message,
stack: (error as Error).stack,
},
});

if (error instanceof ServiceValidationError) {
return json({ error: error.message }, { status: 422 });
} else if (error instanceof OutOfEntitlementError) {
return json({ error: error.message }, { status: 422 });
} else if (error instanceof Error) {
if (error instanceof Error) {
return json(
{ error: error.message },
{ status: 500, headers: { "x-should-retry": "false" } }
Expand Down
19 changes: 15 additions & 4 deletions apps/webapp/app/v3/services/createBackgroundWorker.server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,16 +146,27 @@ export class CreateBackgroundWorkerService extends BaseService {
);

if (schedulesError) {
if (schedulesError instanceof ServiceValidationError) {
// Customer schedule config (typically invalid cron). Surface to
// client via the rethrow; system returns gracefully.
logger.warn("Error syncing declarative schedules", {
error: schedulesError.message,
backgroundWorker,
environment,
});
throw schedulesError;
}

// Wrapping the underlying error into a ServiceValidationError below
// would otherwise hide it once the SDK-level filter drops SVEs; log at
// error so the underlying cause stays visible. Mirrors the
// waitpointCompletionPacket.server.ts pattern from dac9c83bd.
logger.error("Error syncing declarative schedules", {
error: schedulesError,
backgroundWorker,
environment,
});

if (schedulesError instanceof ServiceValidationError) {
throw schedulesError;
}

throw new ServiceValidationError("Error syncing declarative schedules");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -139,14 +139,26 @@ export class CreateDeploymentBackgroundWorkerServiceV4 extends BaseService {
);

if (schedulesError) {
if (schedulesError instanceof ServiceValidationError) {
// Customer schedule config (typically invalid cron). Surface to
// client via the rethrow; system returns gracefully.
logger.warn("Error syncing declarative schedules", {
error: schedulesError.message,
});

await this.#failBackgroundWorkerDeployment(deployment, schedulesError);
throw schedulesError;
}

// Wrapping the underlying error into a ServiceValidationError below
// would otherwise hide it once the SDK-level filter drops SVEs; log at
// error so the underlying cause stays visible. Mirrors the
// waitpointCompletionPacket.server.ts pattern from dac9c83bd.
logger.error("Error syncing declarative schedules", {
error: schedulesError,
});

const serviceError =
schedulesError instanceof ServiceValidationError
? schedulesError
: new ServiceValidationError("Error syncing declarative schedules");
const serviceError = new ServiceValidationError("Error syncing declarative schedules");

await this.#failBackgroundWorkerDeployment(deployment, serviceError);

Expand Down
Loading