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

에러 핸들링을 위한 컴포넌트 추가 및 적용 #546

Merged
merged 18 commits into from
Oct 11, 2023

Conversation

solo5star
Copy link
Member

@solo5star solo5star commented Oct 8, 2023

#️⃣ 연관된 이슈

resolve #540

📝 작업 내용

  • 에러 핸들링을 위한 컴포넌트 추가 및 적용

👻 에러 종류

에러 종류를 다음과 같이 정의하였습니다.

  • e is unknown
    • string, array 등 모든 타입
  • e is Error
    • 정형화된 에러 객체
    • 모든 에러는 Error의 서브 타입이며 복구 불가능한 에러를 포함한다 (TypeError 등)
    • 개발자가 조치해야 한다
    • 예를 들면 throw new Error('부모 트리에서 ToastProvider를 사용해주세요') 가 해당된다.
  • e is AppError
    • Error의 서브 타입이며, 복구 가능한 에러이다
    • 앱 내에서 의도적으로 발생시킨 에러
    • 사용자가 조치할 수 있다
    • 예를 들면 throw new AppError('잘못된 텍스트입니다') 가 해당된다.
  • e is NetworkError
    • AppError 의 서브타입이며, 네트워크 장애에 해당되는 에러
  • e is APIError
    • AppError 의 서브타입이며, API 호출 중 400, 500일 시 이 에러에 해당
    • 백엔드에서 응답한 에러 코드 및 메세지를 포함한다

👻 ErrorBoundary

  • ErrorBoundary

    • 컴포넌트 렌더링 중 에러 발생 시 catch하는 컴포넌트
    • catch 시 대체 컴포넌트(fallback)을 렌더링 함
    • fallbackRender, FallbackComponent props로 대체 컴포넌트를 렌더링할 수 있다
    • caught props로 핸들링 할 에러의 범위를 지정할 수 있다
  • QueryErrorBoundary

    • react-query 에서 suspense: true 로 사용할 시 유용한 에러 바운더리
    • query reset, network online 시 자동으로 retry 기능이 내장되어 있다

👻 useSilentLink

네트워크 상태를 감지하고 오프라인 혹은 온라인 전환될 시 토스트 메세지를 띄운다

👻 retry 는 껐습니다

retry가 기본으로 3회인데, 사용자가 에러든 응답이든 피드백을 받기까지 너무 오래 걸리는 것 같아 retry를 껐습니다

💬 리뷰 요구사항

몹 프로그래밍 이후에 수정한 사항이 많이 있으니 꼭 확인 부탁드립니다!

solo5star and others added 18 commits October 5, 2023 19:16
Co-authored-by: jeongwusi <[email protected]>
Co-authored-by: geuntaek1013 <[email protected]>
Co-authored-by: jeongwusi <[email protected]>
Co-authored-by: geuntaek1013 <[email protected]>
Co-authored-by: jeongwusi <[email protected]>
Co-authored-by: geuntaek1013 <[email protected]>
Co-authored-by: jeongwusi <[email protected]>
Co-authored-by: geuntaek1013 <[email protected]>
Co-authored-by: jeongwusi <[email protected]>
Co-authored-by: geuntaek1013 <[email protected]>
* caught, FallbackComponent 추가
* fallbackRender에서 error도 주입
@solo5star solo5star added 카테고리:기능🛠️ 만들어줘잉 FE 될 때까지 한다잉 작업:몹 몹 프로그래밍으로 진행이 필요합니다 우선순위:🟡보통 급하진 않지만, 필요한 작업입니다. 카테고리:리팩토링🪚 리팩토링입니다. labels Oct 8, 2023
@solo5star solo5star linked an issue Oct 8, 2023 that may be closed by this pull request
Copy link
Collaborator

@jeongwusi jeongwusi left a comment

Choose a reason for hiding this comment

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

간단하게 코멘트 확인 부탁드립니다^^

client/src/App.tsx Show resolved Hide resolved
client/src/hooks/useSilentLink.ts Show resolved Hide resolved
client/src/errors/AppError.ts Show resolved Hide resolved
client/src/errors/APIError.ts Show resolved Hide resolved
Copy link
Collaborator

@jeongwusi jeongwusi left a comment

Choose a reason for hiding this comment

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

확인하였습니다!

Copy link
Collaborator

@geuntaek1013 geuntaek1013 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~!! 간단한 리뷰 확인해주세요:)

@@ -0,0 +1,9 @@
import AppError from './AppError';

class NetworkError extends AppError {
Copy link
Collaborator

Choose a reason for hiding this comment

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

NetworkErrorAppError의 서브 타입으로 정의한 이유가 있으실까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

Error의 서브 타입이며, 복구 가능한 에러이다

앱 내에서 의도적으로 발생시킨 에러

사용자가 조치할 수 있다

AppError를 위와 같이 정의하여서 NetworkErrorAppError의 서브 클래스가 될 수 있습니다. "사용자가 조치할 수 있다"는 말은 즉슨 저희가 이 에러를 핸들링하여 사용자에게 적절한 UI를 제공할 수 있다면 AppError 에 해당될 수 있습니다.

<MainSentence>다시 확인해주세요</MainSentence>
<Sentence>요청하신 내용을 찾을 수 없어요</Sentence>
<MainSentence>{isExpectedError ? '다시 확인해주세요' : '예기치 못한 에러가 발생하였습니다'}</MainSentence>
{isExpectedError && <Sentence>{message}</Sentence>}
Copy link
Collaborator

Choose a reason for hiding this comment

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

isExpectedError가 true일 때, Error 인스턴스의 error.message를 렌더링하는데, 이 부분을 사용자에게 그대로 노출할 필요가 있을까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

isExpectedError인 경우 사용자에게 표시할 만한 메세지가 나타날 것이기 때문에 적절한 코드라고 생각합니다!

@@ -1,31 +1,36 @@
import { MdOutlineErrorOutline } from 'react-icons/md';
import { Link } from 'react-router-dom';
import { Link, useRouteError } from 'react-router-dom';
Copy link
Collaborator

Choose a reason for hiding this comment

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

오 router에 이런 훅이 있었나요? 덕분에 하나 배워갑니다!!👍
페이지 라우팅 중에 액션, 로더, 렌더링 중에 발생한 모든 오류를 반환한다는데 개발자가 따로 처리해야 할 작업이 없나요?
error에 할당하여 사용하면 끝인가요?

Copy link
Member Author

Choose a reason for hiding this comment

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

넵 그렇습니다. 그리고 그 error를 어떻게 취급할 것인지는 개발자에게 달려있다고 할 수 있습니다.

const [toastState, setToastState] = useState<{ variant: ToastVariant; message: string | null }>({
variant: 'default',
message: null,
});

useEffect(() => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

useEffect 는 이제 필요가 없어서 삭제하신건가요?😲

Copy link
Member Author

Choose a reason for hiding this comment

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

그렇습니다~!


return (
<ToastContext.Provider value={contextValue}>
{children}
{toastState.message !== null && (
<Container>
<Container key={toastId}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

key를 상위 부모인 Container에 주신 이유가 있으신가요?

Copy link
Member Author

Choose a reason for hiding this comment

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

토스트 애니메이션을 다시 재생하기 위해 DOM이 새로운 DOM으로 재생성될 필요가 있고, 이를 key 속성을 통해 하고있다고 보심 됩니다!

};
}

static getDerivedStateFromError(error: Error): ErrorBoundaryState {
Copy link
Collaborator

Choose a reason for hiding this comment

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

getDerivedStateFromErrorstatic 정적 메서드로 설정하신 이유가 있으신가요?

Copy link
Member Author

Choose a reason for hiding this comment

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

React에서 error를 처리하는 방법이 그렇기 때문입니다!

@solo5star solo5star merged commit 01a4ddc into dev Oct 11, 2023
1 check failed
@solo5star solo5star deleted the feat/540-improved-error-handling branch October 11, 2023 09:27
@solo5star solo5star restored the feat/540-improved-error-handling branch October 11, 2023 09:27
@solo5star solo5star deleted the feat/540-improved-error-handling branch October 11, 2023 09:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FE 될 때까지 한다잉 우선순위:🟡보통 급하진 않지만, 필요한 작업입니다. 작업:몹 몹 프로그래밍으로 진행이 필요합니다 카테고리:기능🛠️ 만들어줘잉 카테고리:리팩토링🪚 리팩토링입니다.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

세분화된 에러 핸들링
3 participants