[REFACOTR] 레포 등록시 깃허브 웹훅 등록 과정에서 403에러를 애플리케이션 에러로 전환 #96
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughGitHub 403 Forbidden을 전파·처리하기 위해 예외 GithubForbiddenException을 추가하고, GitHub 클라이언트 필터와 RepositoryFacadeService.save에서 403을 404와 동일하게 매핑하도록 예외 처리 분기와 테스트를 확장했습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant S as RepositoryFacadeService
participant G as GithubClient/Webhook API
participant E as Exception Mapper
S->>G: createWebhook(repo)
alt Success (2xx)
G-->>S: Webhook created
S-->>S: 저장 및 이벤트 발행
else 404 Not Found
G-->>S: throw GithubNotFoundException
S->>E: map to GssException(REGISTRY_GITHUB_REPOSITORY_NOT_FOUND)
E-->>S: GssException
else 403 Forbidden
G-->>S: throw GithubForbiddenException
S->>E: map to GssException(REGISTRY_GITHUB_REPOSITORY_NOT_FOUND)
E-->>S: GssException
else Other errors
G-->>S: throw GssException(GITHUB_CLIENT_ERROR)
end
sequenceDiagram
autonumber
participant C as GithubExchangeFilterFunction
participant GH as GitHub API
C->>GH: HTTP request
GH-->>C: HTTP response
alt 404
C-->>C: throw GithubNotFoundException
else 403
C-->>C: throw GithubForbiddenException
else Other error (4xx/5xx)
C-->>C: throw GssException(GITHUB_CLIENT_ERROR)
else 2xx
C-->>C: proceed
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Test Results0 tests 0 ✅ 0s ⏱️ Results for commit 296d29f. |
📝 Test Coverage Report
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
gss-client/gss-github-client/src/main/java/com/devoops/exception/GithubForbiddenException.java (1)
3-8: 예외 생성자/직렬화 ID 보강 제안원인 추적과 직렬화 안전성을 위해 cause 생성자와 serialVersionUID 추가를 권장합니다.
public class GithubForbiddenException extends RuntimeException { + private static final long serialVersionUID = 1L; public GithubForbiddenException(String message) { super(message); } + + public GithubForbiddenException(String message, Throwable cause) { + super(message, cause); + } }gss-client/gss-github-client/src/main/java/com/devoops/client/GithubExchangeFilterFunction.java (2)
34-36: 403 분기: HttpStatus 사용 및 문구/마침표 통일가독성/일관성을 위해 HttpStatus.FORBIDDEN을 사용하고, 안내 문구를 자연스러운 표현으로 마침표까지 통일해주세요.
- if(response.statusCode().isSameCodeAs(HttpStatusCode.valueOf(403))) { - return Mono.error(new GithubForbiddenException("깃허브 자원의 접근 권한이 없습니다")); + if (response.statusCode().isSameCodeAs(HttpStatus.FORBIDDEN)) { + return Mono.error(new GithubForbiddenException("깃허브 자원에 대한 접근 권한이 없습니다.")); }추가(파일 상단 import):
import org.springframework.http.HttpStatus;
25-31: 오류 본문/URL 전체 로깅은 민감정보 및 과다 로그 위험에러 시 응답 본문 전체와 URL 전체를 그대로 남기면 토큰/개인정보 노출 및 로그 폭증 위험이 있습니다. 경로만 로그하고 본문은 길이 제한/마스킹을 권장합니다.
예시:
- log.error("GitHub API Error: {}", Map.of( - "method", request.method(), - "url", request.url(), - "status", response.statusCode(), - "response", body - )); + String responseSnippet = body.length() > 1024 ? body.substring(0, 1024) + "..." : body; + log.error("GitHub API Error: {}", Map.of( + "method", request.method(), + "urlPath", request.url().getPath(), + "status", response.statusCode().value(), + "responseSnippet", responseSnippet + ));gss-api-app/src/main/java/com/devoops/service/facade/RepositoryFacadeService.java (1)
46-48: 403→NOT_FOUND 매핑 정책 확인 및 원인 로깅 추가 제안403을 404와 동일한 애플리케이션 에러로 매핑하는 요구사항이 맞는지 확인 부탁드립니다. 또한 디버깅을 위해 원인 로그를 남기는 편이 좋습니다.
@@ - } catch (GithubNotFoundException | GithubForbiddenException githubException) { - throw new GssException(ErrorCode.REGISTRY_GITHUB_REPOSITORY_NOT_FOUND); + } catch (GithubNotFoundException | GithubForbiddenException githubException) { + log.warn("GitHub webhook 등록 실패로 등록 롤백: {}", githubException.getMessage(), githubException); + throw new GssException(ErrorCode.REGISTRY_GITHUB_REPOSITORY_NOT_FOUND); }추가(파일 상단):
+import lombok.extern.slf4j.Slf4j; @@ -@Service +@Service +@Slf4j @RequiredArgsConstructor public class RepositoryFacadeService {가능하다면 GssException이 cause를 받는 생성자를 제공/사용해 원인 체인을 보존해주세요.
gss-api-app/src/test/java/com/devoops/service/facade/RepositoryFacadeServiceTest.java (3)
63-71: 상호작용 검증 추가로 테스트 안정성 강화예외 매핑만 검증 중입니다. 클라이언트 호출이 의도대로 1회 시도됐는지도 함께 검증하면 회귀를 잡는 데 유익합니다.
assertThatThrownBy(() -> repositoryFacadeService.save(request, user)) .isInstanceOf(GssException.class) .hasMessage(ErrorCode.REGISTRY_GITHUB_REPOSITORY_NOT_FOUND.getMessage()); + + Mockito.verify(gitHubClient, times(1)).getRepositoryInfo(any(), any(), any()); + Mockito.verify(gitHubClient, times(1)).createWebhook(any(), any(), any(), any());
73-82: 403 시나리오도 동일하게 상호작용 검증403 케이스에도 동일한 호출 수 검증을 추가해 주세요.
assertThatThrownBy(() -> repositoryFacadeService.save(request, user)) .isInstanceOf(GssException.class) .hasMessage(ErrorCode.REGISTRY_GITHUB_REPOSITORY_NOT_FOUND.getMessage()); + + Mockito.verify(gitHubClient, times(1)).getRepositoryInfo(any(), any(), any()); + Mockito.verify(gitHubClient, times(1)).createWebhook(any(), any(), any(), any());
136-144: 불필요한 지역 변수 제거
mockWebHookCreateResponse가 사용되지 않습니다. 제거해 테스트 노이즈를 줄여주세요.- WebHookCreateResponse mockWebHookCreateResponse = new WebHookCreateResponse(123); Mockito.when(gitHubClient.getRepositoryInfo(anyString(), anyString(), anyString())) .thenReturn(mockResponse); Mockito.when(gitHubClient.createWebhook(any(), any(), any(), any())) .thenThrow(exception);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
gss-api-app/src/main/java/com/devoops/service/facade/RepositoryFacadeService.java(2 hunks)gss-api-app/src/test/java/com/devoops/service/facade/RepositoryFacadeServiceTest.java(3 hunks)gss-client/gss-github-client/src/main/java/com/devoops/client/GithubExchangeFilterFunction.java(2 hunks)gss-client/gss-github-client/src/main/java/com/devoops/exception/GithubForbiddenException.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
gss-client/gss-github-client/src/main/java/com/devoops/client/GithubExchangeFilterFunction.java (1)
gss-client/gss-github-client/src/main/java/com/devoops/exception/GithubForbiddenException.java (1)
GithubForbiddenException(3-8)
gss-api-app/src/main/java/com/devoops/service/facade/RepositoryFacadeService.java (1)
gss-client/gss-github-client/src/main/java/com/devoops/exception/GithubForbiddenException.java (1)
GithubForbiddenException(3-8)
gss-api-app/src/test/java/com/devoops/service/facade/RepositoryFacadeServiceTest.java (1)
gss-client/gss-github-client/src/main/java/com/devoops/exception/GithubForbiddenException.java (1)
GithubForbiddenException(3-8)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build-and-push
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
🚩 연관 이슈
closd #92
🔂 변경 내역
Summary by CodeRabbit