You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by li...@apache.org on 2021/03/30 06:45:05 UTC

[servicecomb-java-chassis] branch master updated: [SCB-2247] consumer invocation with filter throw NPE when http status is not 2xx (#2331)

This is an automated email from the ASF dual-hosted git repository.

liubao 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 45be294  [SCB-2247] consumer invocation with filter throw NPE when http status is not 2xx (#2331)
45be294 is described below

commit 45be2941689bb354d9bfe372ebacfd286fbe2f4f
Author: wujimin <wu...@huawei.com>
AuthorDate: Tue Mar 30 14:44:54 2021 +0800

    [SCB-2247] consumer invocation with filter throw NPE when http status is not 2xx (#2331)
---
 .../invocation/InvocationTimeoutBootListener.java  | 14 ++-------
 .../core/provider/consumer/InvokerUtils.java       | 36 +++++++++++++---------
 2 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/core/src/main/java/org/apache/servicecomb/core/invocation/InvocationTimeoutBootListener.java b/core/src/main/java/org/apache/servicecomb/core/invocation/InvocationTimeoutBootListener.java
index 41715ec..86fed19 100644
--- a/core/src/main/java/org/apache/servicecomb/core/invocation/InvocationTimeoutBootListener.java
+++ b/core/src/main/java/org/apache/servicecomb/core/invocation/InvocationTimeoutBootListener.java
@@ -25,7 +25,6 @@ import org.apache.servicecomb.core.Const;
 import org.apache.servicecomb.core.Invocation;
 import org.apache.servicecomb.core.event.InvocationBusinessFinishEvent;
 import org.apache.servicecomb.core.event.InvocationBusinessMethodStartEvent;
-import org.apache.servicecomb.core.event.InvocationFinishEvent;
 import org.apache.servicecomb.core.event.InvocationHandlersStartEvent;
 import org.apache.servicecomb.core.event.InvocationRunInExecutorStartEvent;
 import org.apache.servicecomb.core.event.InvocationStartEvent;
@@ -42,6 +41,7 @@ import org.springframework.stereotype.Component;
 import com.google.common.eventbus.Subscribe;
 import com.netflix.config.DynamicPropertyFactory;
 
+@SuppressWarnings({"UnstableApiUsage", "unused"})
 @Component
 public class InvocationTimeoutBootListener implements BootListener {
   private static final Logger LOGGER = LoggerFactory.getLogger(InvocationTimeoutBootListener.class);
@@ -49,8 +49,7 @@ public class InvocationTimeoutBootListener implements BootListener {
   public static final String ENABLE_TIMEOUT_CHECK = "servicecomb.invocation.enableTimeoutCheck";
 
   public static boolean timeoutCheckEnabled() {
-    return DynamicPropertyFactory.getInstance().getBooleanProperty
-        (ENABLE_TIMEOUT_CHECK, true).get();
+    return DynamicPropertyFactory.getInstance().getBooleanProperty(ENABLE_TIMEOUT_CHECK, true).get();
   }
 
   @Override
@@ -109,8 +108,7 @@ public class InvocationTimeoutBootListener implements BootListener {
         return;
       }
       invocation.addLocalContext(Const.CONTEXT_TIMED_OUT, true);
-      throw new InvocationException(REQUEST_TIMEOUT,
-          ExceptionCodes.INVOCATION_TIMEOUT, "Invocation Timeout.");
+      throw new InvocationException(REQUEST_TIMEOUT, ExceptionCodes.INVOCATION_TIMEOUT, "Invocation Timeout.");
     }
   }
 
@@ -150,10 +148,4 @@ public class InvocationTimeoutBootListener implements BootListener {
     ensureInvocationNotTimeout(invocation);
     invocation.addContext(Const.CONTEXT_TIME_ELAPSED, Long.toString(calculateElapsedTime(invocation)));
   }
-
-  @Subscribe
-  @EnableExceptionPropagation
-  public void onInvocationFinishEvent(InvocationFinishEvent event) {
-    ensureInvocationNotTimeout(event.getInvocation());
-  }
 }
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 09b4337..790457a 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
@@ -33,6 +33,7 @@ import org.apache.servicecomb.core.definition.OperationMeta;
 import org.apache.servicecomb.core.definition.SchemaMeta;
 import org.apache.servicecomb.core.exception.Exceptions;
 import org.apache.servicecomb.core.invocation.InvocationFactory;
+import org.apache.servicecomb.foundation.common.utils.AsyncUtils;
 import org.apache.servicecomb.foundation.common.utils.ExceptionUtils;
 import org.apache.servicecomb.swagger.invocation.AsyncResponse;
 import org.apache.servicecomb.swagger.invocation.Response;
@@ -228,21 +229,31 @@ public final class InvokerUtils {
     invocation.onStartHandlersRequest();
     return invocation.getMicroserviceMeta().getFilterChain()
         .onFilter(invocation)
-        .exceptionally(throwable -> convertException(invocation, throwable))
-        .whenComplete((response, throwable) -> finishInvocation(invocation, response, throwable));
+        .exceptionally(throwable -> convertExceptionToResponse(invocation, throwable))
+        .whenComplete((response, throwable) -> finishInvocation(invocation, response));
   }
 
-  private static Response convertException(Invocation invocation, Throwable throwable) {
-    throw Exceptions.convert(invocation, throwable);
+  private static Response convertExceptionToResponse(Invocation invocation, Throwable throwable) {
+    InvocationException exception = Exceptions.convert(invocation, throwable);
+    return Response.failResp(exception);
   }
 
-  private static void finishInvocation(Invocation invocation, Response response, Throwable throwable) {
+  private static void finishInvocation(Invocation invocation, Response response) {
     processMetrics(invocation, response);
-    tryLogException(invocation, throwable);
+    tryLogException(invocation, response);
+
+    if (response.isFailed()) {
+      AsyncUtils.rethrow(response.getResult());
+    }
+  }
+
+  private static void processMetrics(Invocation invocation, Response response) {
+    invocation.getInvocationStageTrace().finishHandlersResponse();
+    invocation.onFinish(response);
   }
 
-  private static void tryLogException(Invocation invocation, Throwable throwable) {
-    if (throwable == null) {
+  private static void tryLogException(Invocation invocation, Response response) {
+    if (response.isSucceed()) {
       return;
     }
 
@@ -250,18 +261,13 @@ public final class InvokerUtils {
       LOGGER.error("failed to invoke {}, endpoint={}.",
           invocation.getMicroserviceQualifiedName(),
           invocation.getEndpoint(),
-          throwable);
+          response.<Throwable>getResult());
       return;
     }
 
     LOGGER.error("failed to invoke {}, endpoint={}, message={}.",
         invocation.getMicroserviceQualifiedName(),
         invocation.getEndpoint(),
-        ExceptionUtils.getExceptionMessageWithoutTrace(throwable));
-  }
-
-  private static void processMetrics(Invocation invocation, Response response) {
-    invocation.getInvocationStageTrace().finishHandlersResponse();
-    invocation.onFinish(response);
+        ExceptionUtils.getExceptionMessageWithoutTrace(response.getResult()));
   }
 }