Refactor: API 에러 처리 공통 핸들러 도입 및 onError 정책 통일 - #202
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAPI 에러 핸들링을 통일하기 위해 공유 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
🎨 Storybook 배포 완료📚 Storybook: https://6960ec095e9394ddeaa0f9f3-lbqaapbpai.chromatic.com/
|
|
🎉 구현한 기능 Preview: https://nitrogen-front-ptknz8o9v-ssilver01s-projects.vercel.app |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/web/src/shared/api/handleApiError.ts`:
- Around line 39-42: The current logic uses error.message when
preferServerMessage is true, which can expose ky's generic messages; update
handleApiError to use a distinct server-specific property (e.g.,
error.serverMessage) instead of error.message when preferServerMessage is
requested, and only show that value if it exists; also modify client.ts's
beforeError handler to parse the JSON body and set error.serverMessage =
body.message (or undefined) whenever a server-provided message exists so you can
reliably distinguish server messages from ky's default messages; finally change
the toast call to prefer error.serverMessage (if preferServerMessage) else
fallback.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 981f7883-85ef-403a-bac9-6cd6140de972
📒 Files selected for processing (11)
apps/web/src/features/auth/model/useWithdraw.tsapps/web/src/features/expense/ui/ExpenseEditBottomSheet.tsxapps/web/src/features/expense/ui/steps/AddCategoryStep/AddCategoryStep.tsxapps/web/src/features/inquiry/model/useInquiryForm.tsapps/web/src/features/review/model/useSpendingReview.tsapps/web/src/shared/api/client.tsapps/web/src/shared/api/handleApiError.tsapps/web/src/shared/api/index.tsapps/web/src/widgets/addCategory/ui/AddCategory.tsxapps/web/src/widgets/expenseRecordFunnel/ui/ExpenseRecordFunnel.tsxapps/web/src/widgets/notifications/ui/Notifications.tsx
| const serverMessage = | ||
| preferServerMessage && error instanceof HTTPError && error.message ? error.message : null; | ||
|
|
||
| toast.attention(serverMessage ?? fallback); |
There was a problem hiding this comment.
preferServerMessage 사용 시 ky의 기본 에러 메시지가 노출될 위험
client.ts의 beforeError는 응답 body를 JSON 파싱해 body.message가 있을 때만 error.message를 덮어씁니다. 따라서 응답이 JSON이 아니거나 message 필드가 없는 경우 error.message는 ky의 기본 문자열(예: Request failed with status code 500)이 그대로 남습니다. 현재 로직은 error.message가 truthy이기만 하면 이를 토스트에 노출하므로, preferServerMessage: true인 useWithdraw 같은 호출에서 사용자에게 영문 ky 메시지가 그대로 보일 수 있습니다.
서버가 실제로 메시지를 내려줬을 때만 이를 우선하도록, 서버 메시지를 별도 필드에 저장해 구분하는 것을 권장합니다.
♻️ 제안: 서버 메시지를 별도 필드로 보관
// apps/web/src/shared/api/client.ts (beforeError)
if (error.response) {
try {
const body = (await error.response.clone().json()) as { message?: string };
if (body?.message) {
error.message = body.message;
+ (error as HTTPError & { serverMessage?: string }).serverMessage = body.message;
}
} catch {
// JSON 파싱 실패 시 원본 message 유지
}
}// handleApiError.ts
const serverMessage =
- preferServerMessage && error instanceof HTTPError && error.message ? error.message : null;
+ preferServerMessage && error instanceof HTTPError
+ ? ((error as HTTPError & { serverMessage?: string }).serverMessage ?? null)
+ : null;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const serverMessage = | |
| preferServerMessage && error instanceof HTTPError && error.message ? error.message : null; | |
| toast.attention(serverMessage ?? fallback); | |
| const serverMessage = | |
| preferServerMessage && error instanceof HTTPError | |
| ? ((error as HTTPError & { serverMessage?: string }).serverMessage ?? null) | |
| : null; | |
| toast.attention(serverMessage ?? fallback); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/web/src/shared/api/handleApiError.ts` around lines 39 - 42, The current
logic uses error.message when preferServerMessage is true, which can expose ky's
generic messages; update handleApiError to use a distinct server-specific
property (e.g., error.serverMessage) instead of error.message when
preferServerMessage is requested, and only show that value if it exists; also
modify client.ts's beforeError handler to parse the JSON body and set
error.serverMessage = body.message (or undefined) whenever a server-provided
message exists so you can reliably distinguish server messages from ky's default
messages; finally change the toast call to prefer error.serverMessage (if
preferServerMessage) else fallback.
ssilver01
left a comment
ssilver01
left a comment
There was a problem hiding this comment.
공통 핸들러 도입 감사합니다 ! ggoodd
2 participants
📝 PR 유형
🔔 관련된 이슈 넘버
close #112
✅ 작업 목록
1. API 에러 처리 공통화
handleApiError유틸 함수 추가 (shared/api/handleApiError.ts)에러 처리 정책
2. client.ts beforeError 개선
body.message만 추출3. React Query 에러 처리 구조 정리
useMutation선언부에onError고정onError제거 (ExpenseRecordFunnel)4. 기존 코드 마이그레이션
preferServerMessage: true)5. 누락된 onError 보강
6. 토스트 문구 톤 통일
🍰 논의사항
현재 API mutation 에러 처리는
handleApiError공통 핸들러로 정리했지만, 이후 새로운 mutation을 추가할 때onError를 누락하면 동일한 문제가 다시 발생할 수 있을 것 같습니다.이번 PR에서도 기존 mutation 중
onError가 누락된 케이스가 발견되어 보강했기 때문에, 장기적으로는useMutation사용 시onError누락을 ESLint 룰로 감지하도록 추가할지 논의해보고 싶습니다.필수 적용까지는 아니더라도, 최소한 경고 수준으로 먼저 도입하면 API 실패 시 사용자 피드백이 빠지는 문제를 예방할 수 있을 것 같습니다.
📷 ETC
사이드 이펙트 점검
Summary by CodeRabbit
릴리스 노트