diff --git a/src/main/java/ssu/eatssu/global/log/ControllerLogAspect.java b/src/main/java/ssu/eatssu/global/log/ControllerLogAspect.java index 9b6f782..59ff50c 100644 --- a/src/main/java/ssu/eatssu/global/log/ControllerLogAspect.java +++ b/src/main/java/ssu/eatssu/global/log/ControllerLogAspect.java @@ -98,10 +98,11 @@ public Object logApi(ProceedingJoinPoint joinPoint) throws Throwable { long time = System.currentTimeMillis() - start; String causeMessage = getCauseMessage(e); String exceptionType = e.getClass().getSimpleName(); - log.error("EXCEPTION {} {} ({} ms) type={} cause={}", method, uri, time, exceptionType, causeMessage, e); - - if (shouldNotifySlack(e)) { + if (isServerError(e)) { + log.error("EXCEPTION {} {} ({} ms) type={} cause={}", method, uri, time, exceptionType, causeMessage, e); slackErrorNotifier.notify(e, method, uri, userId, argsJson); + } else { + log.warn("EXCEPTION {} {} ({} ms) type={} cause={}", method, uri, time, exceptionType, causeMessage); } throw e; } @@ -127,7 +128,7 @@ private boolean isAuthApi(String uri) { return uri != null && uri.startsWith("/oauths/"); } - private boolean shouldNotifySlack(Throwable e) { + private boolean isServerError(Throwable e) { if (e instanceof BaseException baseException) { return BaseResponseStatus.sendSlackNotification(baseException.getStatus()); } diff --git a/src/test/java/ssu/eatssu/global/log/ControllerLogAspectTest.java b/src/test/java/ssu/eatssu/global/log/ControllerLogAspectTest.java index be0606f..91bd826 100644 --- a/src/test/java/ssu/eatssu/global/log/ControllerLogAspectTest.java +++ b/src/test/java/ssu/eatssu/global/log/ControllerLogAspectTest.java @@ -1,10 +1,15 @@ package ssu.eatssu.global.log; +import ch.qos.logback.classic.Level; +import ch.qos.logback.classic.Logger; +import ch.qos.logback.classic.spi.ILoggingEvent; +import ch.qos.logback.core.read.ListAppender; import com.fasterxml.jackson.databind.ObjectMapper; import org.aspectj.lang.ProceedingJoinPoint; import org.aspectj.lang.reflect.MethodSignature; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.Test; +import org.slf4j.LoggerFactory; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.security.authentication.UsernamePasswordAuthenticationToken; import org.springframework.security.core.Authentication; @@ -144,6 +149,65 @@ void doesNotNotifySlackFor4xxBusinessException() throws Throwable { verify(notifier, org.mockito.Mockito.never()).notify(any(), anyString(), anyString(), anyString(), anyString()); } + @Test + void logs4xxBusinessExceptionAsWarnWithoutStackTrace() throws Throwable { + // given + ListAppender appender = attachListAppender(); + ControllerLogAspect aspect = new ControllerLogAspect(new ObjectMapper(), mock(SlackErrorNotifier.class)); + ProceedingJoinPoint joinPoint = joinPointThrowing(new BaseException(BaseResponseStatus.NOT_FOUND_MENU)); + + // when + assertThatThrownBy(() -> aspect.logApi(joinPoint)).isInstanceOf(BaseException.class); + + // then + ILoggingEvent event = findExceptionLog(appender); + assertThat(event.getLevel()).isEqualTo(Level.WARN); + assertThat(event.getThrowableProxy()).isNull(); + } + + @Test + void logs5xxExceptionAsErrorWithStackTrace() throws Throwable { + // given + ListAppender appender = attachListAppender(); + ControllerLogAspect aspect = new ControllerLogAspect(new ObjectMapper(), mock(SlackErrorNotifier.class)); + ProceedingJoinPoint joinPoint = joinPointThrowing(new BaseException(BaseResponseStatus.INTERNAL_SERVER_ERROR)); + + // when + assertThatThrownBy(() -> aspect.logApi(joinPoint)).isInstanceOf(BaseException.class); + + // then + ILoggingEvent event = findExceptionLog(appender); + assertThat(event.getLevel()).isEqualTo(Level.ERROR); + assertThat(event.getThrowableProxy()).isNotNull(); + } + + private ListAppender attachListAppender() { + Logger logger = (Logger) LoggerFactory.getLogger(ControllerLogAspect.class); + ListAppender appender = new ListAppender<>(); + appender.start(); + logger.addAppender(appender); + return appender; + } + + private ILoggingEvent findExceptionLog(ListAppender appender) { + ((Logger) LoggerFactory.getLogger(ControllerLogAspect.class)).detachAppender(appender); + return appender.list.stream() + .filter(event -> event.getFormattedMessage().startsWith("EXCEPTION")) + .findFirst() + .orElseThrow(); + } + + private ProceedingJoinPoint joinPointThrowing(Throwable throwable) throws Throwable { + ProceedingJoinPoint joinPoint = mock(ProceedingJoinPoint.class); + MethodSignature signature = mock(MethodSignature.class); + RequestContextHolder.setRequestAttributes(new ServletRequestAttributes(new MockHttpServletRequest("GET", "/menus"))); + when(joinPoint.getSignature()).thenReturn(signature); + when(signature.getParameterNames()).thenReturn(new String[0]); + when(joinPoint.getArgs()).thenReturn(new Object[0]); + when(joinPoint.proceed()).thenThrow(throwable); + return joinPoint; + } + @Test void 제외_인자와_null_인자와_긴_인자를_안전하게_로그한다() throws Throwable { ProceedingJoinPoint joinPoint = mock(ProceedingJoinPoint.class);