Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

HystrixCommand getFailedExecutionException().getMessage() causes Hystrix circuit short-circuited #1013

Closed
edwardahaynes opened this issue Dec 7, 2015 · 11 comments

Comments

@edwardahaynes
Copy link

I am running a Spring Boot application that make use of Hystrix commands. I have a HystrixCommand where in the getFallback() method, I am calling getFailedExecutionException().getMessage(). When this gets executed more then X times (say 20 failed requests), the Hystrix command stops executing the fallback method and instead the container throws a 500 Internal Server Error. Other then not calling this method (which is a valid workaround), how do I resolve this? What is causing this exception?

Here is the exception

[Request processing failed; nested exception is com.netflix.hystrix.exception.HystrixRuntimeException: ShortCicuitDemoCommand short-circuited and fallback failed.] with root cause

java.lang.RuntimeException: Hystrix circuit short-circuited and is OPEN
    at com.netflix.hystrix.AbstractCommand$1.call(AbstractCommand.java:413) ~[hystrix-core-1.4.20.jar:1.4.20]
    at com.netflix.hystrix.AbstractCommand$1.call(AbstractCommand.java:363) ~[hystrix-core-1.4.20.jar:1.4.20]
//Spring boot application
@SpringBootApplication
@EnableCircuitBreaker
public class ShortCicuitDemoApplication {
    public static void main(String[] args) {
        SpringApplication.run(ShortCicuitDemoApplication.class, args);
    }
}

//Controller command
@RestController
public class ShortCicuitDemoController {
    private RestTemplate restTemplate = new RestTemplate();
    @RequestMapping(value = "/shortcicuit")
    @ResponseBody
    public String doSomething() {
        ShortCicuitDemoCommand command = new ShortCicuitDemoCommand(restTemplate);
        String results = command.execute();
        return results;
    }
}

//HystrixCommand
public class ShortCicuitDemoCommand extends HystrixCommand<String> {
    private static final Logger log = LoggerFactory.getLogger(ShortCicuitDemoCommand.class);
    private RestTemplate restTemplate;
    public ShortCicuitDemoCommand(RestTemplate restTemplate) {
    super(Setter.withGroupKey(HystrixCommandGroupKey.Factory.asKey("ShortCicuitDemoGroup"))
            .andCommandKey(HystrixCommandKey.Factory.asKey("ShortCicuitDemoCommand")));
        this.restTemplate = restTemplate;
    }
    @Override
    protected String run() throws Exception {
        // make the call that will fail
        String response = restTemplate.getForObject("http://localhost/donothing", String.class);
        return response;
    }
    @Override
    protected String getFallback() {
        log.error("Fallback {}", this.getFailedExecutionException().getMessage());
        return null;
    }
}

Attached is the full compressed project (only 3 files)

ShortCicuitDemo.zip

@edwardahaynes
Copy link
Author

I figured out the issue. When you have debug logging turned on using logging.level.=DEBUG, the real issue shows up. Its just a NPE that the Hystrix code is hiding and showing as a different exception.

2015-12-07 16:50:04.396 DEBUG 7576 --- [nio-8080-exec-5] com.netflix.hystrix.AbstractCommand      : HystrixCommand execution SHORTCIRCUIT and fallback failed.

java.lang.NullPointerException: null
    at com.shortcircuit.demo.ShortCicuitDemoShortCircuitCommand.getFallback(ShortCicuitDemoShortCircuitCommand.java:38) ~[classes/:na]
    at com.shortcircuit.demo.ShortCicuitDemoShortCircuitCommand.getFallback(ShortCicuitDemoShortCircuitCommand.java:1) ~[classes/:na]
    at com.netflix.hystrix.HystrixCommand$2.call(HystrixCommand.java:311) [hystrix-core-1.4.20.jar:1.4.20]
    at com.netflix.hystrix.HystrixCommand$2.call(HystrixCommand.java:306) [hystrix-core-1.4.20.jar:1.4.20]

@mattrjacobs
Copy link
Contributor

My interpretation of the above is that returning null as a @ResponseBody is throwing some sort of framework-level exception. Is that correct?

@edwardahaynes
Copy link
Author

The issue was that when the circuit breaker is open, getFailedExecutionException() returns NULL and I was not doing a null check before using it.

@mattrjacobs
Copy link
Contributor

Ah, I see. This was brought up in #974 as well. A path forward here is:

  • AbstractCommand in hystrix-core now has a getExecutionException() method that should return an Exception instance in this case.
  • Once hystrix-javanica uses that method, you won't see a NullPointerException.

@dmgcodevil Any updates on that issue?

@dmgcodevil
Copy link
Contributor

@mattrjacobs javanica uses regular getaExecutionException from hystrix core so it should be fine.

@mattrjacobs
Copy link
Contributor

getExecutionException() was added in #1003 and is not released yet. My understanding was that a change was required in hystrix-javanica to use this method

@dmgcodevil
Copy link
Contributor

Is there any changes in terms how we get execution exception now ? If not, ones the fix will be in place javanica will pick it up and issue should go away without any specific changes, am I right ?

@mattrjacobs
Copy link
Contributor

Yes. I left the semantics of getFailedExecutionException() alone, so I wouldn't break any current consumers of that method. That method returns the exception thrown by a run() method only, and NULL otherwise.

The new method (getExecutionException()) returns the exception instance on any failure of the command, including FAILURE/SHORTCIRCUITED/TIMEOUT/BAD_REQUEST/SEMAPHORE_REJECTED/THREAD_POOL_REJECTED

I'm not sure exactly how javanica is wiring up the exception, but switching to getExecutionException() should resolve the case above, I believe.

@dmgcodevil
Copy link
Contributor

Oh, I see, then yes, I need to tweak javanica to use getExecutionException() instead of getFailedExecutionException(), it's sub penny change.

@dmgcodevil
Copy link
Contributor

@mattrjacobs I guess we can close this issue because 1155 has been resolved. however, the issue herein mentioned isn't releated to javanica because reporter uses vanilla Hystrix.

@mattrjacobs
Copy link
Contributor

Closing out - I don't believe there's anything more to do here. Please re-open if there's more to discuss

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants