Clarify HystrixBadRequestException and ExceptionNotWrappedByHystrix

[akhomchenko]

This is related to #1503 and #1414

So my question. Right now exceptions that implement ExceptionNotWrappedByHystrix behave like exceptions wrapped in HystrixBadRequestException. E.g. they do not trigger fallback logic.
For my understanding the goal of #1503 was to unwrap exceptions wrapped in HystrixBadRequestException but not change the logic itself. Something like:

• if it is an exception that implements ExceptionNotWrappedByHystrix and no fallback is awailable, throw it. Otherwise call fallback.

• if it is an exception that implements ExceptionNotWrappedByHystrix wrapped in HystrixBadRequestException throw it without triggering fallback.

Sample:

private static final class HystrixBadRequestExceptionFallback extends HystrixCommand<String> {
    HystrixBadRequestExceptionFallback() {
        super(asKey("hystrix-bad-request-exception-fallback"));
    }

    @Override
    protected String run() throws Exception {
        throw new HystrixBadRequestException("need to fallback", new RuntimeException());
    }

    // NOTE: not executed as expected
    @Override
    protected String getFallback() {
        System.out.println("[HystrixBadRequestExceptionFallback.fallback] should not happen");
        throw new RuntimeException("not expected");
    }
}

private static final class Unwrapped extends RuntimeException implements ExceptionNotWrappedByHystrix {
}

private static final class UnwrappedFallback extends HystrixCommand<String> {
    UnwrappedFallback() {
        super(asKey("unwrapped-exception-fallback"));
    }


    @Override
    protected String run() throws Exception {
        throw new Unwrapped();
    }

    // NOTE: not executed but expected to be
    @Override
    protected String getFallback() {
        System.out.println("[UnwrappedFallback.fallback] should happen???");
        return "UnwrappedFallback fallback";
    }
}

private static class UnwrappedHystrixBadRequestExceptionFallback extends HystrixCommand<String> {
    UnwrappedHystrixBadRequestExceptionFallback() {
        super(asKey("unwrapped-exception-hystrix-bad-request-exception-fallback"));
    }

    @Override
    protected String run() throws Exception {
        throw new HystrixBadRequestException("expected one", new Unwrapped());
    }

    // NOTE: executed but not expected according to HystrixBadRequestException logic
    @Override
    protected String getFallback() {
        System.out.println("[UnwrappedHystrixBadRequestExceptionFallback.fallback] should not happen???");
        throw new RuntimeException("not expected???");
    }
}

[mattrjacobs]

Thanks for the report @gagoman . That definitely looks like a bug that was overlooked in the implementation of this feature. I can take a look at it shortly. unless @mNantern or @tbvh are interested in getting to it first.

[tbvh]

Hi,
Imo the fallback in UnwrappedFallback should not occur (as observed, right?). My intention with ExceptionNotWrappedByHystrix was to allow error handling happen somewhere up on the stack, so not applying the fallback is appropriate.

Allong the same lines I would not expect the fallback in UnwrappedHystrixBadRequestExceptionFallback either.

Unfortunately I'm not able to dive in the code atm.

[akhomchenko]

Then we need to clarify ExceptionNotWrappedByHystrix behavior. My idea is that it is a marker interface that allows to get an original exception, while general behavior of Hystrix should not be affected.

UPD: I've tested on the latest release. @tbvh is right. Exception is not executed in case of UnwrappedHystrixBadRequestExceptionFallback, which is corrected, while Unwrapped fallback is still not called.

UPD2: I'll provide tests and code to show that and it will be up to you guys to decide.

[dmgcodevil]

Let me share my opinion regarding this feature. Just by looking at name ExceptionNotWrappedByHystrix I'd assume that caller will get business exception, which is cause I believe. Fallback behavior should remain the same, i.e. :

1. class UserException extends HystrixBadRequestException implements ExceptionNotWrappedByHystrix - should not trigger fallback, UserException should be propagated to a caller.

2. class UserException implements ExceptionNotWrappedByHystrix - should trigger fallback, UserException should be propagated to a caller.

Regarding fallback suppression I'd like to have a separate annotation, e.g. ExceptionIgnoredByHystix , if a command throws any exception implements this interface fallback logic should't be triggered.
And then you can combine ExceptionNotWrappedByHystrix and ExceptionIgnoredByHystix to achieve desirable behavior.

Regarding javanica:

1. javanica always propagates business exception unless opposite specified using raiseHystrixExceptions property of @HystrixCommand
2. javanica allows to specify exceptions that should be ignored i.e. don't trigger fallback logic.

[mattrjacobs]

@dmgcodevil That's an interesting way of looking at it that I hadn't considered. It seems very close to HystrixBadRequestException.

2 questions:

1. On balance, do you think the change in Handle not wrapped exceptions same way as all other #1567 is worthy (my opinion is yes)?
2. If you'd like to discuss the ExceptionIgnoredByHystrix, can you open up a new issue?

[mattrjacobs]

I'm accepting #1567. If we should continue discussing the semantics of the other exception type, please open up a new issue.