[REFACTOR] 관리되지 않은 API 에러 분기 처리 개선 (UnhandledError) #408

rbgksqkr · 2024-11-20T05:14:19Z

Issue Number

#407

As-Is

관리되지 않은 에러가 네트워크 에러로 처리되고 있었음

To-Be

UnhandledError 클래스를 만들어 네트워크 에러와 분기처리

Check List

테스트가 전부 통과되었나요?
모든 commit이 push 되었나요?
merge할 branch를 확인했나요?
Assignee를 지정했나요?
Label을 지정했나요?

Test Screenshot

에러 분기 처리 개선

기존에는 CustomError가 아니면 NetworkError를 throw 하게 했었는데, 관리되지 않는 에러도 처리하기 위해 분기처리를 추가하였습니다.
CORS에러 때문에 해당 이슈를 제기했는데, TypeError 클래스에 CORS에러가 포함되어 있어 브라우저의 navigator.onLine을 사용해 처리하였습니다. 이 또한 모바일에서 한번 더 확인해야하지만, 쿠키 정책으로 인해 머지 후 확인하겠습니다!

   try {
       ...
    } catch (error) {
      if (error instanceof TypeError && !navigator.onLine) {
        throw new NetworkError();
      }

      if (error instanceof CustomError) {
        throw error;
      }

      throw new UnhandledError();
    }

UnhandledError UI 처리

처리되지 않은 에러를 어떤 UI에서 처리할지 고민했는데 Toast, Modal, ErrorFallback 중 ErrorFallback을 선택하였습니다. 처리되지 않았다면 단순한 에러도 아니고, 사용자가 해결할 수 있는 문제도 아니여서 ErrorFallback을 보여주고 에러를 Sentry로 수집하도록 구현하였습니다.

CORS 에러	네트워크 에러

(Optional) Additional Description

🌸 Storybook 배포 주소

https://woowacourse-teams.github.io/2024-ddangkong/storybook/

novice0840

PR 확인했습니다. 여러 경우에도 항상 에러 처리가 가능하도록 바뀌었군요 굿굿

novice0840 · 2024-11-26T07:23:17Z

frontend/src/providers/QueryClientDefaultOptionProvider/QueryClientDefaultOptionProvider.tsx

          show(error.message);
          return;
        }
+
+        if (error instanceof UnhandledError) {
+          return;
+        }
+
        showModal(AlertModal, { title: '에러', message: error.message });


🙏 제안 🙏

함수 이름 형식을 맞추는 게 좋다고 생각하는데 show을 showToast로 바꾸는 건 어떠한가요?

오 그것도 좋을 것 같네용

흠 근데 모달도 똑같이 show로 되어 있긴한데, showModal로 바꿀까요?

이미 showModal로 되어있는게 아닌가요?
{show: showModal}
{show: showToast} 를 생각하고 제안한 방식이긴 합니다!

앗 사용할 때를 말한거였나요???
근데 항상 그렇게 사용할거면 그냥 함수 이름 자체를 바꾸는게 좋다고 생각해서 모달 띄우는 함수는 showModal, 토스트 띄우는 함수는 showToast로 수정하였습니다!

novice0840 · 2024-11-26T07:41:56Z

frontend/src/apis/fetcher.ts

+      if (error instanceof TypeError && !navigator.onLine) {
+        throw new NetworkError();
+      }
+


💭 질문 💭

네트워크 에러 발생 시 항상 TypeError가 발생한다는 조건문인 것 같은데 네트워크 에러면서 TypeError는 아닌 경우가 있을 수도 있지 않을까요?

어떤 경우가 있을까요?? (진짜 궁금)

일단 fail to fetch 는 TypeError입니다!

근데 그냥 !navigator.onLine 조건문만 달아도 될 것 같긴 하네용

해당 로직과는 상관이 없지만 검색하다가 알게된 사실을 공유드려용
navigator.onLine이 false면 오프라인 상태라고 가정할 수 있지만, true가 반환되었다고 해서 반드시 브라우저가 인터넷에 액세스할 수 있다는 의미는 아니라고 합니다.

네트워크 어댑터와의 연결은 되었지만 실제 인터넷이 되는지는 판단할 수 없다고 합니다.
예를 들어 공용 와이파이 추가 로그인이나 제한된 인터넷 상태에서 true로 반환될 수 있다고 하네용

So while you can assume that the browser is offline when it returns a false value, you cannot assume that a true value necessarily means that the browser can access the internet.

https://developer.mozilla.org/en-US/docs/Web/API/Navigator/onLine

novice0840

피드백 확인했습니다! Approve 할게요

useon

말우 리뷰가 늦어져서 미안합니다 😭unhandledError도 다룰 수 있도록 처리를 하는 모습 정말 좋네요 .. ^ .^ 애니메이션 깎기에 이어 에러 깎기까지 .. 그냥 궁금한 점에 대해서만 코멘트 달았으니 바로 어프루브 합니다 !!!!

useon · 2024-12-03T06:42:35Z

frontend/src/components/common/ErrorFallback/AsyncErrorFallback/AsyncErrorFallback.tsx

@@ -22,7 +21,7 @@ const AsyncErrorFallback = ({ error, resetError }: AsyncErrorFallback) => {
  return (
    <section css={errorFallbackLayout}>
      <img src={ErrorDdangkong} alt="에러나서 슬픈 땅콩" css={errorImage} />
-      <h2 css={errorText}>{error instanceof CustomError && error.message}</h2>
+      <h2 css={errorText}>{error instanceof Error && error.message}</h2>


💭 질문 💭

그럼 여기서 원래는 CustumError만 잡았는데 이제는 Error로 더 넓게 잡는건가요 .. ?

음 사실 정확히는 error instanceof CustomError || error instanceof UnhandledError 로 잡는 게 맞는데 message만 사용하기도 하고, 한줄로 표현하고 싶어서 Error로 잡았던 것 같아요!

근데 썬데이말듣고 생각해보니 나중에 헷갈릴 수도 있을 것 같네용
error instanceof CustomError || error instanceof UnhandledError 로 명확하게 표현하겠습니다!

useon · 2024-12-03T06:51:26Z

frontend/src/providers/QueryClientDefaultOptionProvider/QueryClientDefaultOptionProvider.tsx

@@ -26,6 +28,11 @@ const QueryClientDefaultOptionProvider = ({ children }: PropsWithChildren) => {
          show(error.message);
          return;
        }
+
+        if (error instanceof UnhandledError) {
+          return;


💭 질문 💭

해당 분기가 없으면 unhandledError도 모달이 뜨기 때문에 처리를 해 준 건가요?

넵 맞습니당

rbgksqkr added 3 commits November 20, 2024 14:04

refactor: 네트워크 에러와 관리되지 않는 에러 분기 처리 추가 #407

b83e741

refactor: 에러 상태 코드 상수화 #407

c62e279

refactor: 처리되지 않는 에러도 에러 폴백 UI를 보여주고 sentry로 추적 #407

fcc2e93

rbgksqkr added ♻️ refactor 리팩토링 🫧 FE front end labels Nov 20, 2024

rbgksqkr requested review from useon and novice0840 November 20, 2024 05:14

rbgksqkr self-assigned this Nov 20, 2024

rbgksqkr linked an issue Nov 20, 2024 that may be closed by this pull request

[REFACTOR] 관리되지 않은 API 에러 분기 처리 개선 #407

Closed

1 task

novice0840 requested changes Nov 26, 2024

View reviewed changes

rbgksqkr added 3 commits November 26, 2024 18:53

refactor: 불필요한 에러 조건문 제거 #407

bcb9618

refactor: toast UI 여는 함수명을 명시적으로 showToast로 변경 #407

ea5707f

refactor: modal UI를 띄우는 함수명을 명시적으로 showModal로 변경 #407

040e343

novice0840 approved these changes Nov 28, 2024

View reviewed changes

github-actions bot added D-3 3일 안에 리뷰 부탁드려요:) D-2 2일 안에 리뷰 부탁드려요:) and removed D-3 3일 안에 리뷰 부탁드려요:) labels Dec 3, 2024

useon approved these changes Dec 3, 2024

View reviewed changes

github-actions bot added D-1 1일 안에 리뷰 부탁드려요:) D-0 이제 그냥 머지합니다? and removed D-2 2일 안에 리뷰 부탁드려요:) D-1 1일 안에 리뷰 부탁드려요:) labels Dec 3, 2024

rbgksqkr removed the D-0 이제 그냥 머지합니다? label Dec 3, 2024

github-actions bot added the D-3 3일 안에 리뷰 부탁드려요:) label Dec 3, 2024

rbgksqkr added 2 commits December 3, 2024 17:24

fix: AsyncErrorFallback Error Type 조건문 명확하게 명시 #407

bdd52e9

merge: conflict 해결 #407

1304afe

rbgksqkr merged commit 7572b19 into develop Dec 3, 2024
2 checks passed

GIVEN53 deleted the refactor/#407 branch December 10, 2024 11:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[REFACTOR] 관리되지 않은 API 에러 분기 처리 개선 (UnhandledError) #408

[REFACTOR] 관리되지 않은 API 에러 분기 처리 개선 (UnhandledError) #408

rbgksqkr commented Nov 20, 2024 •

edited

Loading

novice0840 left a comment

novice0840 Nov 26, 2024

rbgksqkr Nov 26, 2024

rbgksqkr Nov 26, 2024

novice0840 Nov 28, 2024

rbgksqkr Nov 30, 2024

novice0840 Nov 26, 2024

rbgksqkr Nov 26, 2024 •

edited

Loading

rbgksqkr Nov 26, 2024

rbgksqkr Nov 26, 2024

novice0840 left a comment

useon left a comment

useon Dec 3, 2024

rbgksqkr Dec 3, 2024 •

edited

Loading

useon Dec 3, 2024

rbgksqkr Dec 3, 2024

[REFACTOR] 관리되지 않은 API 에러 분기 처리 개선 (UnhandledError) #408

[REFACTOR] 관리되지 않은 API 에러 분기 처리 개선 (UnhandledError) #408

Conversation

rbgksqkr commented Nov 20, 2024 • edited Loading

Issue Number

As-Is

To-Be

Check List

Test Screenshot

에러 분기 처리 개선

UnhandledError UI 처리

(Optional) Additional Description

🌸 Storybook 배포 주소

novice0840 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

🙏 제안 🙏

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

💭 질문 💭

rbgksqkr Nov 26, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

novice0840 left a comment

Choose a reason for hiding this comment

useon left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

💭 질문 💭

rbgksqkr Dec 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

💭 질문 💭

Choose a reason for hiding this comment

rbgksqkr commented Nov 20, 2024 •

edited

Loading

rbgksqkr Nov 26, 2024 •

edited

Loading

rbgksqkr Dec 3, 2024 •

edited

Loading