Skip to content

Edge's 490 exception is thrown and can be used for business processing#2154

Closed
liuguangrong wants to merge 4 commits intoapache:masterfrom
liuguangrong:master
Closed

Edge's 490 exception is thrown and can be used for business processing#2154
liuguangrong wants to merge 4 commits intoapache:masterfrom
liuguangrong:master

Conversation

@liuguangrong
Copy link
Contributor

@liuguangrong liuguangrong commented Dec 21, 2020

Follow this checklist to help us incorporate your contribution quickly and easily:

  • Make sure there is a JIRA issue filed for the change (usually before you start working on it). Trivial changes like typos do not require a JIRA issue. Your pull request should address just this issue, without pulling in other changes.
  • Each commit in the pull request should have a meaningful subject line and body.
  • Format the pull request title like [SCB-XXX] Fixes bug in ApproximateQuantiles, where you replace SCB-XXX with the appropriate JIRA issue.
  • Write a pull request description that is detailed enough to understand what the pull request does, how, and why.
  • Run mvn clean install -Pit to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
  • If this contribution is large, please file an Apache Individual Contributor License Agreement.

@codecov-io
Copy link

Codecov Report

Merging #2154 (19b5f7f) into master (323d5b7) will decrease coverage by 0.01%.
The diff coverage is 60.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #2154      +/-   ##
============================================
- Coverage     80.76%   80.75%   -0.02%     
  Complexity     1339     1339              
============================================
  Files          1463     1463              
  Lines         40142    40147       +5     
  Branches       3411     3411              
============================================
- Hits          32420    32419       -1     
- Misses         6255     6261       +6     
  Partials       1467     1467              
Impacted Files Coverage Δ Complexity Δ
...ansport/rest/client/http/RestClientInvocation.java 93.54% <60.00%> (-1.12%) 0.00 <0.00> (ø)
...pache/servicecomb/config/kie/client/KieClient.java 72.61% <0.00%> (-5.96%) 0.00% <0.00%> (ø%)
.../servicecomb/config/client/ConfigCenterClient.java 52.11% <0.00%> (-0.43%) 0.00% <0.00%> (ø%)
.../servicecomb/registry/discovery/DiscoveryTree.java 100.00% <0.00%> (+3.50%) 0.00% <0.00%> (ø%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 323d5b7...19b5f7f. Read the comment docs.

.error(LOGGER, "Failed to send request, alreadyFailed:{}, local:{}, remote:{}, message={}.",
alreadyFailed, getLocalAddress(), ipPort.getSocketAddress(),
ExceptionUtils.getExceptionMessageWithoutTrace(e));
httpRestClientInvocationHooks.forEach(httpServerExceptionHandler -> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you can add a custom handler to fillful your requirements.

e.g. 'MyHandler implements Handler'

servicecomb.handler.chain.Consumer.defaut: your_current_handlers,myhandler

myhandler can process the exception thrown in RestClientInvocation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://docs.servicecomb.io/java-chassis/zh_CN/references-handlers/intruduction/

I found it didn't meet my requirements through experiments.

My handler executes before the exception is returned, and does not return to handler after the exception is thrown.

I can't get this exception message.

Is there any other way

Copy link
Contributor

@liubao68 liubao68 Dec 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public class MyHandler implements Handler {

  private static final Logger LOGGER = LoggerFactory.getLogger(MyHandler.class);

  @Override
  public void handle(Invocation invocation, AsyncResponse asyncResp) throws Exception {
    LOGGER.info("before");

    invocation.next(response -> {
         LOGGER.info("after");
    });
  }
}

You can find many Handler implementations in servicecomb-java-chassis project.

@Volcannozzz
Copy link
Contributor

I think the handler example given by @liubao68 cannot meet @liuguangrong 's needs. Because the transportHandler is the last handler in the source code.

public class ConsumerHandlerManager extends AbstractHandlerManager {
  @Override
  protected String getName() {
    return "Consumer";
  }

  @Override
  protected String getInnerDefaultChainDef() {
    return "simpleLB";
  }

  @Override
  protected Handler getLastHandler() {
    return TransportClientHandler.INSTANCE;
  }
}

and in AbstractHandlerManager.java

  private List<Handler> createHandlerChain(String chainDef) {
    List<Class<Handler>> chainClasses = convertToChainClass(chainDef);

    List<Handler> handlerList = new ArrayList<>();
    for (Class<Handler> cls : chainClasses) {
      try {
        handlerList.add(cls.newInstance());
      } catch (Exception e) {
        // 在启动阶段直接抛异常出来
        throw new Error(e);
      }
    }
    handlerList.add(getLastHandler());
    return handlerList;
  }

In addition, when the transportHandler calls an exception, it will return the exception information to the caller. Can the exception information be handled by the caller? Hope you provide the calling way and error message stack.

@liubao68
Copy link
Contributor

liubao68 commented Feb 7, 2021

This handler throw exception , will hanldled in code block after in my example code

@Volcannozzz
Copy link
Contributor

Volcannozzz commented Mar 2, 2021

Sorry, maybe we can got the answer from line 86, 125, 544 in RestClientInvocation.java.
so:

  InvokerUtils.reactiveInvoke("pojo", "InvokerEndpoint", "model", args, ClientModel.class, response -> {
    if (response.isFailed()) {
      // do something
    }
  });

Is this okay?
@liubao68 @liuguangrong

@liubao68 liubao68 closed this Oct 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants