finos / spring-bot

Spring Boot + Java Integration for Symphony/Teams Chat Platform Bots and Apps
https://springbot.finos.org
Apache License 2.0
60 stars 35 forks source link

Handle exception for Teams #408

Closed vaibhav-db closed 1 year ago

vaibhav-db commented 1 year ago

Handle exception for Teams

For Teams if channel id is wrong or bot is not added in channel/group and we try to send message, framework throw exception in completedFuture.

As we can't capture the exception, so we can't take action on the exception.

Exception capture in ChatWorkflowErrorHandler.handleError method. Even if add our own ErrorHandler implementation class, we only get error message. We need all config information.

Action.CURRENT_ACTION.get() is null, as micorsoft create the separate thread to send message, so ThreadLocal will not used.

Potential Solutions:

Create our own ErrorHandler, which will have all information like channel id, user id, Map<String, Object> data. and in custom ErrorHandler, capture all the information and sent response to callback url.

vaibhav-db commented 1 year ago

Hi @robmoffat ,

As discussed I have added Action in TeamsResponseHandler.handleErrorAndStorage method. Found one issue: When we pass wrong channel id and MS BDK throw exception and we set Action and flow goes through ChatWorkflowErrorHandler and as Action is set, it try to send error adaptive card to wrong channel and flows goes on infinite.


private BiFunction handleErrorAndStorage(Object out, TeamsAddressable address, Map data, Response t) {
        return (rr, e) -> {
                if (e != null) {                    
                    LOG.error("Error message for stream id {} , message: {} ", address.getKey() , e.getMessage());
                    if (out instanceof ObjectNode){
                        try {
                            LOG.error("json:\n"+new ObjectMapper().writeValueAsString(out));
                        } catch (JsonProcessingException e1) {
                        }
                    } else {
                        LOG.error("message:\n"+out);                        
                    } 
                    ErrorAction sma = new ErrorAction(address, data);
                    Action.CURRENT_ACTION.set(sma); 

                    initErrorHandler();
                    eh.handleError(e);                  
                } else if(rr != null) {
                    performStorage(address, data, teamsState);
                }

                return null;
            };
    }


@Override
    public void handleError(Throwable t) {
        LOG.error("Error thrown:" , t);
        Action currentAction = Action.CURRENT_ACTION.get();
        if (currentAction != null) {
            ErrorResponse er = new ErrorResponse(currentAction.getAddressable(), t, templateName);

            try {
                rh.accept(er);
            } catch (Throwable e) {
                LOG.warn("Couldn't return error {} due to error {} ", er, e);
            }

            Action.CURRENT_ACTION.set(Action.NULL_ACTION);
        }
    }
``
robmoffat commented 1 year ago

Couple of things missing in this that we talked about:

  1. I think in the first bit of code you need to clear the Action, right?
eh.handleError(e);      
Action.CURRENT_ACTION.set(Action.NULL_ACTION);

2 Instead of return null;I think we were doing return rr; ?

vaibhav-db commented 1 year ago

find solution, I have added below condition to set Action

if(!(t  instanceof ErrorResponse)) {
  Action.CURRENT_ACTION.set(new ErrorAction(address, data));
}
vaibhav-db commented 1 year ago

Couple of things missing in this that we talked about:

  1. I think in the first bit of code you need to clear the Action, right?
eh.handleError(e);        
Action.CURRENT_ACTION.set(Action.NULL_ACTION);

2 Instead of return null;I think we were doing return rr; ?

Couple of things missing in this that we talked about:

  1. I think in the first bit of code you need to clear the Action, right?
eh.handleError(e);        
Action.CURRENT_ACTION.set(Action.NULL_ACTION);

2 Instead of return null;I think we were doing return rr; ?

For point 1: Yes I have added clear action in ChatWorkflowErrorHandler. Now move to TeamsResponseHandle For point 2: That is different issue. For synchronize call we were thinking to add return rr ;

vaibhav-db commented 1 year ago

Issue fixed in PR https://github.com/finos/spring-bot/pull/407