Hystrix: HystrixCommand.getFallbackMethodName hard coded to return "getFallback"

[ryanjbaxter]

I believe that the implementation should return null, as the default implementation for getFallback throws an exception.

[mattrjacobs]

This is just an implementation detail to allow AbstractCommand to use reflection to determine if the user has implemented a fallback. Since the method name is different for the 2 subclasses of AbstractCommand (HystrixCommand/HystrixObservableCommand), (getFallback/resumeWithFallback) respectively, this allows the code to be more generic in AbstractCommand. The only time this is consulted is when determining if fallback execution hooks should be fired.

[ryanjbaxter]

@mattrjacobs i responded over in the google group, let me know if you prefer to track this via a github issue instead.

[mattrjacobs]

Github is easiest for me - so I'll stick here. Repasting your message:

Thanks for the response Matt The reason why it stood out to me is because my command did not specify a fallback but yet the execution hooks were still being fired. So if it is valid not to have a fallback method specified I think in this case getFallbackMethodName should not return anything.

[mattrjacobs]

I know there are unit tests which assert that vanilla Hystrix does not invoke the hooks when the getFallback method is implemented. Hystrix uses reflection to determine if the user has overridden the default getFallback() methods.

In the case of hystrix-javanica, the GenericCommand and GenericObservableCommand actually do implement getFallback() / resumeWithFallback() respectively. So the reflection check finds that there is a fallback implementation.

I'm not sure of the best way to solve this in hystrix-javanica. Any thoughts?

[ryanjbaxter]

So why not have them override getFallbackMethodName and return the actual method name then?

[mattrjacobs]

I'm not very familiar with the hystrix-javanica internals, but I think some solution like this would work inside that module.

The goal of that code should be to allow a superclass to write generic logic that is able to tell whether the user has defined a fallback or not. In retrospect, a better design of the method in AbstractCommand would have been a more-general boolean isFallbackUserDefined(), instead of dealing with implementation details like method names.

I can pick up that refactoring as part of the next release. That should make writing the javanica piece more straightforward

[mattrjacobs]

@ryanjbaxter @dmgcodevil This should make it more straightforward to determine if a user-defined fallback exists within hystrix-javanica. That logic will be javanica-specific, but AbstractCommand is now wired to use the new protected abstract boolean isFallbackUserDefined() method. So if the Javanica command re-implements that properly, things should work as expected.

[dmgcodevil]

@mattrjacobs thanks for isFallbackUserDefined(), I'll change javanica internals to use this method.

[mattrjacobs]

Great - thanks! Will leave this issue open until that gets merged in.