Skip to content

Commit b2608f1

Browse files
committed
Coderabbit fixes
1 parent 8feff7d commit b2608f1

File tree

8 files changed

+77
-42
lines changed

8 files changed

+77
-42
lines changed

apps/webapp/app/presenters/v3/ErrorsListPresenter.server.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -234,13 +234,13 @@ export class ErrorsListPresenter extends BasePresenter {
234234

235235
if (statusFilter.includeKeys) {
236236
queryBuilder.where(
237-
"concat(task_identifier, '::', error_fingerprint) IN ({statusIncludeKeys: Array(String)})",
237+
"concat(task_identifier, '::', error_fingerprint) IN {statusIncludeKeys: Array(String)}",
238238
{ statusIncludeKeys: statusFilter.includeKeys }
239239
);
240240
}
241241
if (statusFilter.excludeKeys) {
242242
queryBuilder.where(
243-
"concat(task_identifier, '::', error_fingerprint) NOT IN ({statusExcludeKeys: Array(String)})",
243+
"concat(task_identifier, '::', error_fingerprint) NOT IN {statusExcludeKeys: Array(String)}",
244244
{ statusExcludeKeys: statusFilter.excludeKeys }
245245
);
246246
}

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors.$fingerprint/route.tsx

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
import { type LoaderFunctionArgs, type ActionFunctionArgs, json } from "@remix-run/server-runtime";
22
import { type MetaFunction, Form, useNavigation, useSubmit } from "@remix-run/react";
33
import { BellAlertIcon } from "@heroicons/react/20/solid";
4-
import {
5-
IconAlarmSnooze as IconAlarmSnoozeBase,
6-
IconCircleDotted,
7-
} from "@tabler/icons-react";
4+
import { IconAlarmSnooze as IconAlarmSnoozeBase, IconCircleDotted } from "@tabler/icons-react";
85
import { parse } from "@conform-to/zod";
96
import { z } from "zod";
107
import { ErrorStatusBadge } from "~/components/errors/ErrorStatusBadge";
@@ -77,11 +74,7 @@ import { cn } from "~/utils/cn";
7774
import { LogsVersionFilter } from "~/components/logs/LogsVersionFilter";
7875
import { CodeBlock } from "~/components/code/CodeBlock";
7976
import { getSeriesColor } from "~/components/code/chartColors";
80-
import {
81-
Popover,
82-
PopoverArrowTrigger,
83-
PopoverContent,
84-
} from "~/components/primitives/Popover";
77+
import { Popover, PopoverArrowTrigger, PopoverContent } from "~/components/primitives/Popover";
8578
import { ErrorGroupActions } from "~/v3/services/errorGroupActions.server";
8679
import { ErrorStatusMenuItems, CustomIgnoreDialog } from "~/components/errors/ErrorStatusMenu";
8780

@@ -573,7 +566,7 @@ function ErrorDetailSidebar({
573566
</Property.Item>
574567

575568
{/* Error message */}
576-
<Property.Item>
569+
<Property.Item className="gap-1">
577570
<Property.Label>Error</Property.Label>
578571
<Property.Value>
579572
<CodeBlock

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors._index/route.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,10 @@ function FiltersBar({
403403
searchParams.has("status") ||
404404
searchParams.has("tasks") ||
405405
searchParams.has("versions") ||
406-
searchParams.has("search");
406+
searchParams.has("search") ||
407+
searchParams.has("period") ||
408+
searchParams.has("from") ||
409+
searchParams.has("to");
407410

408411
return (
409412
<div className="flex items-start justify-between gap-x-2 border-b border-grid-bright p-2">

apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.errors/route.tsx

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,9 @@ export const action = async ({ request, params }: ActionFunctionArgs) => {
8787

8888
const { emails, webhooks, slackChannel, slackIntegrationId } = submission.value;
8989

90+
const emailEnabled = env.ALERT_FROM_EMAIL !== undefined && env.ALERT_RESEND_API_KEY !== undefined;
91+
const slackEnabled = !!slackIntegrationId;
92+
9093
const existingChannels = await prisma.projectAlertChannel.findMany({
9194
where: {
9295
projectId: project.id,
@@ -99,19 +102,21 @@ export const action = async ({ request, params }: ActionFunctionArgs) => {
99102
const environmentTypes = [environment.type];
100103
const processedChannelIds = new Set<string>();
101104

102-
for (const email of emails) {
103-
const options: CreateAlertChannelOptions = {
104-
name: `Error alert to ${email}`,
105-
alertTypes: ["ERROR_GROUP"],
106-
environmentTypes,
107-
deduplicationKey: `error-email:${email}:${environment.type}`,
108-
channel: { type: "EMAIL", email },
109-
};
110-
const channel = await service.call(project.externalRef, userId, options);
111-
processedChannelIds.add(channel.id);
105+
if (emailEnabled) {
106+
for (const email of emails) {
107+
const options: CreateAlertChannelOptions = {
108+
name: `Error alert to ${email}`,
109+
alertTypes: ["ERROR_GROUP"],
110+
environmentTypes,
111+
deduplicationKey: `error-email:${email}:${environment.type}`,
112+
channel: { type: "EMAIL", email },
113+
};
114+
const channel = await service.call(project.externalRef, userId, options);
115+
processedChannelIds.add(channel.id);
116+
}
112117
}
113118

114-
if (slackChannel) {
119+
if (slackEnabled && slackChannel) {
115120
const [channelId, channelName] = slackChannel.split("/");
116121
if (channelId && channelName) {
117122
const options: CreateAlertChannelOptions = {
@@ -144,10 +149,10 @@ export const action = async ({ request, params }: ActionFunctionArgs) => {
144149
}
145150

146151
const editableTypes = new Set<string>(["WEBHOOK"]);
147-
if (env.ALERT_FROM_EMAIL !== undefined && env.ALERT_RESEND_API_KEY !== undefined) {
152+
if (emailEnabled) {
148153
editableTypes.add("EMAIL");
149154
}
150-
if (slackIntegrationId) {
155+
if (slackEnabled) {
151156
editableTypes.add("SLACK");
152157
}
153158

apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
11
import {
22
type ChatPostMessageArguments,
33
ErrorCode,
4-
type WebAPIHTTPError,
54
type WebAPIPlatformError,
65
type WebAPIRateLimitedError,
7-
type WebAPIRequestError,
86
} from "@slack/web-api";
97
import { type ProjectAlertChannelType } from "@trigger.dev/database";
108
import assertNever from "assert-never";

apps/webapp/app/v3/services/alerts/errorAlertEvaluator.server.ts

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,18 @@ export class ErrorAlertEvaluator {
7474
}
7575

7676
const allEnvTypes = this.collectEnvironmentTypes(channels);
77-
const environments = await this.resolveEnvironments(projectId, allEnvTypes);
77+
const [project, environments] = await Promise.all([
78+
this._replica.project.findFirst({
79+
where: { id: projectId },
80+
select: { organizationId: true },
81+
}),
82+
this.resolveEnvironments(projectId, allEnvTypes),
83+
]);
84+
85+
if (!project) {
86+
logger.error("[ErrorAlertEvaluator] Project not found", { projectId });
87+
return;
88+
}
7889

7990
if (environments.length === 0) {
8091
logger.info("[ErrorAlertEvaluator] No matching environments found", { projectId });
@@ -86,7 +97,12 @@ export class ErrorAlertEvaluator {
8697
const envMap = new Map(environments.map((e) => [e.id, e]));
8798
const channelsByEnvId = this.buildChannelsByEnvId(channels, environments);
8899

89-
const activeErrors = await this.getActiveErrors(projectId, envIds, scheduledAt);
100+
const activeErrors = await this.getActiveErrors(
101+
project.organizationId,
102+
projectId,
103+
envIds,
104+
scheduledAt
105+
);
90106

91107
if (activeErrors.length === 0) {
92108
await this.selfChain(projectId, nextScheduledAt, minIntervalMs);
@@ -96,7 +112,12 @@ export class ErrorAlertEvaluator {
96112
const states = await this.getErrorGroupStates(activeErrors);
97113
const stateMap = this.buildStateMap(states);
98114

99-
const occurrenceCounts = await this.getOccurrenceCountsSince(projectId, envIds, scheduledAt);
115+
const occurrenceCounts = await this.getOccurrenceCountsSince(
116+
project.organizationId,
117+
projectId,
118+
envIds,
119+
scheduledAt
120+
);
100121
const occurrenceMap = this.buildOccurrenceMap(occurrenceCounts);
101122

102123
const alertableErrors: AlertableError[] = [];
@@ -301,11 +322,13 @@ export class ErrorAlertEvaluator {
301322
}
302323

303324
private async getActiveErrors(
325+
organizationId: string,
304326
projectId: string,
305327
envIds: string[],
306328
scheduledAt: number
307329
): Promise<ActiveErrorsSinceQueryResult[]> {
308330
const qb = this._clickhouse.errors.activeErrorsSinceQueryBuilder();
331+
qb.where("organization_id = {organizationId: String}", { organizationId });
309332
qb.where("project_id = {projectId: String}", { projectId });
310333
qb.where("environment_id IN {envIds: Array(String)}", { envIds });
311334
qb.groupBy("environment_id, task_identifier, error_fingerprint");
@@ -346,6 +369,7 @@ export class ErrorAlertEvaluator {
346369
}
347370

348371
private async getOccurrenceCountsSince(
372+
organizationId: string,
349373
projectId: string,
350374
envIds: string[],
351375
scheduledAt: number
@@ -358,6 +382,7 @@ export class ErrorAlertEvaluator {
358382
}>
359383
> {
360384
const qb = this._clickhouse.errors.occurrenceCountsSinceQueryBuilder();
385+
qb.where("organization_id = {organizationId: String}", { organizationId });
361386
qb.where("project_id = {projectId: String}", { projectId });
362387
qb.where("environment_id IN {envIds: Array(String)}", { envIds });
363388
qb.where("minute >= toStartOfMinute(fromUnixTimestamp64Milli({scheduledAt: Int64}))", {

apps/webapp/app/v3/services/alerts/errorGroupWebhook.server.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import type { ErrorWebhook } from "@trigger.dev/core/v3/schemas";
33

44
export type ErrorAlertClassification = "new_issue" | "regression" | "unignored";
55

6-
export interface ErrorGroupAlertData {
6+
export type ErrorGroupAlertData = {
77
classification: ErrorAlertClassification;
88
error: {
99
fingerprint: string;
@@ -29,7 +29,7 @@ export interface ErrorGroupAlertData {
2929
name: string;
3030
};
3131
dashboardUrl: string;
32-
}
32+
};
3333

3434
/**
3535
* Generates a webhook payload for an error group alert that conforms to the

apps/webapp/test/slackErrorAlerts.test.ts

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,11 @@ type ErrorAlertPayload = {
2323
};
2424
};
2525

26-
// Test context for database setup
2726
let testChannelId: string;
2827
let testProjectId: string;
2928
let testOrganizationId: string;
29+
let testSecretKey: string;
30+
let testSecretReferenceId: string;
3031

3132
// Helper to create mock error payloads
3233
function createMockErrorPayload(
@@ -90,20 +91,20 @@ describe.skipIf(!hasSlackCredentials)("Slack Error Alert Visual Tests", () => {
9091
});
9192
testProjectId = project.id;
9293

93-
// Create secret reference for Slack token
9494
const secretStore = getSecretStore("DATABASE");
95-
const secretKey = `slack-test-token-${Date.now()}`;
95+
testSecretKey = `slack-test-token-${Date.now()}`;
9696

97-
await secretStore.setSecret(secretKey, {
97+
await secretStore.setSecret(testSecretKey, {
9898
botAccessToken: process.env.TEST_SLACK_BOT_TOKEN!,
9999
});
100100

101101
const secretReference = await prisma.secretReference.create({
102102
data: {
103-
key: secretKey,
103+
key: testSecretKey,
104104
provider: "DATABASE",
105105
},
106106
});
107+
testSecretReferenceId = secretReference.id;
107108

108109
// Create Slack organization integration
109110
const integration = await prisma.organizationIntegration.create({
@@ -136,21 +137,31 @@ describe.skipIf(!hasSlackCredentials)("Slack Error Alert Visual Tests", () => {
136137
});
137138

138139
afterAll(async () => {
139-
// Clean up test data
140140
if (testChannelId) {
141141
await prisma.projectAlertChannel.deleteMany({
142142
where: { id: testChannelId },
143143
});
144144
}
145+
if (testOrganizationId) {
146+
await prisma.organizationIntegration.deleteMany({
147+
where: { organizationId: testOrganizationId },
148+
});
149+
}
150+
if (testSecretReferenceId) {
151+
await prisma.secretReference.deleteMany({
152+
where: { id: testSecretReferenceId },
153+
});
154+
}
155+
if (testSecretKey) {
156+
const secretStore = getSecretStore("DATABASE");
157+
await secretStore.deleteSecret(testSecretKey);
158+
}
145159
if (testProjectId) {
146160
await prisma.project.deleteMany({
147161
where: { id: testProjectId },
148162
});
149163
}
150164
if (testOrganizationId) {
151-
await prisma.organizationIntegration.deleteMany({
152-
where: { organizationId: testOrganizationId },
153-
});
154165
await prisma.organization.deleteMany({
155166
where: { id: testOrganizationId },
156167
});

0 commit comments

Comments
 (0)