This is an automated email from the ASF dual-hosted git repository. wujimin pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/servicecomb-java-chassis.git
The following commit(s) were added to refs/heads/master by this push: new d1983e5 [SCB-2158] optimize exception log d1983e5 is described below commit d1983e557605ada80172488553dfe6cd15b39147 Author: wujimin <wuji...@huawei.com> AuthorDate: Thu Dec 10 01:49:57 2020 +0800 [SCB-2158] optimize exception log --- .../servicecomb/core/exception/Exceptions.java | 8 ++++++++ .../converter/DefaultExceptionConverter.java | 4 ++-- .../core/filter/impl/ProducerOperationFilter.java | 20 ++------------------ .../core/provider/consumer/InvokerUtils.java | 9 ++++++++- .../provider/pojo/FilterInvocationCaller.java | 11 +++++++++++ 5 files changed, 31 insertions(+), 21 deletions(-) diff --git a/core/src/main/java/org/apache/servicecomb/core/exception/Exceptions.java b/core/src/main/java/org/apache/servicecomb/core/exception/Exceptions.java index fa6115f..51b6e8a 100644 --- a/core/src/main/java/org/apache/servicecomb/core/exception/Exceptions.java +++ b/core/src/main/java/org/apache/servicecomb/core/exception/Exceptions.java @@ -36,8 +36,12 @@ import org.apache.servicecomb.foundation.common.utils.SPIServiceUtils; import org.apache.servicecomb.swagger.invocation.Response; import org.apache.servicecomb.swagger.invocation.exception.ExceptionFactory; import org.apache.servicecomb.swagger.invocation.exception.InvocationException; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public final class Exceptions { + private static final Logger LOGGER = LoggerFactory.getLogger(Exceptions.class); + @SuppressWarnings("unchecked") private static final List<ExceptionConverter<Throwable>> CONVERTERS = SPIServiceUtils .getOrLoadSortedService(ExceptionConverter.class).stream() @@ -108,6 +112,10 @@ public final class Exceptions { public static Response exceptionToResponse(@Nullable Invocation invocation, Throwable exception, StatusType genericStatus) { InvocationException invocationException = Exceptions.convert(invocation, exception, genericStatus); + if (invocation != null) { + LOGGER.error("failed to process {} invocation, operation={}.", + invocation.getInvocationType(), invocation.getMicroserviceQualifiedName(), invocationException); + } return Response.status(invocationException.getStatus()) .entity(invocationException.getErrorData()); } diff --git a/core/src/main/java/org/apache/servicecomb/core/exception/converter/DefaultExceptionConverter.java b/core/src/main/java/org/apache/servicecomb/core/exception/converter/DefaultExceptionConverter.java index ef1777f..b48bdb4 100644 --- a/core/src/main/java/org/apache/servicecomb/core/exception/converter/DefaultExceptionConverter.java +++ b/core/src/main/java/org/apache/servicecomb/core/exception/converter/DefaultExceptionConverter.java @@ -54,13 +54,13 @@ public class DefaultExceptionConverter implements ExceptionConverter<Throwable> @Override public InvocationException convert(@Nullable Invocation invocation, Throwable throwable, StatusType genericStatus) { - LOGGER.error("convert unknown exception to InvocationException.", throwable); - String msg = throwable.getMessage(); if (msg == null) { msg = "Unexpected exception when processing."; } + LOGGER.error("convert unknown exception({}) to InvocationException, message={}.", + throwable.getClass().getName(), msg); return new InvocationException(genericStatus, ExceptionConverter.getGenericCode(genericStatus), msg, throwable); } diff --git a/core/src/main/java/org/apache/servicecomb/core/filter/impl/ProducerOperationFilter.java b/core/src/main/java/org/apache/servicecomb/core/filter/impl/ProducerOperationFilter.java index 6b58df4..1695ba8 100644 --- a/core/src/main/java/org/apache/servicecomb/core/filter/impl/ProducerOperationFilter.java +++ b/core/src/main/java/org/apache/servicecomb/core/filter/impl/ProducerOperationFilter.java @@ -30,7 +30,6 @@ import org.apache.servicecomb.foundation.common.utils.AsyncUtils; import org.apache.servicecomb.swagger.engine.SwaggerProducerOperation; import org.apache.servicecomb.swagger.invocation.Response; import org.apache.servicecomb.swagger.invocation.context.ContextUtils; -import org.apache.servicecomb.swagger.invocation.exception.InvocationException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -48,7 +47,7 @@ public class ProducerOperationFilter implements Filter { Object[] args = invocation.toProducerArguments(); return invoke(invocation, instance, method, args) .thenApply(result -> convertResultToResponse(invocation, producerOperation, result)) - .whenComplete((response, throwable) -> whenComplete(invocation, throwable)); + .whenComplete((response, throwable) -> processMetrics(invocation)); } @SuppressWarnings("unchecked") @@ -74,23 +73,8 @@ public class ProducerOperationFilter implements Filter { return producerOperation.getResponseMapper().mapResponse(invocation.getStatus(), result); } - protected void whenComplete(Invocation invocation, Throwable throwable) { - if (throwable != null) { - Throwable unwrapped = Exceptions.unwrap(throwable); - if (shouldPrintErrorLog(unwrapped)) { - LOGGER.error("unexpected error, invocation={},", invocation.getInvocationQualifiedName(), unwrapped); - } - } - + protected void processMetrics(Invocation invocation) { invocation.onBusinessMethodFinish(); invocation.onBusinessFinish(); } - - protected boolean shouldPrintErrorLog(Throwable throwable) { - if (throwable == null) { - return false; - } - - return !(throwable instanceof InvocationException); - } } diff --git a/core/src/main/java/org/apache/servicecomb/core/provider/consumer/InvokerUtils.java b/core/src/main/java/org/apache/servicecomb/core/provider/consumer/InvokerUtils.java index 5648f5f..b804f36 100644 --- a/core/src/main/java/org/apache/servicecomb/core/provider/consumer/InvokerUtils.java +++ b/core/src/main/java/org/apache/servicecomb/core/provider/consumer/InvokerUtils.java @@ -233,7 +233,14 @@ public final class InvokerUtils { } private static Response convertException(Invocation invocation, Throwable throwable) { - throw Exceptions.convert(invocation, throwable); + InvocationException invocationException = Exceptions.convert(invocation, throwable); + + LOGGER.error("failed to invoke {}, endpoint={}.", + invocation.getMicroserviceQualifiedName(), + invocation.getEndpoint(), + invocationException); + + throw invocationException; } private static void processMetrics(Invocation invocation, Response response) { diff --git a/providers/provider-pojo/src/main/java/org/apache/servicecomb/provider/pojo/FilterInvocationCaller.java b/providers/provider-pojo/src/main/java/org/apache/servicecomb/provider/pojo/FilterInvocationCaller.java index ece166e..ecb4358 100644 --- a/providers/provider-pojo/src/main/java/org/apache/servicecomb/provider/pojo/FilterInvocationCaller.java +++ b/providers/provider-pojo/src/main/java/org/apache/servicecomb/provider/pojo/FilterInvocationCaller.java @@ -23,16 +23,22 @@ import java.util.concurrent.CompletableFuture; import javax.annotation.Nonnull; +import org.apache.servicecomb.core.exception.Exceptions; import org.apache.servicecomb.core.provider.consumer.InvokerUtils; import org.apache.servicecomb.foundation.common.utils.AsyncUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; public class FilterInvocationCaller implements InvocationCaller { + private static final Logger LOGGER = LoggerFactory.getLogger(FilterInvocationCaller.class); + // if not a sync method, should never throw exception directly @Override public Object call(Method method, PojoConsumerMetaRefresher metaRefresher, PojoInvocationCreator invocationCreator, Object[] args) { CompletableFuture<Object> future = AsyncUtils .tryCatchSupplier(() -> invocationCreator.create(method, metaRefresher, args)) + .exceptionally(throwable -> logCreateInvocationException(method, throwable)) .thenCompose(this::doCall); return isAsyncMethod(method) ? future : AsyncUtils.toSync(future); @@ -42,4 +48,9 @@ public class FilterInvocationCaller implements InvocationCaller { return InvokerUtils.invoke(invocation) .thenApply(invocation::convertResponse); } + + protected PojoInvocation logCreateInvocationException(Method method, Throwable throwable) { + LOGGER.error("failed to create invocation, method=", method); + throw Exceptions.consumer("SCB_PROVIDER_POJO.400000001", "failed to create invocation.", throwable); + } }