faet: dev log stacktrace 추가 - #159 #161
Merged
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
📝 WalkthroughSummary by CodeRabbit릴리스 노트
요약티켓 확인 API 엔드포인트를 변경 사항
Sequence Diagram(s)sequenceDiagram
participant Client
participant Filter as DevRequestResponseLoggingFilter
participant Chain as FilterChain
participant Logger as LogJsonBuilder
Client->>Filter: HTTP Request
activate Filter
alt Exception Occurs
Filter->>Chain: doFilter()
Chain-->>Filter: throws Exception
Filter->>Filter: catch & log exception
Note over Filter: Set response status to 500
Note over Filter: Capture exception details
else Normal Flow
Filter->>Chain: doFilter()
Chain-->>Filter: response
end
Filter->>Logger: buildJsonLog(..., exception)
activate Logger
alt Exception != null
Logger->>Logger: append FIELD_EXCEPTION
Logger->>Logger: append FIELD_STACKTRACE
end
Logger-->>Filter: JSON log
deactivate Logger
Filter->>Filter: finally: copy response body
Filter-->>Client: HTTP Response
deactivate Filter
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes 합리적인 변경 규모의 혼합: 엔드포인트 경로 변경(단순), 로깅 시스템 개선(중간 복잡도의 예외 처리 로직 추가), 코멘트 정리(보조적) 관련 가능성 있는 PR
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (3)
src/main/java/com/permitseoul/permitserver/global/config/SecurityConfig.java (1)
40-41: 화이트리스트 경로 변경 일치 — LGTM. 추가 보안 검토 권장컨트롤러의
/door/staff/confirm경로와 일치합니다. 다만 permitAll 공개 엔드포인트로 열면 코드 대입 시도가 쉬워집니다. 최소한의 서버측 rate‑limit/슬리핑, 시도 횟수 제한, IP/키 기반 제한 등을 검토해 주세요. 클라이언트/게이트웨이 설정(리버스 프록시 캐시·WAF 규칙)도 함께 업데이트 되었는지 확인 바랍니다.src/main/java/com/permitseoul/permitserver/global/util/HttpReqResLogJsonBuilder.java (1)
72-83: 하드코딩 상수 사용과 원인 체인 미포함
- 1500, "...more" 하드코딩 대신 상수(MAX_LENGTH, OVER_MAX_LENGTH) 사용을 권장합니다.
- 현재는 최상위 스택만 포함합니다.
Throwable#getCause()체인을 포함하면 디버깅 품질이 올라갑니다.패치 제안:
- private static String getStackTraceAsString(final Exception e) { - final StringBuilder sb = new StringBuilder(); - for (StackTraceElement el : e.getStackTrace()) { - sb.append(el.toString()).append("\n"); - if (sb.length() > 1500) { // Discord 메시지 길이 제한 방지 - sb.append("...more"); - break; - } - } - return sb.toString(); - } + private static String getStackTraceAsString(final Exception e) { + final StringBuilder sb = new StringBuilder(); + Throwable cur = e; + while (cur != null) { + sb.append(cur.getClass().getSimpleName()) + .append(": ") + .append(cur.getMessage() == null ? "" : cur.getMessage()) + .append("\n"); + for (StackTraceElement el : cur.getStackTrace()) { + sb.append(" at ").append(el).append("\n"); + if (sb.length() > MAX_LENGTH) { + sb.append(OVER_MAX_LENGTH); + return sb.toString(); + } + } + cur = cur.getCause(); + if (cur != null) sb.append("Caused by: "); + } + return sb.toString(); + }src/main/java/com/permitseoul/permitserver/domain/ticket/api/controller/TicketController.java (1)
39-46: 경로 변경 일치 — LGTM. 메서드 이름 정리 제안
/door/staff/confirm로의 변경은 SecurityConfig 화이트리스트와 일치합니다.- 메서드명이
getUserBuyTicketInfo로 중복/오해의 소지가 있습니다.confirmTicket등으로 변경을 권장합니다.- 해당 엔드포인트가 인증 없이 공개(permitAll)되는 만큼 체크코드 브루트포스 방지(속도 제한/슬리핑, 시도 횟수 제한)도 함께 고려해 주세요. 클라이언트/문서 업데이트 여부 확인 부탁드립니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/permitseoul/permitserver/domain/ticket/api/controller/TicketController.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/ticket/core/component/TicketGenerator.java(2 hunks)src/main/java/com/permitseoul/permitserver/global/config/SecurityConfig.java(1 hunks)src/main/java/com/permitseoul/permitserver/global/filter/DevRequestResponseLoggingFilter.java(1 hunks)src/main/java/com/permitseoul/permitserver/global/util/HttpReqResLogJsonBuilder.java(3 hunks)
🔇 Additional comments (1)
src/main/java/com/permitseoul/permitserver/global/filter/DevRequestResponseLoggingFilter.java (1)
25-26: HEALTH_CHECK_URL 상수화 — 좋습니다헬스체크 필터링 가독성이 좋아졌습니다.
Comment on lines
+41
to
43
| ? reservation.getTotalAmount() | ||
| : ticketTypeEntity.getTicketPrice() | ||
| ) |
There was a problem hiding this comment.
🧩 Analysis chain
쿠폰 사용 시 각 티켓 가격을 예약 총액으로 설정 — 과금 오류 위험
hasCoupon일 때 개별 티켓의 ticketPrice가 reservation.getTotalAmount()(예약 전체 금액)로 설정됩니다. 여러 장 생성 시 각 티켓이 총액을 개별적으로 갖게 되어 과금/정산 오류가 발생할 수 있습니다.
빠른 안전조치(권장):
- hasCoupon
- ? reservation.getTotalAmount()
- : ticketTypeEntity.getTicketPrice()
+ ticketTypeEntity.getTicketPrice()정확한 할인 배분이 필요하면, 전체 원가 합계 대비 reservation.getTotalAmount() 비율로 티켓 단가를 비례 배분(반올림 규칙 포함)하는 로직을 별도 단계에서 계산해 주입하는 방식을 제안드립니다. 비즈니스 룰 확인 부탁드립니다.
🏁 Script executed:
#!/bin/bash
# Find and display TicketGenerator.java with context around lines 41-43
fd 'TicketGenerator.java' --type f -x cat {}Length of output: 2618
🏁 Script executed:
#!/bin/bash
# Search for ticketPrice usage patterns to understand how it's used
rg -n 'ticketPrice' --type java -C 5Length of output: 18145
🏁 Script executed:
#!/bin/bash
# Search for coupon/discount related logic
rg -n -i 'hasCoupon|coupon|discount' --type java -C 3 | head -100Length of output: 13848
🏁 Script executed:
#!/bin/bash
# Check how many tickets are created per reservation
rg -n 'new.*Ticket|createTicket|for.*ticket' --type java -C 2 | head -80Length of output: 10511
🏁 Script executed:
#!/bin/bash
# Look for getTotalAmount usage
rg -n 'getTotalAmount' --type java -C 3Length of output: 7105
🏁 Script executed:
#!/bin/bash
# Search for getTicketPrice usage
rg -n 'getTicketPrice' --type java -C 3Length of output: 8937
쿠폰 적용 시 각 티켓 가격이 예약 총액으로 설정됨 — 중복 청구 위험 (치명적)
hasCoupon일 때 IntStream.range(0, reservationTicket.getCount())를 통해 생성되는 각 티켓이 reservation.getTotalAmount()(예약 전체 금액)로 설정됩니다. 별도 검증으로 쿠폰 구매는 티켓 1장으로 제한되어 있지만, 제너레이터가 이를 강제하지 않아 다음과 같은 위험이 있습니다:
- 검증 우회/실패 시 여러 장이 생성되면 각 티켓이 전체 금액을 개별 보유
- 예: 3장 구매, 쿠폰 적용, 총액 30,000원 → 각 티켓 ticketPrice = 30,000원 → DB 합계 = 90,000원 (300% 과금)
ticketPrice는 DB 저장 및 정산 쿼리(sumTicketPriceByTicketTypeIdAndStatuses)에 사용되므로 회계 데이터 오염
권장 조치:
검증 실패 시에도 안전하도록 제너레이터 내부에 방어 로직 추가:
ticketPrice(
- hasCoupon
- ? reservation.getTotalAmount()
- : ticketTypeEntity.getTicketPrice()
+ ticketTypeEntity.getTicketPrice() // 기본값 사용
)또는 쿠폰 할인을 별도 단계에서 명시적으로 처리(금액 비례 배분, 반올림 규칙 포함)하고 검증 계층에서만 쿠폰 복수 구매 방지.
📝 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.
Suggested change
| ? reservation.getTotalAmount() | |
| : ticketTypeEntity.getTicketPrice() | |
| ) | |
| ticketTypeEntity.getTicketPrice() | |
| ) |
🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/domain/ticket/core/component/TicketGenerator.java
around lines 41-43, the generator sets each ticket's price to
reservation.getTotalAmount() when hasCoupon is true, causing overcharging if
multiple tickets are created; add defensive logic so that when a coupon is
applied you do NOT assign the entire reservation total to every ticket: either
enforce a single-ticket rule inside the generator (treat count>1 as count=1 for
coupon price), or compute a per-ticket share = reservation.getTotalAmount() /
actualCount with a deterministic rounding strategy (e.g., floor for all and add
remainder to first ticket) and assign ticketPrice per ticket accordingly;
implement the check and per-ticket pricing calculation in this method so even if
upstream validation fails, DB-stored ticketPrice values remain correct and
accounting queries stay accurate.
Comment on lines
+38
to
+57
| } catch (Exception e) { | ||
| exception = e; | ||
| log.error("🚨 [DevRequestResponseLoggingFilter] 요청 중 예외 발생: {}", e.getMessage(), e); | ||
| res.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR); | ||
| } | ||
| finally { | ||
| final long duration = System.currentTimeMillis() - start; | ||
| final String uri = req.getRequestURI(); | ||
| if (!uri.startsWith("/actuator/health")) { | ||
| final String jsonLog = HttpReqResLogJsonBuilder.buildJsonLog(req, res, duration); | ||
| if (!uri.startsWith(HEALTH_CHECK_URL)) { | ||
| final String jsonLog = HttpReqResLogJsonBuilder.buildJsonLog(req, res, duration, exception); | ||
| discordSender.send(jsonLog); | ||
| } | ||
| res.copyBodyToResponse(); | ||
| } | ||
|
|
||
| if (exception != null) { | ||
| if (exception instanceof ServletException se) throw se; | ||
| if (exception instanceof IOException ioe) throw ioe; | ||
| throw new ServletException(exception); | ||
| } |
There was a problem hiding this comment.
예외 재던지기(rethrow) 전에 응답을 커밋함 → double‑commit 위험
catch에서 상태코드 500 설정 후 finally에서 copyBodyToResponse()로 응답을 커밋하고, 그 다음 예외를 다시 던지면 DispatcherServlet/에러 처리기가 추가로 에러 바디를 쓰지 못해 경고가 나거나 동작이 꼬일 수 있습니다. 예외 발생 시에는 응답 복사/커밋을 건너뛰고, 상태코드 설정도 이 필터가 아닌 예외 처리기에게 맡기는 편이 안전합니다.
권장 패치:
- } catch (Exception e) {
+ } catch (Exception e) {
exception = e;
log.error("🚨 [DevRequestResponseLoggingFilter] 요청 중 예외 발생: {}", e.getMessage(), e);
- res.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
- }
- finally {
+ }
+ finally {
final long duration = System.currentTimeMillis() - start;
final String uri = req.getRequestURI();
if (!uri.startsWith(HEALTH_CHECK_URL)) {
final String jsonLog = HttpReqResLogJsonBuilder.buildJsonLog(req, res, duration, exception);
- discordSender.send(jsonLog);
+ try {
+ discordSender.send(jsonLog);
+ } catch (Exception sendEx) {
+ log.warn("Discord 전송 실패: {}", sendEx.toString());
+ }
}
- res.copyBodyToResponse();
+ if (exception == null) {
+ res.copyBodyToResponse();
+ }
}
if (exception != null) {
if (exception instanceof ServletException se) throw se;
if (exception instanceof IOException ioe) throw ioe;
throw new ServletException(exception);
}🤖 Prompt for AI Agents
In
src/main/java/com/permitseoul/permitserver/global/filter/DevRequestResponseLoggingFilter.java
around lines 38-57, the filter currently sets the response status in the catch
block and always calls res.copyBodyToResponse() in finally, which can commit the
response before the exception is rethrown; remove the
res.setStatus(HttpServletResponse.SC_INTERNAL_SERVER_ERROR) from the catch (keep
capturing the exception and logging), and change the finally block so that when
exception != null you do NOT call res.copyBodyToResponse() (and avoid committing
the response); only build/send the JSON log and copy the buffered body to the
actual response when exception == null (if you still want to notify Discord on
exceptions, send a log without committing the response), then after finally
rethrow the captured exception as currently implemented.
Comment on lines
+53
to
+57
|
|
||
| if (exception != null) { | ||
| logMap.put(FIELD_EXCEPTION, exception.getClass().getSimpleName() + ": " + exception.getMessage()); | ||
| logMap.put(FIELD_STACKTRACE, getStackTraceAsString(exception)); | ||
| } |
There was a problem hiding this comment.
예외/스택트레이스 로그 추가는 좋습니다. 다만 민감정보 마스킹이 필요합니다.
요청/응답 바디, 예외 메시지에 비밀번호·토큰·이메일 등 PII/시크릿이 포함될 수 있습니다. Discord로 외부 전송되는 만큼 기본 마스킹을 추가해 주세요(키 기반 마스킹: password, pass, pwd, token, accessToken, refreshToken, authorization, email, phone 등).
예시(핵심 아이디어):
- JSON이면 ObjectMapper로 파싱 → 키 매칭 시 값 마스킹 → 문자열화
- 비‑JSON은 정규식 기반 최소 마스킹
원하는 경우 보조 헬퍼 redactSensitive(String)/redactSensitive(Map<String,Object>)를 만들어 FIELD_REQUEST_BODY/FIELD_RESPONSE_BODY/FIELD_EXCEPTION 적용이 필요합니다.
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.
🔥Pull requests
⛳️ 작업한 브랜치
👷 작업한 내용
🚨 참고 사항