[FEATURE] ExternalApiException 내 resposeBody 필드 추가 및 에러 핸들링 추가#36
[FEATURE] ExternalApiException 내 resposeBody 필드 추가 및 에러 핸들링 추가#36
ExternalApiException 내 resposeBody 필드 추가 및 에러 핸들링 추가#36Conversation
WalkthroughAdds a new external API error status, introduces ExternalApiException response body support, updates Feign executor to capture and propagate the body, and adds an exception handler that returns 502 with the external response body. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant Ctrl as Controller/Service
participant Feign as SafeFeignExecutor
participant Ext as External API
participant Adv as ExceptionAdvice
participant Sen as Sentry
C->>Ctrl: Request triggering external call
Ctrl->>Feign: execute(...)
Feign->>Ext: HTTP request
Ext-->>Feign: Error response (e.g., 4xx/5xx + body)
Note over Feign: FeignException thrown
Feign->>Feign: Extract body (contentUTF8)
Feign-->>Ctrl: throw ExternalApiException(msg, body, cause)
Ctrl-->>Adv: Exception propagates
Adv->>Sen: capture(exception)
Adv-->>C: 502 BAD_GATEWAY + ApiResponse(body, _EXTERNAL_API_ERROR)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/main/java/com/example/spot/common/api/exception/base/ExternalApiException.java (2)
5-5: Storing raw external response: consider size/PII guardPersisting entire external response bodies can be large and may contain PII/secrets. Even if you don’t log it, it’s later returned to clients via the advice. Recommend sanitizing and truncating at the capture site (see SafeFeignExecutor comment) or defaulting to a bounded, sanitized representation here.
7-10: ExternalApiException constructor: rename parameters, widen cause type, and null-safe responseBodyOnly one instantiation of the current three-arg constructor remains (
SafeFeignExecutor.java:19–21), so renaming parameters and acceptingThrowablewon’t break any call sites. Applying the suggested refactor will improve clarity and guard against null response bodies.Affected location:
- src/main/java/com/example/spot/common/api/exception/base/ExternalApiException.java
Apply this diff:
--- a/src/main/java/com/example/spot/common/api/exception/base/ExternalApiException.java +++ b/src/main/java/com/example/spot/common/api/exception/base/ExternalApiException.java @@ -7,10 +7,11 @@ /** * Constructs a new ExternalApiException. - * @param s the detail message - * @param body the raw response body - * @param e the underlying exception + * @param message the detail message + * @param responseBody the raw response body (empty string if null) + * @param cause the underlying throwable cause */ - public ExternalApiException(String s, String body, Exception e) { - super(s, e); - this.responseBody = body; + public ExternalApiException(String message, String responseBody, Throwable cause) { + super(message, cause); + this.responseBody = (responseBody == null) ? "" : responseBody; }src/main/java/com/example/spot/common/infrastructure/feign/SafeFeignExecutor.java (1)
25-37: Masking patterns: consider precompiled Pattern and centralizing sanitizerRepeated
replaceAllcompiles regexes each call. For hot paths, precompilePatterns or move the sanitizer to a shared utility to reuse in both Feign and advice. Not blocking.src/main/java/com/example/spot/common/api/code/status/ErrorStatus.java (1)
204-206: Double semicolon after enum constants is unusual
;;compiles but is noise. Prefer a single semicolon for readability.Apply this diff:
- _STUDY_TODO_NULL(HttpStatus.BAD_REQUEST, "TODO4005", "투두 리스트 아이디가 입력되지 않았습니다."), - ;; + _STUDY_TODO_NULL(HttpStatus.BAD_REQUEST, "TODO4005", "투두 리스트 아이디가 입력되지 않았습니다."), + ;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
src/main/java/com/example/spot/common/api/code/status/ErrorStatus.java(3 hunks)src/main/java/com/example/spot/common/api/exception/ExceptionAdvice.java(8 hunks)src/main/java/com/example/spot/common/api/exception/base/ExternalApiException.java(1 hunks)src/main/java/com/example/spot/common/infrastructure/feign/SafeFeignExecutor.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/example/spot/common/api/exception/ExceptionAdvice.java (1)
src/main/java/com/example/spot/common/api/exception/base/ExternalApiException.java (1)
ExternalApiException(3-15)
🔇 Additional comments (6)
src/main/java/com/example/spot/common/api/exception/base/ExternalApiException.java (1)
12-13: Accessor LGTMGetter is straightforward and final field ensures immutability.
src/main/java/com/example/spot/common/api/code/status/ErrorStatus.java (2)
33-34: Good addition: dedicated external API error with 502 mapping
_EXTERNAL_API_ERRORaligns with the new advice and communicates intent clearly.
75-75: Whitespace-only tweakNo behavioral impact.
src/main/java/com/example/spot/common/api/exception/ExceptionAdvice.java (3)
5-5: Import of ExternalApiException is correct and scopedMatches the new handler usage.
25-27: Doc/format-only changesJavadoc spacing updates look fine; nothing to do.
Also applies to: 36-38, 49-51, 62-67
68-92: Constraint violation response shaping LGTMCollecting messages and returning 400 with a structured list is consistent with other handlers.
| @ExceptionHandler(ExternalApiException.class) | ||
| public ResponseEntity<ApiResponse<String>> handleExternalApiException(ExternalApiException exception) { | ||
| String errorMessage = String.format("외부 API 호출 실패: %s", exception.getMessage()); | ||
| log.error("ExternalApiException. error message: {}", errorMessage, exception); | ||
|
|
||
| captureException(exception); | ||
|
|
||
| ApiResponse<String> response = ApiResponse.onFailure( | ||
| ErrorStatus._EXTERNAL_API_ERROR.getCode(), | ||
| ErrorStatus._EXTERNAL_API_ERROR.getMessage(), | ||
| exception.getResponseBody() | ||
| ); | ||
|
|
||
| return new ResponseEntity<>(response, HttpStatus.BAD_GATEWAY); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Returning raw external response body to clients risks PII/secrets leakage
The handler currently returns exception.getResponseBody() directly. If the upstream body contains tokens, cookies, emails, or stack traces, they will be exposed to clients. Given the sanitizer already exists in SafeFeignExecutor, either ensure that executor stores a sanitized/truncated body (preferred; see executor comment), or sanitize here before responding.
Option A (preferred): rely on sanitized body from executor; then only ensure null-safety/truncation here:
- ApiResponse<String> response = ApiResponse.onFailure(
- ErrorStatus._EXTERNAL_API_ERROR.getCode(),
- ErrorStatus._EXTERNAL_API_ERROR.getMessage(),
- exception.getResponseBody()
- );
+ String body = exception.getResponseBody();
+ String safeBody = (body == null) ? "" : (body.length() > 2048 ? body.substring(0, 2048) : body);
+ ApiResponse<String> response = ApiResponse.onFailure(
+ ErrorStatus._EXTERNAL_API_ERROR.getCode(),
+ ErrorStatus._EXTERNAL_API_ERROR.getMessage(),
+ safeBody
+ );Option B (alternate): derive HTTP status from the enum for consistency:
- return new ResponseEntity<>(response, HttpStatus.BAD_GATEWAY);
+ return new ResponseEntity<>(response, ErrorStatus._EXTERNAL_API_ERROR.getHttpStatus());I can add unit tests for:
- SafeFeignExecutor: masks Authorization/Cookie and truncates long bodies.
- ExceptionAdvice: returns 502 and a sanitized payload.
Would you like me to push a test commit?
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @ExceptionHandler(ExternalApiException.class) | |
| public ResponseEntity<ApiResponse<String>> handleExternalApiException(ExternalApiException exception) { | |
| String errorMessage = String.format("외부 API 호출 실패: %s", exception.getMessage()); | |
| log.error("ExternalApiException. error message: {}", errorMessage, exception); | |
| captureException(exception); | |
| ApiResponse<String> response = ApiResponse.onFailure( | |
| ErrorStatus._EXTERNAL_API_ERROR.getCode(), | |
| ErrorStatus._EXTERNAL_API_ERROR.getMessage(), | |
| exception.getResponseBody() | |
| ); | |
| return new ResponseEntity<>(response, HttpStatus.BAD_GATEWAY); | |
| } | |
| @ExceptionHandler(ExternalApiException.class) | |
| public ResponseEntity<ApiResponse<String>> handleExternalApiException(ExternalApiException exception) { | |
| String errorMessage = String.format("외부 API 호출 실패: %s", exception.getMessage()); | |
| log.error("ExternalApiException. error message: {}", errorMessage, exception); | |
| captureException(exception); | |
| String body = exception.getResponseBody(); | |
| String safeBody = (body == null) | |
| ? "" | |
| : (body.length() > 2048 | |
| ? body.substring(0, 2048) | |
| : body); | |
| ApiResponse<String> response = ApiResponse.onFailure( | |
| ErrorStatus._EXTERNAL_API_ERROR.getCode(), | |
| ErrorStatus._EXTERNAL_API_ERROR.getMessage(), | |
| safeBody | |
| ); | |
| return new ResponseEntity<>(response, ErrorStatus._EXTERNAL_API_ERROR.getHttpStatus()); | |
| } |
| String body = e.contentUTF8(); | ||
| String masked = mask(message); | ||
| throw new ExternalApiException( | ||
| "Feign API 호출 실패(" + e.status() + "): " + masked, e | ||
| "Feign API 호출 실패(" + e.status() + "): " + masked, body, e | ||
| ); |
There was a problem hiding this comment.
Do not propagate raw external bodies to clients; sanitize + truncate first
You’re passing e.contentUTF8() straight into ExternalApiException, which the advice returns to clients. That can leak tokens/PII or huge payloads. Sanitize with the existing mask and cap length before propagating.
Apply this diff to sanitize and bound the body:
- String body = e.contentUTF8();
+ String body = e.contentUTF8();
+ String safeBody = mask(truncate(body, 2048)); // prevent PII leakage and huge payloads
String masked = mask(message);
throw new ExternalApiException(
- "Feign API 호출 실패(" + e.status() + "): " + masked, body, e
+ "Feign API 호출 실패(" + e.status() + "): " + masked, safeBody, e
);Add this helper in the class (outside the selected range):
private static String truncate(String s, int max) {
if (s == null) return "";
return s.length() <= max ? s : s.substring(0, max);
}🤖 Prompt for AI Agents
In
src/main/java/com/example/spot/common/infrastructure/feign/SafeFeignExecutor.java
around lines 17-21, the code currently passes e.contentUTF8() directly into
ExternalApiException; instead, sanitize and truncate the body before
propagating: add the provided private static truncate(String s, int max) helper
method somewhere in the class (outside the shown range), then replace the direct
body usage with a sanitized version that first masks the content (using the
existing mask method) and then truncates it to a safe max length (e.g., 1000
chars) and pass that masked+truncated string into ExternalApiException while
keeping the original exception as the cause.
1 participant
#️⃣ 연관된 이슈
🔎 작업 내용
ExternalApiException내resposeBody필드 추가ExternalApiException에러 핸들링을 통한 처리📷 스크린샷 (선택)
💬리뷰 요구사항 (선택)
Summary by CodeRabbit