Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
WalkthroughSentry 오류 처리를 공통 유틸리티로 통합하고, API 경로·기능·상태·네트워크 상태를 기반으로 이벤트 메타데이터와 심각도를 산출하도록 변경했습니다. Axios, APIError, ErrorBoundary의 중복 캡처 방지 흐름도 공통 플래그 헬퍼를 사용하도록 조정했습니다. ChangesSentry 오류 맥락 처리
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant AxiosRequest
participant SentryUtilities
participant Sentry
participant ErrorBoundary
AxiosRequest->>SentryUtilities: AxiosError 분석
SentryUtilities-->>AxiosRequest: 스킵 여부·메타데이터·Sentry Error
AxiosRequest->>Sentry: API 오류 캡처 및 플래그 표시
AxiosRequest->>ErrorBoundary: APIError 전달
ErrorBoundary->>Sentry: 미캡처 렌더링 오류 전송
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🚀 Preview 배포 완료!
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
src/util/sentry.ts (2)
17-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win상수 네이밍 컨벤션 위반.
criticalFeatures는 새로 추가된 모듈 레벨 상수 배열인데 camelCase로 작성되어 있습니다. As per coding guidelines,**/*.{ts,tsx}파일은 "UPPER_SNAKE_CASE for constants"를 요구합니다.♻️ 제안
-const criticalFeatures: DebateFeature[] = ['timer', 'vote', 'live-share']; +const CRITICAL_FEATURES: DebateFeature[] = ['timer', 'vote', 'live-share'];(사용처인
resolveApiErrorLevel의 참조도 함께 변경 필요)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/util/sentry.ts` at line 17, Rename the module-level constant criticalFeatures to the required UPPER_SNAKE_CASE convention, and update its reference in resolveApiErrorLevel accordingly while preserving the existing array contents and behavior.Source: Coding guidelines
147-157: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
sentryError에 설정한__sentry_captured__플래그는 사용되지 않는 죽은 코드입니다.154번 줄에서 새로 생성한
sentryError에 플래그를 설정하지만, 이 객체는Sentry.captureException에만 전달되고 버려집니다. 실제 중복 캡처 방지는 axiosInstance.ts에서 원본error에 대해 별도로 호출하는markSentryCaptured(error)가 담당합니다. 이 줄은 오해를 유발할 수 있으니 제거를 고려해 주세요.♻️ 제안
const sentryError = new Error(error.message); sentryError.name = `[${metadata.statusLabel}] ${metadata.method} ${metadata.endpoint}`; sentryError.stack = error.stack; - (sentryError as SentryCapturedError).__sentry_captured__ = true; return sentryError;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/util/sentry.ts` around lines 147 - 157, Remove the __sentry_captured__ assignment from createSentryApiError; duplicate-capture prevention is handled by markSentryCaptured(error) on the original AxiosError, so leave the constructed sentryError without this unused flag.src/util/sentry.test.ts (1)
2-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
markSentryCaptured/isSentryCaptured에 대한 직접 테스트 추가를 권장합니다.이 두 함수는 axiosInstance, primitives, ErrorBoundary 세 곳이 공유하는 중복 캡처 방지 계약의 핵심입니다. 계약이 깨지면 조용히 중복 알림 또는 알림 누락이 발생할 수 있으므로, import 목록에 추가해 간단한 단위 테스트를 붙이면 좋겠습니다.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/util/sentry.test.ts` around lines 2 - 9, sentry 유틸리티의 markSentryCaptured와 isSentryCaptured를 직접 import하고, 동일한 오류 객체에 캡처 표시를 설정한 뒤 isSentryCaptured가 표시 전후에 올바른 값을 반환하는 단위 테스트를 추가하세요. 서로 다른 오류 객체는 독립적으로 처리되는지와 기존 중복 캡처 방지 계약을 검증하세요.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/apis/axiosInstance.ts`:
- Around line 71-83: Prevent raw user-controlled data from reaching Sentry by
introducing a shared masking or allowlist utility. In
src/apis/axiosInstance.ts#L71-L83, apply it to request params and response data
before setContext; in src/components/ErrorBoundary/ErrorBoundary.tsx#L49-L53,
sanitize window.location.search by masking or removing sensitive parameters such
as token, code, and state before setting the render context.
In `@src/util/sentry.ts`:
- Around line 48-96: Update resolveFeatureFromPathname so every startsWith route
check enforces a path-segment boundary, matching removeLanguagePrefix behavior:
accept the route only when the pathname ends at that prefix or the next
character is /. Apply this to /composition, /overview, /vote, /table/customize,
/oauth, /share, and /live while preserving the existing route order and feature
mappings.
---
Nitpick comments:
In `@src/util/sentry.test.ts`:
- Around line 2-9: sentry 유틸리티의 markSentryCaptured와 isSentryCaptured를 직접
import하고, 동일한 오류 객체에 캡처 표시를 설정한 뒤 isSentryCaptured가 표시 전후에 올바른 값을 반환하는 단위 테스트를
추가하세요. 서로 다른 오류 객체는 독립적으로 처리되는지와 기존 중복 캡처 방지 계약을 검증하세요.
In `@src/util/sentry.ts`:
- Line 17: Rename the module-level constant criticalFeatures to the required
UPPER_SNAKE_CASE convention, and update its reference in resolveApiErrorLevel
accordingly while preserving the existing array contents and behavior.
- Around line 147-157: Remove the __sentry_captured__ assignment from
createSentryApiError; duplicate-capture prevention is handled by
markSentryCaptured(error) on the original AxiosError, so leave the constructed
sentryError without this unused flag.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 90ced027-62ba-45c6-b28c-234f5bfa68e1
📒 Files selected for processing (5)
src/apis/axiosInstance.tssrc/apis/primitives.tssrc/components/ErrorBoundary/ErrorBoundary.tsxsrc/util/sentry.test.tssrc/util/sentry.ts
| scope.setContext('request', { | ||
| pathname: metadata.pathname, | ||
| search: window.location.search, | ||
| url: config?.url, | ||
| method: config?.method, | ||
| params: config?.params, | ||
| timeout: config?.timeout ?? requestTimeoutMs, | ||
| errorCode: code, | ||
| }, | ||
| fingerprint: [ | ||
| url: error.config?.url, | ||
| method: error.config?.method, | ||
| params: error.config?.params, | ||
| timeout: error.config?.timeout ?? requestTimeoutMs, | ||
| errorCode: error.code, | ||
| }); | ||
| scope.setContext('response', { | ||
| status: metadata.status, | ||
| data: error.response?.data, | ||
| }); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Sentry 컨텍스트에 원본 요청/쿼리 데이터를 가공 없이 전송하고 있어 개인정보 유출 위험이 있습니다.
두 위치 모두 사용자 입력이 포함될 수 있는 원본 데이터(쿼리스트링, 요청 파라미터, 응답 바디)를 그대로 Sentry(제3자 서비스)로 전송합니다. 여기에는 이메일, 초대 코드, OAuth code/state, 서버 에러 메시지에 담긴 사용자 식별정보 등 민감정보가 포함될 수 있습니다. 두 사이트는 동일한 근본 원인(원본 데이터를 그대로 전달)을 공유하므로 공통 마스킹/allowlist 유틸을 만들어 적용하는 것을 권장합니다.
src/apis/axiosInstance.ts#L71-L83:setContext('request', ...)의params와setContext('response', { data: error.response?.data })의data를 민감 필드 마스킹 또는 allowlist 기반 축약 후 전송하도록 수정.src/components/ErrorBoundary/ErrorBoundary.tsx#L49-L53:setContext('render', ...)의search: window.location.search를 알려진 민감 쿼리 파라미터(토큰, code, state 등)를 제거/마스킹한 값으로 대체.
📍 Affects 2 files
src/apis/axiosInstance.ts#L71-L83(this comment)src/components/ErrorBoundary/ErrorBoundary.tsx#L49-L53
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/apis/axiosInstance.ts` around lines 71 - 83, Prevent raw user-controlled
data from reaching Sentry by introducing a shared masking or allowlist utility.
In src/apis/axiosInstance.ts#L71-L83, apply it to request params and response
data before setContext; in
src/components/ErrorBoundary/ErrorBoundary.tsx#L49-L53, sanitize
window.location.search by masking or removing sensitive parameters such as
token, code, and state before setting the render context.
| export function resolveFeatureFromPathname(pathname: string): DebateFeature { | ||
| const path = removeLanguagePrefix(pathname); | ||
|
|
||
| if (path === '/home') { | ||
| return 'landing'; | ||
| } | ||
|
|
||
| if (path === '/') { | ||
| return 'table-list'; | ||
| } | ||
|
|
||
| if (path.startsWith('/composition')) { | ||
| return 'table-composition'; | ||
| } | ||
|
|
||
| if (path.startsWith('/overview')) { | ||
| return 'table-overview'; | ||
| } | ||
|
|
||
| if (path.includes('/end/vote') || path.startsWith('/vote')) { | ||
| return 'vote'; | ||
| } | ||
|
|
||
| if (path.includes('/end/feedback')) { | ||
| return 'timer'; | ||
| } | ||
|
|
||
| if (path.includes('/end')) { | ||
| return 'debate-end'; | ||
| } | ||
|
|
||
| if (path.startsWith('/table/customize')) { | ||
| return 'timer'; | ||
| } | ||
|
|
||
| if (path.startsWith('/oauth')) { | ||
| return 'auth'; | ||
| } | ||
|
|
||
| if (path.startsWith('/share')) { | ||
| return 'share'; | ||
| } | ||
|
|
||
| if (path.startsWith('/live')) { | ||
| return 'live-share'; | ||
| } | ||
|
|
||
| return 'unknown'; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
startsWith 접두사 매칭에 경로 구분자 경계 검사가 빠져 있습니다.
/vote, /live, /share, /oauth, /composition, /overview, /table/customize 등의 검사가 모두 단순 startsWith로만 이뤄져, 예를 들어 /livechat, /sharepoint, /overviewer처럼 우연히 같은 접두사로 시작하는 무관한 경로도 잘못된 feature로 분류될 수 있습니다. 같은 파일의 removeLanguagePrefix는 (?=\/|$) lookahead로 세그먼트 경계를 정확히 검사하는데, 이 함수에는 동일한 안전장치가 없습니다. 이 유틸의 목적이 알림 우선순위 판단(예: vote/live-share는 fatal로 승격)이기 때문에, 오분류는 실제 대응 우선순위 판단을 왜곡할 수 있습니다.
🐛 제안 수정
+function matchesSegment(path: string, prefix: string) {
+ return path === prefix || path.startsWith(`${prefix}/`);
+}
+
export function resolveFeatureFromPathname(pathname: string): DebateFeature {
...
- if (path.startsWith('/composition')) {
+ if (matchesSegment(path, '/composition')) {
return 'table-composition';
}
... (다른 startsWith 검사에도 동일하게 적용)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/util/sentry.ts` around lines 48 - 96, Update resolveFeatureFromPathname
so every startsWith route check enforces a path-segment boundary, matching
removeLanguagePrefix behavior: accept the route only when the pathname ends at
that prefix or the next character is /. Apply this to /composition, /overview,
/vote, /table/customize, /oauth, /share, and /live while preserving the existing
route order and feature mappings.
i-meant-to-be
left a comment
i-meant-to-be
left a comment
There was a problem hiding this comment.
저도 매번 센트리 오류 오는 거 잘 안 봤는데 이렇게 개선되면 확실히 보기 편하겠네요... 조은 PR 감사합니다!
딱히 문제 될 내용은 없어 보여 승인 남깁니다 깔끔한 작업 최고 👍👍👍
2 participants
🚩 연관 이슈
closed #468
📝 작업 내용
기존에는 이렇게 알림이 와서 알림만 보고 어떤 에러가 어떻게 발생한다는 것을 빠르게 파악하기 힘들었습니다.
이번 작업은 Sentry 알림을 단순히 “에러가 발생했다”는 신호로 두지 않고, 알림을 받았을 때 어떤 사용자 흐름에서 문제가 생겼고 지금 확인해야 하는지 판단할 수 있도록 이벤트 맥락을 정리한 작업입니다.
기존 Sentry 설정은 에러 수집, Replay, Source Map을 통해 디버깅 가능한 상태를 만드는 데에는 효과적이었습니다. 다만 실제로 알림을 받아보면, 알림만 보고 어떤 기능에서 발생한 문제인지, 사용자 핵심 흐름에 영향을 주는지, 바로 대응해야 하는지 판단하기 어려웠습니다.
그래서 이번 PR에서는 알림 수신자의 다음 행동을 돕는 방향으로 Sentry 이벤트의 제목, 태그, level, grouping 기준을 개선했습니다.
1. Sentry 알림 판단 기준 유틸 분리
src/util/sentry.ts로 분리했습니다.axiosInstance내부에 섞이지 않도록 분리해서, 이후 라우트나 핵심 기능 기준이 바뀌어도 수정 범위를 좁힐 수 있게 했습니다.이번 작업은 “어떤 에러를 어떤 맥락의 알림으로 볼 것인가”를 테스트 가능한 기준으로 만들기 위한 목적입니다.
2. 알림에서 사용자 영향 흐름이 보이도록 feature tag 추가
timer,vote,live-share,debate-end등 사용자 흐름 단위의 feature를 분류했습니다.기존에는 endpoint나 status는 확인할 수 있었지만, 이 에러가 실제로 어떤 사용자 행동에 영향을 주는지 알림만 보고 판단하기 어려웠습니다. 이번 작업에서는 알림을 여는 순간 “타이머 진행 중 문제인지”, “투표 흐름 문제인지”, “실시간 공유 문제인지”를 먼저 볼 수 있게 만드는 데 집중했습니다.
3. API 에러 이슈 제목과 grouping 기준 개선
[network-error] POST /api/live/:idapi-error + status + method + endpoint기준으로 정리했습니다.사용자별 id 값 때문에 같은 장애가 여러 이슈로 흩어지면 알림 피로도가 커지고 원인 파악도 늦어진다고 봤습니다. 그래서 “같은 문제는 같은 알림 이슈로 묶이게 하는 것”을 기준으로 정리했습니다.
4. 대응 가치 기준으로 error level과 필터링 정리
여기서 중요한 기준은 에러를 덜 보는 것이 아니라, 실제로 확인해야 할 알림의 신뢰도를 높이는 것이었습니다.
5. 렌더링 에러에도 feature 맥락 추가
ErrorBoundary에서 잡힌 렌더링 에러는 화면이 깨진 상태로 보고fatal로 분류했습니다.render-error + feature기준 fingerprint를 설정해 같은 기능의 렌더링 장애가 묶이도록 했습니다.✅ 검증
src/util/sentry.test.tstsc --build --noEmit🗣️ 리뷰 요구사항 (선택)
이번 PR에서는 “Sentry에 더 많이 남기기”보다 “알림을 받았을 때 바로 판단할 수 있게 만들기”에 초점을 뒀습니다.
feature 분류 기준이나 error level 기준이 실제 운영 관점에서 어색한 부분이 있다면 편하게 의견 주세요 ~ !!
Summary by CodeRabbit
개선 사항
테스트