-
Notifications
You must be signed in to change notification settings - Fork 7
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
feat: add error handling for NoticeBloc #471
feat: add error handling for NoticeBloc #471
Conversation
📝 Walkthrough📝 WalkthroughWalkthrough
Changes
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
lib/app/modules/notices/presentation/bloc/notice_bloc.dart (1)
25-25
: 예외 로그를 남기세요예외 발생 시 예외 정보를 로그에 남기면 문제 분석에 도움이 됩니다.
수정 제안:
catch (e) { // 예외를 로그에 기록 print(e); emit(NoticeState.error('알 수 없는 오류가 발생했습니다.')); }Also applies to: 39-39, 53-53, 63-63, 74-74, 84-84
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- lib/app/modules/notices/presentation/bloc/notice_bloc.dart (2 hunks)
🧰 Additional context used
🔇 Additional comments (6)
lib/app/modules/notices/presentation/bloc/notice_bloc.dart (6)
30-40
: 이전 코멘트와 동일한 사항 적용
_SendNotification
이벤트 핸들러에서도 동일하게 일반 예외 대신 구체적인 예외를 처리하고, 예외 메시지를 직접 노출하지 않도록 수정이 필요합니다.
44-54
: 이전 코멘트와 동일한 사항 적용
_Delete
이벤트 핸들러에서도 앞서 언급한 예외 처리 개선 사항을 적용해주세요.
58-64
: 이전 코멘트와 동일한 사항 적용
_AddReaction
이벤트 핸들러에서도 동일한 예외 처리 개선이 필요합니다.
68-75
: 이전 코멘트와 동일한 사항 적용
_RemoveReaction
이벤트 핸들러에서도 예외 처리 방식을 개선해주세요.
79-85
: 이전 코멘트와 동일한 사항 적용
_GetFull
이벤트 핸들러도 동일하게 수정이 필요합니다.
108-108
: 에러 상태 추가 확인
NoticeState
에error
상태를 추가하여 에러 처리를 강화한 부분이 확인되었습니다. 전체적인 상태 관리 로직과 일관성이 있으며, 에러 발생 시 UI에서 적절히 처리되는지 확인해주세요.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
에러 핸들링은 좋습니다. 다만 NoticeBloc을 사용하는 화면에서 BlocListener를 사용해서 에러 메시지를 유저에게 보여줄 수 있도록 해주세요.
NoticeListBloc에도 에러 핸들링이 되어있는지는 모르겠는데 동일하게 적용해주시면 됩니다
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
lib/app/modules/notices/presentation/layouts/notice_edit_shell_layout.dart (1)
27-34
: 오류 처리 구현이 적절하게 되었습니다.BlocListener를 통한 오류 상태 처리와 SnackBar를 통한 사용자 피드백이 잘 구현되었습니다. 다만 다음 사항들을 고려해보시면 좋을 것 같습니다:
- 오류 메시지가 사용자 친화적인지 확인
- SnackBar의 표시 시간 설정 검토
다음과 같이 SnackBar의 동작을 개선할 수 있습니다:
- SnackBar(content: Text(error.message)), + SnackBar( + content: Text(error.message), + duration: const Duration(seconds: 3), + behavior: SnackBarBehavior.floating, + ),lib/app/modules/notices/presentation/layouts/single_notice_shell_layout.dart (1)
30-36
: 사용자 경험 개선을 위한 제안사항이 있습니다.현재 구현에 대한 개선 제안:
- 로딩 상태에 진행 상황이나 메시지를 추가하면 좋을 것 같습니다.
- 에러 발생 시 재시도 기능을 추가하는 것을 고려해보세요.
다음과 같이 개선할 수 있습니다:
- return const Center(child: CircularProgressIndicator()); + return const Center( + child: Column( + mainAxisAlignment: MainAxisAlignment.center, + children: [ + CircularProgressIndicator(), + SizedBox(height: 16), + Text('공지사항을 불러오는 중입니다...'), + ], + ), + );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
lib/app/modules/notices/presentation/layouts/notice_edit_shell_layout.dart
(2 hunks)lib/app/modules/notices/presentation/layouts/single_notice_shell_layout.dart
(1 hunks)
🔇 Additional comments (2)
lib/app/modules/notices/presentation/layouts/notice_edit_shell_layout.dart (1)
2-2
: Material 패키지로의 전환이 확인되었습니다.
Material 디자인 가이드라인을 따르는 것으로 전환된 것이 확인되었습니다. 이는 일관된 UI/UX를 제공하는데 도움이 될 것입니다.
lib/app/modules/notices/presentation/layouts/single_notice_shell_layout.dart (1)
24-29
: 에러 처리 구현이 잘 되었습니다!
BlocListener를 사용한 에러 처리 구현이 적절합니다. SnackBar를 통해 사용자에게 에러 메시지를 표시하는 것은 좋은 UX 패턴입니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
lib/app/modules/notices/presentation/layouts/notice_edit_shell_layout.dart (1)
28-33
: 오류 처리 구현이 잘 되었습니다!BlocListener를 통한 오류 상태 감지 및 toast 메시지 표시 구현이 적절합니다. 다만, 사용자 경험을 더욱 향상시키기 위한 제안사항이 있습니다.
다음과 같이 오류 메시지에 대한 사용자 액션을 추가하는 것을 고려해보세요:
listener: (context, state) => state.mapOrNull( - error: (error) => context.showToast(error.message), + error: (error) => context.showToast( + error.message, + action: SnackBarAction( + label: '다시 시도', + onPressed: () => context.read<NoticeBloc>().add(const NoticeEvent.getFull()), + ), + ), ),lib/app/modules/notices/presentation/pages/notice_edit_preview_page.dart (1)
48-57
: 오류 처리 구현이 잘 되었습니다BlocListener를 사용한 오류 처리 구현이 깔끔하게 되었습니다:
- NoticeBloc의 상태 변화를 적절하게 감지
- mapOrNull을 사용한 타입 안전한 상태 처리
- 사용자 친화적인 토스트 메시지 표시
다만, 한 가지 제안사항이 있습니다:
오류 메시지에 대한 사용자 경험을 더욱 향상시키기 위해 다음과 같은 개선을 고려해보세요:
return BlocListener<NoticeBloc, NoticeState>( listener: (context, state) { state.mapOrNull( - error: (error) => context.showToast(error.message), + error: (error) => context.showToast( + error.message, + duration: const Duration(seconds: 5), + ), ); }, child: NoticeRenderer(lib/app/modules/notices/presentation/pages/write_additional_notice_page.dart (1)
80-85
: 에러 메시지 처리 개선 제안에러 상태에 대한 처리가 구현되었지만, 사용자에게 더 구체적인 에러 메시지를 제공하면 좋을 것 같습니다.
다음과 같이 에러 메시지를 더 구체적으로 처리하는 것을 고려해보세요:
state.mapOrNull( - error: (error) => context.showToast(error.message), + error: (error) => context.showToast( + error.message.isEmpty + ? context.t.common.unknownError + : error.message + ), );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
lib/app/modules/notices/presentation/layouts/notice_edit_shell_layout.dart
(2 hunks)lib/app/modules/notices/presentation/layouts/single_notice_shell_layout.dart
(2 hunks)lib/app/modules/notices/presentation/pages/notice_edit_preview_page.dart
(2 hunks)lib/app/modules/notices/presentation/pages/write_additional_notice_page.dart
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- lib/app/modules/notices/presentation/layouts/single_notice_shell_layout.dart
🧰 Additional context used
📓 Learnings (1)
lib/app/modules/notices/presentation/pages/write_additional_notice_page.dart (4)
Learnt from: 2paperstar
PR: gsainfoteam/ziggle-flutter#423
File: lib/app/modules/notices/presentation/pages/write_additional_notice_page.dart:38-40
Timestamp: 2024-11-12T04:55:44.974Z
Learning: In `WriteAdditionalNoticePage`, `context.read<NoticeBloc>().state.entity` is validated before the page is entered and is guaranteed to be non-null.
Learnt from: 2paperstar
PR: gsainfoteam/ziggle-flutter#423
File: lib/app/modules/notices/presentation/pages/write_additional_notice_page.dart:38-40
Timestamp: 2024-11-12T04:55:50.221Z
Learning: In `WriteAdditionalNoticePage`, `context.read<NoticeBloc>().state.entity` is validated before the page is entered and is guaranteed to be non-null.
Learnt from: 2paperstar
PR: gsainfoteam/ziggle-flutter#423
File: lib/app/modules/notices/presentation/pages/write_additional_notice_page.dart:42-44
Timestamp: 2024-11-12T04:55:44.974Z
Learning: In `WriteAdditionalNoticePage`, `context.read<NoticeBloc>().state.entity` is validated before entering the page, so force unwrapping with `!` is safe.
Learnt from: 2paperstar
PR: gsainfoteam/ziggle-flutter#423
File: lib/app/modules/notices/presentation/pages/write_additional_notice_page.dart:42-44
Timestamp: 2024-11-12T04:55:44.973Z
Learning: In `WriteAdditionalNoticePage`, `context.read<NoticeBloc>().state.entity` is validated before entering the page, so force unwrapping with `!` is safe.
🔇 Additional comments (4)
lib/app/modules/notices/presentation/layouts/notice_edit_shell_layout.dart (1)
2-2
: 적절한 import 변경 확인됨!
Material 디자인으로의 전환과 오류 처리를 위한 toast 확장 기능의 import가 올바르게 추가되었습니다.
Also applies to: 5-5
lib/app/modules/notices/presentation/pages/notice_edit_preview_page.dart (1)
4-4
: 토스트 확장 기능 추가 적절
오류 메시지 표시를 위한 toast 확장 기능 import가 적절하게 추가되었습니다.
lib/app/modules/notices/presentation/pages/write_additional_notice_page.dart (2)
5-5
: 토스트 확장 기능 추가 승인
에러 메시지를 표시하기 위한 토스트 확장 기능이 적절하게 임포트되었습니다.
152-163
: 입력 필드 구현 승인
한글과 영문 입력 필드가 적절하게 구현되었으며, 널 안전성이 잘 보장되어 있습니다. 이전 PR에서 학습된 내용과 일치하는 방식으로 구현되었습니다.
lib/app/modules/notices/presentation/pages/write_additional_notice_page.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (1)
lib/app/modules/notices/presentation/bloc/notice_list_bloc.dart (1)
23-39
: 에러 처리 구현을 개선하세요현재 구현은 기본적인 에러 처리를 포함하고 있지만, 다음과 같은 개선사항을 고려해보세요:
- 구체적인 예외 타입을 사용하여 더 세분화된 에러 처리
- 에러 발생 시 이전 상태 복원 로직 추가
try { // ... existing code ... -} catch (e) { +} on SocketException catch (e) { + emit(NoticeListState.error('네트워크 연결을 확인해주세요', state.notices)); +} on TimeoutException catch (e) { + emit(NoticeListState.error('요청 시간이 초과되었습니다', state.notices)); +} catch (e) { emit(NoticeListState.error(e.toString())); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
lib/app/modules/notices/presentation/bloc/notice_list_bloc.dart
(1 hunks)
🧰 Additional context used
📓 Learnings (1)
lib/app/modules/notices/presentation/bloc/notice_list_bloc.dart (2)
Learnt from: 2paperstar
PR: gsainfoteam/ziggle-flutter#494
File: lib/app/modules/notices/presentation/bloc/notice_write_bloc.dart:22-31
Timestamp: 2024-11-12T04:55:44.974Z
Learning: In `lib/app/modules/notices/presentation/bloc/notice_write_bloc.dart`, during initialization, it is intended to emit both `_Error` and `_Draft` states in case of an error because the `Bloc` is a stream object, and the view can capture both states to handle them appropriately.
Learnt from: 2paperstar
PR: gsainfoteam/ziggle-flutter#494
File: lib/app/modules/notices/presentation/bloc/notice_write_bloc.dart:119-123
Timestamp: 2024-11-12T04:55:50.220Z
Learning: In the file `lib/app/modules/notices/presentation/bloc/notice_write_bloc.dart`, within the `NoticeWriteBloc`'s `_Save` event handler, it's intentional to ignore errors when calling `draftSaveRepository.saveDraft` by using `catchError` with an empty handler.
🔇 Additional comments (1)
lib/app/modules/notices/presentation/bloc/notice_list_bloc.dart (1)
53-65
: 구현이 잘 되었습니다!
페이지네이션 구현이 다음과 같은 좋은 사례들을 포함하고 있습니다:
- 전체 개수 확인
- 로딩 중 기존 목록 유지
- 적절한 에러 처리
lib/app/modules/notices/presentation/bloc/notice_list_bloc.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (2)
lib/app/modules/notices/presentation/pages/feed_page.dart (1)
59-65
: 에러 처리 구현이 적절하게 추가되었습니다.
BlocListener
를 통한 에러 처리 구현이 잘 되어있습니다. 사용자에게 에러 메시지를 toast로 표시하는 것은 좋은 UX 방식입니다.다만, 다음과 같은 개선사항을 고려해보시면 좋을 것 같습니다:
child: BlocListener<NoticeListBloc, NoticeListState>( listener: (context, state) => state.mapOrNull( error: (error) => context.showToast(error.message), + loading: (_) => context.showToast(context.t.common.loading), ), child: const ListLayout( noticeType: NoticeType.all, ), ),
로딩 상태에 대한 피드백을 추가하면 사용자 경험을 더욱 향상시킬 수 있습니다.
lib/app/modules/notices/presentation/pages/search_page.dart (1)
43-48
: 오류 처리 구현이 적절하나 추가 고려사항 있음오류 상태에 대한 기본적인 처리가 잘 구현되어 있습니다. 다만 다음 사항들을 고려해보시기 바랍니다:
- 네트워크 연결 오류와 같은 특정 오류 유형에 따른 다른 처리 방식
- 오류 발생 시 재시도 기능 추가
- 치명적인 오류와 일반 오류를 구분하여 처리
예시 코드:
child: BlocListener<NoticeListBloc, NoticeListState>( listener: (context, state) => state.mapOrNull( - error: (error) => context.showToast(error.message), + error: (error) { + if (error.isCritical) { + context.showErrorDialog(error.message); + } else { + context.showToast( + error.message, + action: SnackBarAction( + label: '재시도', + onPressed: () => context.read<NoticeListBloc>().add( + NoticeListEvent.retry(), + ), + ), + ); + } + }, ), child: const _Layout(), ),
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
lib/app/modules/notices/presentation/bloc/notice_list_bloc.dart
(2 hunks)lib/app/modules/notices/presentation/pages/feed_page.dart
(1 hunks)lib/app/modules/notices/presentation/pages/list_page.dart
(2 hunks)lib/app/modules/notices/presentation/pages/search_page.dart
(2 hunks)
🧰 Additional context used
📓 Learnings (1)
lib/app/modules/notices/presentation/bloc/notice_list_bloc.dart (2)
Learnt from: 2paperstar
PR: gsainfoteam/ziggle-flutter#494
File: lib/app/modules/notices/presentation/bloc/notice_write_bloc.dart:22-31
Timestamp: 2024-11-12T04:55:44.974Z
Learning: In `lib/app/modules/notices/presentation/bloc/notice_write_bloc.dart`, during initialization, it is intended to emit both `_Error` and `_Draft` states in case of an error because the `Bloc` is a stream object, and the view can capture both states to handle them appropriately.
Learnt from: 2paperstar
PR: gsainfoteam/ziggle-flutter#494
File: lib/app/modules/notices/presentation/bloc/notice_write_bloc.dart:119-123
Timestamp: 2024-11-12T04:55:50.220Z
Learning: In the file `lib/app/modules/notices/presentation/bloc/notice_write_bloc.dart`, within the `NoticeWriteBloc`'s `_Save` event handler, it's intentional to ignore errors when calling `draftSaveRepository.saveDraft` by using `catchError` with an empty handler.
🔇 Additional comments (4)
lib/app/modules/notices/presentation/pages/list_page.dart (1)
5-5
: 토스트 확장 기능 추가 승인
오류 메시지 표시를 위한 토스트 확장 기능의 import가 적절하게 추가되었습니다.
lib/app/modules/notices/presentation/bloc/notice_list_bloc.dart (2)
40-49
: 새로고침 구현이 잘 되었습니다!
로딩 및 에러 상태에서 이전 목록을 보존하여 사용자 경험이 개선되었습니다.
145-146
: 에러 상태 정의가 개선되었습니다!
매개변수 이름을 'message'로 변경하고 기본값을 추가한 것이 좋습니다.
lib/app/modules/notices/presentation/pages/search_page.dart (1)
7-7
: 토스트 확장 기능 추가 승인
오류 메시지 표시를 위한 토스트 확장 기능 import가 적절하게 추가되었습니다.
Summary by CodeRabbit
Toast
메시지를 추가했습니다.Cupertino
에서Material
로 변경했습니다.BlocListener
를 추가했습니다.