Skip to content

Commit

Permalink
Merge pull request Netflix#334 from dmgcodevil/master
Browse files Browse the repository at this point in the history 
Fix for issue Netflix#332 (Hystrix Javanica Error Propagation does not work cor...
  • Loading branch information
@benjchristensen
benjchristensen committed Nov 11, 2014
2 parents 020b8d9 + 7ec5fde commit 3f8e2b5
Show file tree
Hide file tree
Showing 14 changed files with 517 additions and 81 deletions.
30 changes: 16 additions & 14 deletions hystrix-contrib/hystrix-javanica/build.gradle
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,24 @@ apply plugin: 'idea'
aspectjVersion = '1.7.4'

dependencies {
compile project(':hystrix-core')
compile project(':hystrix-core')

compile "org.aspectj:aspectjweaver:$aspectjVersion"
compile "org.aspectj:aspectjrt:$aspectjVersion"
compile "org.aspectj:aspectjweaver:$aspectjVersion"
compile "org.aspectj:aspectjrt:$aspectjVersion"

compile 'com.google.guava:guava:15.0'
compile 'commons-collections:commons-collections:3.2.1'
compile 'org.apache.commons:commons-lang3:3.1'
compile 'com.google.code.findbugs:jsr305:2.0.0'
testCompile 'junit:junit-dep:4.10'
testCompile 'org.springframework:spring-core:4.0.0.RELEASE'
testCompile 'org.springframework:spring-context:4.0.0.RELEASE'
testCompile 'org.springframework:spring-aop:4.0.0.RELEASE'
testCompile 'org.springframework:spring-test:4.0.0.RELEASE'
testCompile 'cglib:cglib:3.1'
testCompile 'org.mockito:mockito-all:1.9.5'
compile 'com.google.guava:guava:15.0'
compile 'commons-collections:commons-collections:3.2.1'
compile 'org.apache.commons:commons-lang3:3.1'
compile 'com.google.code.findbugs:jsr305:2.0.0'
testCompile 'junit:junit-dep:4.10'
testCompile 'org.springframework:spring-core:4.0.0.RELEASE'
testCompile 'org.springframework:spring-context:4.0.0.RELEASE'
testCompile 'org.springframework:spring-aop:4.0.0.RELEASE'
testCompile 'org.springframework:spring-test:4.0.0.RELEASE'
testCompile 'cglib:cglib:3.1'
testCompile 'org.mockito:mockito-all:1.9.5'
testCompile 'log4j:log4j:1.2.17'
testCompile 'org.slf4j:slf4j-log4j12:1.7.7'
}

eclipse {
Expand Down
14 changes: 9 additions & 5 deletions ...vanica/src/main/java/com/netflix/hystrix/contrib/javanica/collapser/CommandCollapser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package com.netflix.hystrix.contrib.javanica.collapser;

import com.google.common.base.Optional;
import com.netflix.hystrix.HystrixCollapser;
import com.netflix.hystrix.HystrixCollapserKey;
import com.netflix.hystrix.HystrixCommand;
Expand All @@ -33,7 +34,7 @@
* Collapses multiple requests into a single {@link HystrixCommand} execution based
* on a time window and optionally a max batch size.
*/
public class CommandCollapser extends HystrixCollapser<List<Object>, Object, Object> {
public class CommandCollapser extends HystrixCollapser<List<Optional<Object>>, Object, Object> {

private MetaHolder metaHolder;

Expand Down Expand Up @@ -69,7 +70,7 @@ public Object getRequestArgument() {
* Creates batch command.
*/
@Override
protected HystrixCommand<List<Object>> createCommand(
protected HystrixCommand<List<Optional<Object>>> createCommand(
Collection<CollapsedRequest<Object, Object>> collapsedRequests) {
BatchHystrixCommand command = BatchHystrixCommandFactory.getInstance().create(metaHolder, collapsedRequests);
command.setFallbackEnabled(metaHolder.getHystrixCollapser().fallbackEnabled());
Expand All @@ -80,14 +81,17 @@ protected HystrixCommand<List<Object>> createCommand(
* {@inheritDoc}
*/
@Override
protected void mapResponseToRequests(List<Object> batchResponse,
protected void mapResponseToRequests(List<Optional<Object>> batchResponse,
Collection<CollapsedRequest<Object, Object>> collapsedRequests) {
if (batchResponse.size() < collapsedRequests.size()) {
throw new RuntimeException(createMessage(collapsedRequests, batchResponse));
}
int count = 0;
for (CollapsedRequest<Object, Object> request : collapsedRequests) {
request.setResponse(batchResponse.get(count++));
Optional response = batchResponse.get(count++);
if (response.isPresent()) { // allows prevent IllegalStateException
request.setResponse(response.get());
}
}
}

Expand Down Expand Up @@ -116,7 +120,7 @@ public Setter build() {
}

private String createMessage(Collection<CollapsedRequest<Object, Object>> requests,
List<Object> response) {
List<Optional<Object>> response) {
return ERROR_MSG + arrayFormat(ERROR_MSF_TEMPLATE, new Object[]{getCollapserKey().name(), requests.size(), response.size()}).getMessage();
}

Expand Down
79 changes: 71 additions & 8 deletions ...ca/src/main/java/com/netflix/hystrix/contrib/javanica/command/AbstractHystrixCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
import com.google.common.base.Throwables;
import com.netflix.hystrix.HystrixCollapser;
import com.netflix.hystrix.contrib.javanica.conf.HystrixPropertiesManager;
import com.netflix.hystrix.contrib.javanica.exception.CommandActionExecutionException;
import com.netflix.hystrix.exception.HystrixBadRequestException;
import com.netflix.hystrix.exception.HystrixRuntimeException;

import javax.annotation.concurrent.ThreadSafe;
import java.util.Collection;
Expand Down Expand Up @@ -136,12 +138,18 @@ protected String getCacheKey() {
return key;
}

/**
* Check whether triggered exception is ignorable.
*
* @param throwable the exception occurred during a command execution
* @return true if exception is ignorable, otherwise - false
*/
boolean isIgnorable(Throwable throwable) {
if (ignoreExceptions == null || ignoreExceptions.length == 0) {
return false;
}
for (Class<? extends Throwable> ignoreException : ignoreExceptions) {
if (throwable.getClass().isAssignableFrom(ignoreException)) {
if (ignoreException.isAssignableFrom(throwable.getClass())) {
return true;
}
}
Expand All @@ -150,20 +158,28 @@ boolean isIgnorable(Throwable throwable) {

/**
* Executes an action. If an action has failed and an exception is ignorable then propagate it as HystrixBadRequestException
* otherwise propagate it as RuntimeException to trigger fallback method.
* otherwise propagate original exception to trigger fallback method.
* Note: If an exception occurred in a command directly extends {@link java.lang.Throwable} then this exception cannot be re-thrown
* as original exception because HystrixCommand.run() allows throw subclasses of {@link java.lang.Exception}.
* Thus we need to wrap cause in RuntimeException, anyway in this case the fallback logic will be triggered.
*
* @param action the action
* @return result of command action execution
*/
Object process(Action action) throws RuntimeException {
Object process(Action action) throws Exception {
Object result;
try {
result = action.execute();
} catch (Throwable throwable) {
if (isIgnorable(throwable)) {
throw new HystrixBadRequestException(throwable.getMessage(), throwable);
} catch (CommandActionExecutionException throwable) {
Throwable cause = throwable.getCause();
if (isIgnorable(cause)) {
throw new HystrixBadRequestException(cause.getMessage(), cause);
}
if (cause instanceof Exception) {
throw (Exception) cause;
} else {
throw Throwables.propagate(cause);
}
throw Throwables.propagate(throwable);
}
return result;
}
Expand All @@ -186,7 +202,54 @@ protected T getFallback() {
* Common action.
*/
abstract class Action {
abstract Object execute();
/**
* Each implementation of this method should wrap any exceptions in CommandActionExecutionException.
*
* @return execution result
* @throws CommandActionExecutionException
*/
abstract Object execute() throws CommandActionExecutionException;
}


/**
* Builder to create error message for failed fallback operation.
*/
static class FallbackErrorMessageBuilder {
private StringBuilder builder = new StringBuilder("failed to processed fallback");

static FallbackErrorMessageBuilder create() {
return new FallbackErrorMessageBuilder();
}

public FallbackErrorMessageBuilder append(CommandAction action, Throwable throwable) {
return commandAction(action).exception(throwable);
}

private FallbackErrorMessageBuilder commandAction(CommandAction action) {
if (action instanceof CommandExecutionAction || action instanceof LazyCommandExecutionAction) {
builder.append(": '").append(action.getActionName()).append("'. ")
.append(action.getActionName()).append(" fallback is a hystrix command. ");
} else if (action instanceof MethodExecutionAction) {
builder.append(" is the method: '").append(action.getActionName()).append("'. ");
}
return this;
}

private FallbackErrorMessageBuilder exception(Throwable throwable) {
if (throwable instanceof HystrixBadRequestException) {
builder.append("exception: '").append(throwable.getCause().getClass())
.append("' occurred in fallback was ignored and wrapped to HystrixBadRequestException.\n");
} else if (throwable instanceof HystrixRuntimeException) {
builder.append("exception: '").append(throwable.getCause().getClass())
.append("' occurred in fallback wasn't ignored.\n");
}
return this;
}

public String build() {
return builder.toString();
}
}

}
54 changes: 37 additions & 17 deletions ...anica/src/main/java/com/netflix/hystrix/contrib/javanica/command/BatchHystrixCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,13 @@
package com.netflix.hystrix.contrib.javanica.command;


import com.google.common.base.Optional;
import com.google.common.collect.Lists;
import com.netflix.hystrix.HystrixCollapser;
import com.netflix.hystrix.contrib.javanica.exception.FallbackInvocationException;
import com.netflix.hystrix.exception.HystrixBadRequestException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.annotation.concurrent.ThreadSafe;
import java.util.Collection;
Expand All @@ -29,7 +33,9 @@
* This command is used in collapser.
*/
@ThreadSafe
public class BatchHystrixCommand extends AbstractHystrixCommand<List<Object>> {
public class BatchHystrixCommand extends AbstractHystrixCommand<List<Optional<Object>>> {

private static final Logger LOGGER = LoggerFactory.getLogger(GenericCommand.class);

/**
* If some error occurs during the processing in run() then if {@link #fallbackEnabled} is true then the {@link #processWithFallback}
Expand Down Expand Up @@ -63,22 +69,21 @@ public void setFallbackEnabled(boolean fallbackEnabled) {
* {@inheritDoc}
*/
@Override
protected List<Object> run() throws Exception {
List<Object> response = Lists.newArrayList();
protected List<Optional<Object>> run() throws Exception {
List<Optional<Object>> response = Lists.newArrayList();
for (HystrixCollapser.CollapsedRequest<Object, Object> request : getCollapsedRequests()) {
final Object[] args = (Object[]) request.getArgument();
try {
response.add(fallbackEnabled ? processWithFallback(args) : process(args));
} catch (RuntimeException ex) {
response.add(Optional.of(fallbackEnabled ? processWithFallback(args) : process(args)));
} catch (Exception ex) {
request.setException(ex);
response.add(null);
response.add(Optional.absent());
}

}
return response;
}

private Object process(final Object[] args) {
private Object process(final Object[] args) throws Exception {
return process(new Action() {
@Override
Object execute() {
Expand All @@ -87,29 +92,44 @@ Object execute() {
});
}

private Object processWithFallback(final Object[] args) {
Object result = null;
private Object processWithFallback(final Object[] args) throws Exception {
Object result;
try {
result = process(args);
} catch (RuntimeException ex) {
if (ex instanceof HystrixBadRequestException || getFallbackAction() == null) {
} catch (Exception ex) {
if (ex instanceof HystrixBadRequestException) {
throw ex;
} else {
if (getFallbackAction() != null) {
result = processFallback(args);
} else {
// if command doesn't have fallback then
// call super.getFallback() that throws exception by default.
result = super.getFallback();
}
}
}
return result;
}

private Object processFallback(final Object[] args) {
return process(new Action() {
@Override
Object execute() {
return getFallbackAction().executeWithArgs(ExecutionType.SYNCHRONOUS, args);
if (getFallbackAction() != null) {
final CommandAction commandAction = getFallbackAction();
try {
return process(new Action() {
@Override
Object execute() {
return commandAction.executeWithArgs(ExecutionType.SYNCHRONOUS, args);
}
});
} catch (Throwable e) {
LOGGER.error(FallbackErrorMessageBuilder.create()
.append(commandAction, e).build());
throw new FallbackInvocationException(e.getCause());
}
});
} else {
return super.getFallback();
}
}


Expand Down
28 changes: 26 additions & 2 deletions ...ix-javanica/src/main/java/com/netflix/hystrix/contrib/javanica/command/CommandAction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,37 @@
*/
package com.netflix.hystrix.contrib.javanica.command;

import com.netflix.hystrix.contrib.javanica.exception.CommandActionExecutionException;

/**
* Simple action to encapsulate some logic to process it in a Hystrix command.
*/
public abstract class CommandAction {

public abstract Object execute(ExecutionType executionType);
/**
* Executes action in accordance with the given execution type.
*
* @param executionType the execution type
* @return result of execution
* @throws com.netflix.hystrix.contrib.javanica.exception.CommandActionExecutionException
*/
public abstract Object execute(ExecutionType executionType) throws CommandActionExecutionException;

/**
* Executes action with parameters in accordance with the given execution ty
*
* @param executionType the execution type
* @param args the parameters of the action
* @return result of execution
* @throws com.netflix.hystrix.contrib.javanica.exception.CommandActionExecutionException
*/
public abstract Object executeWithArgs(ExecutionType executionType, Object[] args) throws CommandActionExecutionException;

public abstract Object executeWithArgs(ExecutionType executionType, Object[] args);
/**
* Gets action name. Useful for debugging.
*
* @return the action name
*/
public abstract String getActionName();

}
Loading

0 comments on commit 3f8e2b5

Please sign in to comment.