Skip to content
Open
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
12 changes: 6 additions & 6 deletions packages/api/src/routers/api/__tests__/webhooks.int.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { Types } from 'mongoose';
import { getLoggedInAgent, getServer } from '@/fixtures';
import Alert from '@/models/alert';
import Webhook, { WebhookService } from '@/models/webhook';
import * as template from '@/tasks/checkAlerts/template';
import * as transports from '@/tasks/checkAlerts/transports';

const MOCK_WEBHOOK = {
name: 'Test Webhook',
Expand Down Expand Up @@ -1221,10 +1221,10 @@ describe('webhooks router', () => {

beforeEach(() => {
genericSpy = jest
.spyOn(template, 'handleSendGenericWebhook')
.spyOn(transports, 'handleSendGenericWebhook')
.mockResolvedValue(undefined);
slackSpy = jest
.spyOn(template, 'handleSendSlackWebhook')
.spyOn(transports, 'handleSendSlackWebhook')
.mockResolvedValue(undefined);
});

Expand Down Expand Up @@ -1258,7 +1258,7 @@ describe('webhooks router', () => {

// The outbound call should receive the real URL and headers
expect(genericSpy).toHaveBeenCalledTimes(1);
const sentWebhook = genericSpy.mock.calls[0][0];
const sentWebhook = genericSpy.mock.calls[0][0].channel;
expect(sentWebhook.url).toBe(realUrl);
expect(sentWebhook.headers.toJSON()).toEqual({
Authorization: 'Bearer real-secret',
Expand Down Expand Up @@ -1289,7 +1289,7 @@ describe('webhooks router', () => {

// The outbound call should receive the attacker URL and literal ****
// (NOT the stored real secret)
const sentWebhook = genericSpy.mock.calls[0][0];
const sentWebhook = genericSpy.mock.calls[0][0].channel;
expect(sentWebhook.url).toBe('https://attacker.example.com/capture');
expect(sentWebhook.headers.toJSON()).toEqual({
Authorization: '****',
Expand Down Expand Up @@ -1383,7 +1383,7 @@ describe('webhooks router', () => {
.expect(200);

expect(genericSpy).toHaveBeenCalledTimes(1);
const sentWebhook = genericSpy.mock.calls[0][0];
const sentWebhook = genericSpy.mock.calls[0][0].channel;
expect(sentWebhook.url).toBe('https://example.com/webhook');
});

Expand Down
8 changes: 5 additions & 3 deletions packages/api/src/routers/api/webhooks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import Webhook, { WebhookService } from '@/models/webhook';
import {
handleSendGenericWebhook,
handleSendSlackWebhook,
} from '@/tasks/checkAlerts/template';
} from '@/tasks/checkAlerts/transports';
import { isDuplicateKeyError } from '@/utils/errors';
import {
validateWebhookUrl,
Expand Down Expand Up @@ -470,13 +470,15 @@ router.post(
eventId: 'test-event-id',
};

const testChannel = { type: 'webhook' as const, channel: testWebhook };

if (service === WebhookService.Slack) {
await handleSendSlackWebhook(testWebhook, testMessage);
await handleSendSlackWebhook(testChannel, testMessage);
} else if (
service === WebhookService.Generic ||
service === WebhookService.IncidentIO
) {
await handleSendGenericWebhook(testWebhook, testMessage);
await handleSendGenericWebhook(testChannel, testMessage);
} else {
return res.status(400).json({
message: 'Unsupported webhook service type',
Expand Down
31 changes: 31 additions & 0 deletions packages/api/src/tasks/checkAlerts/__tests__/notifications.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { InlineNotificationDispatcher } from '@/tasks/checkAlerts/notifications';

// The dispatcher forwards this opaque, so its shape doesn't matter for these
// tests — only that dispatch()/shutdown() behave per the documented contract.
const fakeJob: any = {};

describe('InlineNotificationDispatcher', () => {
it('resolves after the deliver fn resolves', async () => {
const order: string[] = [];
const dispatcher = new InlineNotificationDispatcher(async () => {
order.push('delivered');
});
await dispatcher.dispatch(fakeJob);
order.push('dispatch-returned');
expect(order).toEqual(['delivered', 'dispatch-returned']);
});

it('propagates a delivery rejection to the caller', async () => {
const dispatcher = new InlineNotificationDispatcher(async () => {
throw new Error('webhook exploded');
});
await expect(dispatcher.dispatch(fakeJob)).rejects.toThrow(
'webhook exploded',
);
});

it('shutdown resolves immediately — nothing is buffered', async () => {
const dispatcher = new InlineNotificationDispatcher(async () => {});
await expect(dispatcher.shutdown(1000)).resolves.toBeUndefined();
});
});
12 changes: 12 additions & 0 deletions packages/api/src/tasks/checkAlerts/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,18 @@ export class WebhookRedirectError extends Error {
}
}

// Carries the destination's HTTP status alongside its response body so
// callers can log/classify without an `as any` assertion on a plain Error.
export class WebhookResponseError extends Error {
readonly status: number;

constructor(message: string, status: number) {
super(message);
this.name = 'WebhookResponseError';
this.status = status;
}
}

// @clickhouse/client (Node) rejects with these exact messages when the
// configured request_timeout elapses or the request is aborted. See
// clickhouse-js packages/client-node/src/connection/socket_pool.ts.
Expand Down
2 changes: 1 addition & 1 deletion packages/api/src/tasks/checkAlerts/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@ import {
import {
AlertMessageTemplateDefaultView,
buildAlertMessageTemplateTitle,
handleSendGenericWebhook,
renderAlertTemplate,
} from '@/tasks/checkAlerts/template';
import { handleSendGenericWebhook } from '@/tasks/checkAlerts/transports';
import { tasksTracer } from '@/tasks/tracer';
import { CheckAlertsTaskArgs, HdxTask } from '@/tasks/types';
import {
Expand Down
70 changes: 70 additions & 0 deletions packages/api/src/tasks/checkAlerts/notifications.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import type {
Message,
PopulatedAlertChannel,
} from '@/tasks/checkAlerts/transports';
import { deliverToChannel } from '@/tasks/checkAlerts/transports';

/**
* Alert notification dispatch seam.
*
* Evaluation produces fully rendered NotificationJobs; a NotificationDispatcher
* owns delivery. `eventId` is the idempotency key, so a queueing dispatcher can
* deduplicate without re-deriving it.
*/
export type NotificationJob = {
/** Idempotency key: objectHash(alertId, channel, group). */
eventId: string;
alertId?: string;
teamId?: string;
group?: string;
/**
* Named `populatedChannel`, not `channel`, deliberately: a downstream build
* carries both a serializable channel *reference* and the resolved document,
* and reusing `channel` for the resolved one collides with that.
*/
populatedChannel: PopulatedAlertChannel;
message: Message;
};

export type NotificationDeliverFn = (job: NotificationJob) => Promise<void>;

/**
* dispatch() contract: the inline implementation resolves after delivery, so
* errors propagate to the caller. Queueing implementations resolve after
* enqueue; their errors surface in their own logs and metrics.
*/
export interface NotificationDispatcher {
dispatch(job: NotificationJob): Promise<void>;
/** Flush anything pending, giving up after deadlineMs. */
shutdown(deadlineMs: number): Promise<void>;
}

/**
* The default delivery implementation, wired to the transports registry. A
* downstream dispatcher composes this rather than calling `deliverToChannel`
* directly, so it stays swappable in one place.
*
* @public
*/
export const deliverNotification: NotificationDeliverFn = async job => {
await deliverToChannel(job.populatedChannel, job.message, {
group: job.group,
});
};

/** Delivers synchronously so errors flow into the caller's executionErrors. */
export class InlineNotificationDispatcher implements NotificationDispatcher {
constructor(private readonly deliver: NotificationDeliverFn) {}

async dispatch(job: NotificationJob): Promise<void> {
await this.deliver(job);
}

async shutdown(_deadlineMs: number): Promise<void> {
// Nothing buffered.
}
}

export const inlineNotificationDispatcher = new InlineNotificationDispatcher(
deliverNotification,
);
Loading
Loading