Skip to content

[TASK-178 / 25.06.27] Feature - 센트리 슬랙 웹훅 연동 #38

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Jul 2, 2025
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
311 changes: 311 additions & 0 deletions src/controllers/__test__/webhook.controller.test.ts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Invalid Body값에 대한 실패 케이스 테스트가 없는 것 같네요!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

반영했습니다!

Original file line number Diff line number Diff line change
@@ -0,0 +1,311 @@
import 'reflect-metadata';
import { Request, Response } from 'express';
import { WebhookController } from '@/controllers/webhook.controller';
import { sendSlackMessage } from '@/modules/slack/slack.notifier';

// Mock dependencies
jest.mock('@/modules/slack/slack.notifier');

// logger 모킹
jest.mock('@/configs/logger.config', () => ({
error: jest.fn(),
info: jest.fn(),
}));

describe('WebhookController', () => {
let webhookController: WebhookController;
let mockRequest: Partial<Request>;
let mockResponse: Partial<Response>;
let nextFunction: jest.Mock;
let mockSendSlackMessage: jest.MockedFunction<typeof sendSlackMessage>;

beforeEach(() => {
// WebhookController 인스턴스 생성
webhookController = new WebhookController();

// Request, Response, NextFunction 모킹
mockRequest = {
body: {},
headers: {},
};

mockResponse = {
json: jest.fn().mockReturnThis(),
status: jest.fn().mockReturnThis(),
};

nextFunction = jest.fn();
mockSendSlackMessage = sendSlackMessage as jest.MockedFunction<typeof sendSlackMessage>;
});

afterEach(() => {
jest.clearAllMocks();
});

describe('handleSentryWebhook', () => {
// 실제 동작에 필요한 필수 값만 사용하도록 타입 미적용
const mockSentryData = {
action: 'created',
data: {
issue: {
id: 'test-issue-123',
title: '테스트 오류입니다',
culprit: 'TestFile.js:10',
status: 'unresolved',
count: "5",
userCount: 3,
firstSeen: '2024-01-01T12:00:00.000Z',
permalink: 'https://velog-dashboardv2.sentry.io/issues/test-issue-123/',
project: {
id: 'project-123',
name: 'Velog Dashboard',
slug: 'velog-dashboard'
}
}
}
};

it('유효한 Sentry 웹훅 데이터로 처리에 성공해야 한다', async () => {
mockRequest.body = mockSentryData;
mockSendSlackMessage.mockResolvedValue();

await webhookController.handleSentryWebhook(
mockRequest as Request,
mockResponse as Response,
nextFunction
);

expect(mockSendSlackMessage).toHaveBeenCalledWith(
expect.stringContaining('🚨 *새로운 오류가 발생하였습니다*')
);
expect(mockSendSlackMessage).toHaveBeenCalledWith(
expect.stringContaining('🔴 *제목:* 테스트 오류입니다')
);
expect(mockSendSlackMessage).toHaveBeenCalledWith(
expect.stringContaining('📍 *위치:* TestFile.js:10')
);
expect(mockSendSlackMessage).toHaveBeenCalledWith(
expect.stringContaining('🔗 *상세 보기:* https://velog-dashboardv2.sentry.io/issues/test-issue-123/')
);
expect(mockResponse.status).toHaveBeenCalledWith(200);
expect(mockResponse.json).toHaveBeenCalledWith({
success: true,
message: 'Sentry 웹훅 처리에 성공하였습니다.',
data: {},
error: null
});
});

it('permalink가 없는 경우 기본 URL 패턴을 사용해야 한다', async () => {
const dataWithoutPermalink = {
...mockSentryData,
data: {
...mockSentryData.data,
issue: {
...mockSentryData.data.issue,
permalink: undefined
}
}
};
mockRequest.body = dataWithoutPermalink;
mockSendSlackMessage.mockResolvedValue();

await webhookController.handleSentryWebhook(
mockRequest as Request,
mockResponse as Response,
nextFunction
);

expect(mockSendSlackMessage).toHaveBeenCalledWith(
expect.stringContaining('🔗 *상세 보기:* https://velog-dashboardv2.sentry.io/issues/test-issue-123/')
);
});

it('Slack 메시지 전송 실패 시 에러를 전달해야 한다', async () => {
mockRequest.body = mockSentryData;
const slackError = new Error('Slack 전송 실패');
mockSendSlackMessage.mockRejectedValue(slackError);

await webhookController.handleSentryWebhook(
mockRequest as Request,
mockResponse as Response,
nextFunction
);

expect(nextFunction).toHaveBeenCalledWith(slackError);
expect(mockResponse.json).not.toHaveBeenCalled();
});

// Invalid Body 케이스 테스트들
it('action이 created가 아닌 경우 400 에러를 반환해야 한다', async () => {
mockRequest.body = { action: 'resolved' };

await webhookController.handleSentryWebhook(
mockRequest as Request,
mockResponse as Response,
nextFunction
);

expect(mockResponse.status).toHaveBeenCalledWith(400);
expect(mockResponse.json).toHaveBeenCalledWith({
success: true,
message: 'Sentry 웹훅 처리에 실패했습니다',
data: {},
error: null
});
expect(nextFunction).not.toHaveBeenCalled();
});

it('빈 body인 경우 400 에러를 반환해야 한다', async () => {
mockRequest.body = {};

await webhookController.handleSentryWebhook(
mockRequest as Request,
mockResponse as Response,
nextFunction
);

expect(mockResponse.status).toHaveBeenCalledWith(400);
expect(mockResponse.json).toHaveBeenCalledWith({
success: true,
message: 'Sentry 웹훅 처리에 실패했습니다',
data: {},
error: null
});
});

it('action이 없는 경우 400 에러를 반환해야 한다', async () => {
mockRequest.body = { data: { issue: {} } };

await webhookController.handleSentryWebhook(
mockRequest as Request,
mockResponse as Response,
nextFunction
);

expect(mockResponse.status).toHaveBeenCalledWith(400);
expect(mockResponse.json).toHaveBeenCalledWith({
success: true,
message: 'Sentry 웹훅 처리에 실패했습니다',
data: {},
error: null
});
});

it('전혀 다른 형태의 객체인 경우 400 에러를 반환해야 한다', async () => {
mockRequest.body = {
username: 'test',
password: '123456',
email: '[email protected]'
};

await webhookController.handleSentryWebhook(
mockRequest as Request,
mockResponse as Response,
nextFunction
);

expect(mockResponse.status).toHaveBeenCalledWith(400);
expect(mockResponse.json).toHaveBeenCalledWith({
success: true,
message: 'Sentry 웹훅 처리에 실패했습니다',
data: {},
error: null
});
});

it('action은 created이지만 필수 필드가 없는 경우 에러를 전달해야 한다', async () => {
mockRequest.body = {
action: 'created',
data: {
issue: {
// 필수 필드들이 누락됨
}
}
};

await webhookController.handleSentryWebhook(
mockRequest as Request,
mockResponse as Response,
nextFunction
);

expect(nextFunction).toHaveBeenCalledWith(
expect.objectContaining({
message: 'Sentry 웹훅 데이터가 올바르지 않습니다'
})
);
expect(mockResponse.json).not.toHaveBeenCalled();
});

it('action은 created이지만 data가 없는 경우 에러를 전달해야 한다', async () => {
mockRequest.body = { action: 'created' };

await webhookController.handleSentryWebhook(
mockRequest as Request,
mockResponse as Response,
nextFunction
);

expect(nextFunction).toHaveBeenCalled();
expect(mockResponse.json).not.toHaveBeenCalled();
});

it('잘못된 타입의 body인 경우 400 에러를 반환해야 한다', async () => {
mockRequest.body = 'invalid string body';

await webhookController.handleSentryWebhook(
mockRequest as Request,
mockResponse as Response,
nextFunction
);

expect(mockResponse.status).toHaveBeenCalledWith(400);
});
});

describe('formatSentryMessage (private method integration test)', () => {
it('완전한 Sentry 데이터로 올바른 형식의 메시지를 생성해야 한다', async () => {
// 실제 동작에 필요한 필수 값만 사용하도록 타입 미적용
const completeData = {
action: 'created',
data: {
issue: {
id: 'issue-456',
title: 'TypeError: Cannot read property of undefined',
culprit: 'components/UserProfile.tsx:25',
status: 'unresolved',
count: "12",
userCount: 8,
firstSeen: '2024-01-15T14:30:00.000Z',
permalink: 'https://velog-dashboardv2.sentry.io/issues/issue-456/',
project: {
id: 'proj-789',
name: 'Velog Dashboard V2',
slug: 'velog-dashboard-v2'
}
}
}
};

mockRequest.body = completeData;
mockSendSlackMessage.mockResolvedValue();

await webhookController.handleSentryWebhook(
mockRequest as Request,
mockResponse as Response,
nextFunction
);

const expectedMessage = `🚨 *새로운 오류가 발생하였습니다*

🔴 *제목:* TypeError: Cannot read property of undefined

📍 *위치:* components/UserProfile.tsx:25

🔗 *상세 보기:* https://velog-dashboardv2.sentry.io/issues/issue-456/`;

expect(mockSendSlackMessage).toHaveBeenCalledWith(expectedMessage);
});
});
});
57 changes: 57 additions & 0 deletions src/controllers/webhook.controller.ts
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

벨리데이션 미들웨어가 없어도 괜찮을 것 같은데 다만, 이를 아무나 호출할 수 있게 해도 괜찮을까 싶은 강력한 의문점이 있네요.
엔드포인트만 알아내면 무차별 공격이 가능한 상태라서요, 더욱이 컨트롤러 단에서 모든 로직을 처리하니까요 :)

어떻게 막을 수 있는 방법이 없을까요? 센트리에서 커스텀 헤더를 강제 추가해서 해당 헤더가 있는 경우만 OK 한다던지
또는 rate-limit 를 걸던지 등의 방식이요

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저도 이 부분은 고민해보고 있는데.. Hook 쪽에서 강제로 어떤 헤더를 전송하게 하는 방법은 찾아보기가 어렵네요;;
그나마 rate-limit이 제일 현실적인 것 같은데, 이 부분을 서버에서 처리해줘야 하는건지, 아니면 로드밸런서 단에서 처리해줘야 하는건지 애매합니다
(적어도 현재 서버에 express-load-limitter같은 제한 관련 라이브러리가 설치되어있는 것 같지는 않네요)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LB 에서 처리하면 모든 라우팅에 제한이 됩니다, 하물며 저희 배치쪽에서도 쓰로풋이 제한이 되어버리죠. 더욱이 추후 서버 추가하는 순간 LB 에서 완벽한 보틀넥 지옥이라서요. L4 면 모를까, L7 에서는 주로 하지 못하는 방법이긴 해요.

공식홈페이지를 찾아보니까 "Sentry webhooks support various functionalities and are made up of four integral HTTP headers described below" - https://docs.sentry.io/organization/integrations/integration-platform/webhooks/

라고 이미 Sentry-Hook-Signature 헤더 값을 준다고 하네요 ㅎㅎ 기준님ㅋㅋ 찾은거.. 믿어도 괜찮은거죠..? 이걸 활용해 보는 것은 어떨까요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sentry-Hook-Signature값이 계속 바뀌는 값이다 보니 놓친 것 같은데, 공식 문서를 제대로 읽어보지 못한 것 같네요;;
지적 감사합니다! 한 번 적용해볼게요

Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { NextFunction, Request, RequestHandler, Response } from 'express';
import { EmptyResponseDto, SentryWebhookData } from '@/types';
import logger from '@/configs/logger.config';
import { sendSlackMessage } from '@/modules/slack/slack.notifier';

export class WebhookController {
private readonly STATUS_EMOJI = {
'unresolved': '🔴',
'resolved': '✅',
'ignored': '🔇',
} as const;

handleSentryWebhook: RequestHandler = async (
req: Request,
res: Response,
next: NextFunction,
): Promise<void> => {
try {

if (req.body?.action !== "created") {
const response = new EmptyResponseDto(true, 'Sentry 웹훅 처리에 실패했습니다', {}, null);
res.status(400).json(response);
return;
}

const sentryData: SentryWebhookData = req.body;

const slackMessage = this.formatSentryMessage(sentryData);
await sendSlackMessage(slackMessage);

const response = new EmptyResponseDto(true, 'Sentry 웹훅 처리에 성공하였습니다.', {}, null);
res.status(200).json(response);
} catch (error) {
logger.error('Sentry 웹훅 처리 실패:', error);
next(error);
}
};

private formatSentryMessage(sentryData: SentryWebhookData): string {
const { data: { issue } } = sentryData;

if(!issue.status || !issue.title || !issue.culprit || !issue.id) throw new Error('Sentry 웹훅 데이터가 올바르지 않습니다');
Copy link
Contributor

@ooheunda ooheunda Jul 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

요건 위 19번째줄이랑 같은 400 예외처리 같은데, 통합하면 좋을 것 같긴 합니다~!
그런데 지금 코드도 괜찮아요! 그리고 사실 통합한다면 완전히 DTO 검증의 느낌이라서요, 또 DTO 형태로 빼고 싶어질 것 같아요ㅋㅋ
제가 생각하는 베스트는 그런데, 계속 말씀드렸다시피 슬랙 웹훅에서 굳이? 싶긴 해요. 기준님 편하신대로 하시면 될 것 같아요.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 개인적으로 통합하는것보다 뺴는게 맞다고 생각하긴 합니다!
함수별로 어느 정도 역할의 분리가 필요하다고 생각하기도 했고, 결정적으로 코드가 너무 길어질 것 같거든요;;


const { status, title: issueTitle, culprit, permalink, id } = issue;
const statusEmoji = this.STATUS_EMOJI[status as keyof typeof this.STATUS_EMOJI];

// URL 생성 - permalink가 있으면 사용, 없으면 실제 프로젝트 URL 패턴으로 생성
const detailUrl = permalink || `https://velog-dashboardv2.sentry.io/issues/${id}/`;

let message = `🚨 *새로운 오류가 발생하였습니다*\n\n`;
message += `${statusEmoji} *제목:* ${issueTitle}\n\n`;
message += `📍 *위치:* ${culprit}\n\n`;
message += `🔗 *상세 보기:* ${detailUrl}`;

return message;
}
}
2 changes: 2 additions & 0 deletions src/routes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import PostRouter from './post.router';
import NotiRouter from './noti.router';
import LeaderboardRouter from './leaderboard.router';
import TotalStatsRouter from './totalStats.router';
import WebhookRouter from './webhook.router';

const router: Router = express.Router();

Expand All @@ -16,5 +17,6 @@ router.use('/', PostRouter);
router.use('/', NotiRouter);
router.use('/', LeaderboardRouter);
router.use('/', TotalStatsRouter);
router.use('/', WebhookRouter);

export default router;
Loading
Loading