Skip to content
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

DEP-363 feat: api 에러 시 토스트 띄우기 #207

Merged
merged 13 commits into from
Feb 4, 2023

Conversation

kimhyeing
Copy link
Contributor

@kimhyeing kimhyeing commented Jan 16, 2023

💁‍♂️ 변경 내용

AS-IS

  • api 에러 시 처리가 되나, 그에 맞는 액션이 없거나 처리되지 않아 앱이 꺼졌습니다.

TO-BE

  • 네트워크 연결 실패 시 토스트를 띄웁니다
  • api와 연결에 성공은 했으나 400,500 번대의 에러일 경우 errorBody에서 가져온 메시지로 에러 토스트를 띄웁니다.

📢 전달사항

  • DataState 로직 대신에 코틀린 기본의 Result를 사용하게되어 다 삭제해버렸..는데 유연하게 로직 원하시는 방향으로 수정하셔도 됩니다 (그러나 Result 에 커스텀 Exception이 같이 담겨와서.. Result는 같이 사용하셔야 할 것 같습니다)

  • getResult 함수는 Result<ApiResponse<T>>Resut<T> 로 변환하는 함수입니다.

  1. 여기서 Result.success , Result.failure 등으로 분류해서 return을 하게 되는데, Result.success(value)에서 value값에 null이 허용되지 않기 때문에, success임에도 datanull인 경우는 Result로 감싸지 않은 null 자체를 리턴하게 하여, getResult를 사용하는 함수 측에서 디폴트 data값을 설정하도록 했습니다.
  2. datanull임에도 성공하는 몇가지 경우 (ex) 습관 리스트가 아무것도 없는 경우) 는 따로 Result.success(적절한 디폴트 값)DataSourceImpl 에 엘비스 연산자로 넣어주면 됩니다.
override suspend fun getHabits(status: String): Result<List<HabitEntity>> {
      return habitService.getHabits(status).getResult() ?: Result.success(emptyList())
  }
  1. 만약 data가 null인게 에러인 상황이라면 Result.failure 로 전달해주시면 됩니다
override suspend fun postHabit(request: PostHabitRequest): Result<SingleHabitEntity> {
      return habitService.postHabit(request).getResult() ?: Result.failure(ThreeDaysException("데이터가 비어있습니다.", IllegalStateException()))
  }
  • 모든 ViewModel에서 예외(onFailure에서 받는 값)은 as ThreeDaysException 을 사용해주셔야 message(서버에서 가져온 토스트용 메시지 값) 을 사용하실 수 있습니다.
habitRepository.createHabit(habit)
    .onSuccess {
        _action.emit(Action.SaveClick)
    }.onFailure { throwable ->
        throwable as ThreeDaysException
        sendErrorMessage(throwable.message)
    }
  • sendErrorMessageBaseViewModel에 있는 함수로, BaseViewModel에 있는 error란 변수에 메시지를 집어넣어 방출합니다. 이를 Activity / Fragment에서 가져다가 토스트로 출력하실 때 사용하시면 됩니다.
viewModel.error
    .onEach { errorMessage -> ThreeDaysToast().error(this, errorMessage) }
    .launchIn(lifecycleScope)
  • @junhaesung MateViewModel 에서 짝궁 삭제 시, 실패할 때도 짝궁 삭제 성공했다는 토스트를 띄우는 로직이 있어서 임의로 주석처리 해놨습니당..!

@kimhyeing kimhyeing self-assigned this Jan 16, 2023
@kimhyeing kimhyeing requested a review from a team as a code owner January 16, 2023 16:13
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 14 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

toast.view = view
toast.duration = duration
toast.show()
}
Copy link
Member

Choose a reason for hiding this comment

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

show()랑 내용이 거의 겹치는 것 같은데
분기 처리 조금 넣어서 하나로 합쳐도 괜찮을 것 같아용

Copy link
Contributor Author

Choose a reason for hiding this comment

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

저도 고민을 했는데.. 뭔가 그럴려면 파라미터로 true/false를 받거나 그런식일 것 같은데 오히려 사용할때 헷갈릴 것 같아서 명시적인 함수명으로 변경하는게 에러용인지 아닌지 판단하기 쉬울 것 같아서욥.. 이건 저도 한번 더 생각해보겠습니다!

}.onFailure { throwable ->
throwable as ThreeDaysException
sendErrorMessage(throwable.message)
}
Copy link
Member

Choose a reason for hiding this comment

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

나중에 flow에서 제공하는 방법도 사용해보면 좋을 것 같아요! (링크)
값이 비어있을 때도 처리할 수 있고.. 다양한 것 같아요
저도 이건 안 써봐서 다음에 한 번 해보려고요. 공유 차원에서 남겨봅니다아

Copy link
Contributor Author

Choose a reason for hiding this comment

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

오 한번 적용ㄹ해보겠습니다!! 감사합니다! 아직 공부할게 많네요...

fetchMate()
_uiEffect.emit(
value = UiEffect.ShowToastMessage(R.string.delete_mate)
)
Copy link
Member

Choose a reason for hiding this comment

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

삭제 성공시 null을 받아오는데
받는 타입이 맞지 않아서 항상 onFailure로 빠진다는 거죠??
이해한 게 맞다면 지라에 이슈 등록 해놓고 다음 배포 때 해결해보겠습니당

Copy link
Member

@juhwankim-dev juhwankim-dev Jan 26, 2023

Choose a reason for hiding this comment

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

는 해결해주셨군요
시간 순서대로 봤더니... 뒤쪽에서 해결해주셨네요 감사합니다 👍
주석만 없애주시면 될 것 같아요!

// METHOD_ARGUMENT_NOT_VALID_EXCEPTION_BIND_EXCEPTION, BAD_REQUEST, UNKNOWN_ERROR -> "알 수 없는 오류가 발생했어요."
// NO_INTERNET_CONNECTION -> "인터넷 연결이 끊겼습니다."
// else -> null
// }
Copy link
Member

Choose a reason for hiding this comment

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

요거 안 쓰는 방향으로 바꾼 이유가 있을까욤 ?_?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

처음에 서버에서 메시지가 제대로 구축..? 이 안된 상황이여서 제가 코드받고 그 코드에 해당되는 메시지를 띄우려고했는데 지금은 서버에서 토스트용 메시지로 바로 보내주셔서 코드->메시지 변환처리 과정이 필요없어졌습니다! 근데 혹시몰라 주석처리만 해놨었어욥

@juhwankim-dev
Copy link
Member

양이 어마어마하네요... 정말 x 3 고생 많으셨습니다

@kimhyeing kimhyeing merged commit 5224d60 into develop Feb 4, 2023
@kimhyeing kimhyeing deleted the feature/DEP-363_handling_error branch February 4, 2023 07:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants