feat(be): implement GraphQL Exceptions #1267

SH9480P · 2024-01-29T08:32:39Z

Description

Admin Server에서 Client Server와 동일한 방식으로 HTTP Exception을 던지면 다음과 같은 문제가 발생합니다.

HTTP Status Code가 반영되지 않음: original error property로 status code가 들어가긴 하지만, 실제 response의 status code가 변경되지는 않습니다. 또한, field validation error 등으로 발생하는 GraphQL Error에는 status code가 전혀 포함되지 않기 때문에, status code로 error case를 구분하는 데에는 한계가 있습니다.
GraphQL Error Code가 제각기 다름: @nestjs/apollo 패키지를 들여다본 결과, default format error function을 통해 HTTP status code가 400, 401, 403인 경우에는 따로 정의된 Gql Error Code를 전달해주지만, 그 외의 경우에는 INTERAL_SERVER_ERROR를 전달하도록 구현되어있습니다.

위 문제를 해결하기 위해, Client Server에서 사용하던 HTTP Exception이 의미하는 error case를 그대로 가져와 Gql Error Code를 담은 Exception을 만들어 직접 던지도록 구현하고자 했습니다. 그러나, 이 방식에도 단점이 존재했습니다.

Guard에서는 현재 실행 컨텍스트 타입이 gql인지 확인하고, 상황에 맞는 GraphQL Error를 던져야 해서 번거로웠습니다. Pipe에서는 gql인지 아닌지 확인이 불가능하기 때문에, 다음과 같은 방법을 취해야 했습니다.

pipe로 gql임을 나타내는 인자를 따로 전달하여 예외처리
gql error만을 던지는 새로운 pipe를 만들어 적용
모든 Args를 dto로 만들고, class-validator로 검사

위 방법 중에는 마지막 대안이 제일 깔끔해 보이긴 했으나, 1~2개밖에 없는 인자에 dto를 씌우는 것이 가독성을 저해하는 요소가 되지는 않을지 의문이 들었고, 무엇보다 guard, pipe 등 common library component에 gql인지 구분하는 로직을 포함시켜야 한다는 번거로움 때문에 확신이 들지 않았습니다.

그래서 새로운 방법을 생각해보았습니다.

Resolver, Guard, Pipe는 Client API에서와 동일하게 Nestjs built-in Http Exception을 사용
Apollo Format Error 기능을 통해, 우리가 주로 사용하는 400, 401, 403, 404, 409, 422 에러를 extension.originalError로 확인하고 이에 맞는 Custom Gql Error Code로 변환

이는 다음과 같은 장점과 단점이 있습니다.

장점: 구현 시, GQL 에러를 신경쓰지 않아도 됨
단점: 400, 401, 403, 404, 409, 422 에러가 아닌 다른 상태 코드를 사용할 경우, apollo format error 옵션에도 추가해주어야 함. 추가하지 않으면 4XX 에러가 INTERNAL_SERVER_ERROR로 뜰 것임.

위와 같이 구혔했으니 참고해주시기 바랍니다.

추가로, BusinessException에 convert2HTTPException 메서드를 생성하여, 컨트롤러 단에서 비즈니스 로직을 확인하는 과정에서 transport layer의 exception을 직접 import하고 생성하여 반환하지 않게 구조를 변경합니다.

Additional context

controller에서 비즈니스 예외로 처리되지 않았거나, Admin API에서 GraphQLError로 변경되지 않은 부분을 일부 포함합니다.
이슈 단위가 커지는 것을 우려하여, 이는 #1273 으로 분리하여 처리합니다.

Before submitting the PR, please make sure you do the following

Read the Contributing Guidelines
Read the Contributing Guidelines and follow the Commit Convention
Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
Ideally, include relevant tests that fail without this PR but pass with it.

…t2~ methods

vercel · 2024-01-29T08:32:44Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
codedang	✅ Ready (Inspect)	Visit Preview	Feb 3, 2024 9:42am

SH9480P · 2024-01-31T11:57:57Z

~~구현할 거 남음~~
구현완료, PR Review Please

HTTPException의 상태 코드를 GQL 에러 코드로 변환시켜줌

backend/libs/exception/src/business.exception.ts

- Custom GraphQL Errors 삭제 - 파일 이름 변경 (graphql-error.exception -> graphql-error-code)

Jaehyeon1020

Admin User 모듈도 수정 부탁드려요! 근데 Admin User 모듈은 서비스 코드에서 Business Exception을 사용하지 않고 바로 에러를 던지게 구현이 되어있어서 그부분도 수정이 필요할 것 같습니다 ..

SH9480P · 2024-02-03T06:28:48Z

Admin User 모듈도 수정 부탁드려요! 근데 Admin User 모듈은 서비스 코드에서 Business Exception을 사용하지 않고 바로 에러를 던지게 구현이 되어있어서 그부분도 수정이 필요할 것 같습니다 ..

그래서 일부러 안했습니다
#1273 에서 PrismaClientKnownRequestError 넣어서 코드 개선 예정인데, 거기서 한번에 변경하는 것이 어떨까 싶습니다

Jaehyeon1020

수고하셨습니다~

* feat: create transport exceptions for GraphQL Request * feat: add exception conversion onto the transport level * feat: add optional exception message setting argument * refactor: add default message to InternalServerGraphQLError * refactor: replace error throwing logic in controllers by using convert2~ methods * refactor: change exception name * feat: add apollo server error formatter HTTPException의 상태 코드를 GQL 에러 코드로 변환시켜줌 * fix: make resolvers throw HTTP Exceptions * fix: delete status property * refactor: refactor apollo-error-formatter * fix: change Gql Error to HTTP Error * refactor: delete unused class method * refactor: delete unused exceptions - Custom GraphQL Errors 삭제 - 파일 이름 변경 (graphql-error.exception -> graphql-error-code) --------- Co-authored-by: Jaehyeon Kim <[email protected]>

SH9480P added 5 commits January 28, 2024 05:53

feat: create transport exceptions for GraphQL Request

1db0e25

feat: add exception conversion onto the transport level

92d5be2

feat: add optional exception message setting argument

23be576

refactor: add default message to InternalServerGraphQLError

71fa8ac

refactor: replace error throwing logic in controllers by using conver…

1f081ad

…t2~ methods

SH9480P self-assigned this Jan 29, 2024

SH9480P linked an issue Jan 29, 2024 that may be closed by this pull request

GraphQL Base Exception 구현 #1258

Closed

3 tasks

refactor: change exception name

dc03505

vercel bot deployed to Preview January 29, 2024 08:44 View deployment

gyunseo mentioned this pull request Jan 31, 2024

feat(be): implement admin announcement module #1270

Closed

4 tasks

cho-to added the ⛳️ team-backend label Feb 1, 2024

SH9480P added 4 commits February 1, 2024 08:06

feat: add apollo server error formatter

2f89b36

HTTPException의 상태 코드를 GQL 에러 코드로 변환시켜줌

fix: make resolvers throw HTTP Exceptions

8d315a0

fix: delete status property

5aad8a9

refactor: refactor apollo-error-formatter

67025e0

vercel bot deployed to Preview February 1, 2024 08:48 View deployment

fix: change Gql Error to HTTP Error

d2e7f87

vercel bot deployed to Preview February 1, 2024 09:08 View deployment

dotoleeoak reviewed Feb 1, 2024

View reviewed changes

backend/libs/exception/src/business.exception.ts Outdated Show resolved Hide resolved

refactor: delete unused class method

d6670a4

vercel bot deployed to Preview February 2, 2024 01:32 View deployment

Jaehyeon1020 mentioned this pull request Feb 2, 2024

Admin Problem api docs 보충 #1264

Closed

3 tasks

refactor: delete unused exceptions

c933636

- Custom GraphQL Errors 삭제 - 파일 이름 변경 (graphql-error.exception -> graphql-error-code)

SH9480P requested a review from dotoleeoak February 2, 2024 07:24

vercel bot deployed to Preview February 2, 2024 07:24 View deployment

SH9480P requested review from gyunseo, Lee-won-hyeok and Jaehyeon1020 February 3, 2024 02:41

SH9480P requested review from cho-to, lshtar13 and mnseok February 3, 2024 02:41

Jaehyeon1020 reviewed Feb 3, 2024

View reviewed changes

SH9480P requested a review from Jaehyeon1020 February 3, 2024 09:21

Jaehyeon1020 approved these changes Feb 3, 2024

View reviewed changes

Merge branch 'main' into 1258-graphql-base-exception

a07e623

vercel bot deployed to Preview February 3, 2024 09:43 View deployment

Jaehyeon1020 merged commit b580107 into main Feb 3, 2024
10 checks passed

Jaehyeon1020 deleted the 1258-graphql-base-exception branch February 3, 2024 09:43

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(be): implement GraphQL Exceptions #1267

feat(be): implement GraphQL Exceptions #1267

SH9480P commented Jan 29, 2024 •

edited

Loading

vercel bot commented Jan 29, 2024 •

edited

Loading

SH9480P commented Jan 31, 2024 •

edited

Loading

Jaehyeon1020 left a comment

SH9480P commented Feb 3, 2024

Jaehyeon1020 left a comment

feat(be): implement GraphQL Exceptions #1267

feat(be): implement GraphQL Exceptions #1267

Conversation

SH9480P commented Jan 29, 2024 • edited Loading

Description

Additional context

Before submitting the PR, please make sure you do the following

vercel bot commented Jan 29, 2024 • edited Loading

SH9480P commented Jan 31, 2024 • edited Loading

Jaehyeon1020 left a comment

Choose a reason for hiding this comment

SH9480P commented Feb 3, 2024

Jaehyeon1020 left a comment

Choose a reason for hiding this comment

SH9480P commented Jan 29, 2024 •

edited

Loading

vercel bot commented Jan 29, 2024 •

edited

Loading

SH9480P commented Jan 31, 2024 •

edited

Loading