APP-2970: Fixed infinite call loop in SymConfig #187
Conversation
| @@ -101,7 +101,7 @@ | |||
| public String getAgentUrl() { | |||
| String port = (this.getAgentPort() == 443) ? "" : ":" + this.getAgentPort(); | |||
| String contextPath = formatContextPath(this.getAgentContextPath()); | |||
| return CommonConstants.HTTPS_PREFIX + this.getAgentHost() + port + contextPath; | |||
| return CommonConstants.HTTPS_PREFIX + this.agentHost + port + contextPath; | |||
There was a problem hiding this comment.
isn't changing the logic? i.e the agent url won't be "load balanced" anymore?
There was a problem hiding this comment.
Truth is, I don't know the exact logic here and how to remove the stack overflow error (see my comment on issue APP-2970). So, please advise.
https://github.com/SymphonyPlatformSolutions/symphony-api-client-java/blob/master/symphony-bdk-legacy/symphony-api-client-java/src/main/java/configuration/SymLoadBalancedConfig.java#L98 says : Retrieves actual Agent FQDN (Fully Qualified Domain Name) by calling /agent/v1/info endpoint. so I was assuming we needed the non-load balanced agent host (the one configured in the bot configuration file).
There was a problem hiding this comment.
@symphony-elias with this test I was able to reproduce the stack overflow error:
@Test
public void getAgentHost_stackOverflow() {
final SymLoadBalancedConfig config = new SymLoadBalancedConfig();
final LoadBalancing loadBalancing = new LoadBalancing();
loadBalancing.setMethod(LoadBalancingMethod.external);
config.setLoadBalancing(loadBalancing);
assertNotNull(config.getAgentHost());
}
Not sure this is the exact same configuration as reported in the ticket but this only should not produce a stack overflow
There was a problem hiding this comment.
And with your fix I get
java.lang.IndexOutOfBoundsException: Index: 0
at java.util.Collections$EmptyList.get(Collections.java:4456)
at configuration.SymLoadBalancedConfig.rotateAgent(SymLoadBalancedConfig.java:86)
at configuration.SymLoadBalancedConfig.getAgentHost(SymLoadBalancedConfig.java:61)
Will there be only one and only one agentServers defined if the LB method is external?
maybe in that case we can improve the error message if it is not defined
There was a problem hiding this comment.
@symphony-youri Yes we expect to have one and only one item (clarified with Philippe Vialatte). I will add unit tests and add an explicit error message.
|
@symphony-elias We (or I?) recently refactored the |
I guess the related change is this one : b110cee#diff-fc189cefbad045bb241cea9622a1f008 |
No description provided.