Skip to content

Commit fdfb8f9

Browse files
mattrjacobsmattrjacobs
authored
Merge pull request #1567 from gagoman/fix/1536
Handle not wrapped exceptions same way as all other
2 parents f2b61c7 + 88b88e8 commit fdfb8f9

File tree

4 files changed

+69
-32
lines changed

4 files changed

+69
-32
lines changed

‎hystrix-contrib/hystrix-javanica/src/test/java/com/netflix/hystrix/contrib/javanica/test/common/error/BasicErrorPropagationTest.java‎

Lines changed: 25 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
package com.netflix.hystrix.contrib.javanica.test.common.error;
1717

1818
import com.netflix.hystrix.HystrixEventType;
19+
import com.netflix.hystrix.HystrixInvokableInfo;
1920
import com.netflix.hystrix.HystrixRequestLog;
2021
import com.netflix.hystrix.contrib.javanica.annotation.HystrixCommand;
2122
import com.netflix.hystrix.contrib.javanica.annotation.HystrixProperty;
@@ -198,24 +199,39 @@ public void testCommandWithFallbackThatFailsByTimeOut() {
198199
}
199200

200201
@Test
201-
public void testCommandThrowsNotWrappedException() {
202+
public void testCommandWithNotWrappedExceptionAndNoFallback() {
202203
try {
203-
userService.throwNotWrappedCheckedException();
204+
userService.throwNotWrappedCheckedExceptionWithoutFallback();
204205
fail();
205206
} catch (NotWrappedCheckedException e) {
206207
// pass
207208
} catch (Throwable e) {
208209
fail("'NotWrappedCheckedException' is expected exception.");
209210
}finally {
210211
assertEquals(1, HystrixRequestLog.getCurrentRequest().getAllExecutedCommands().size());
211-
com.netflix.hystrix.HystrixInvokableInfo getUserCommand = getHystrixCommandByKey("throwNotWrappedCheckedException");
212+
HystrixInvokableInfo getUserCommand = getHystrixCommandByKey("throwNotWrappedCheckedExceptionWithoutFallback");
212213
// record failure in metrics
213214
assertTrue(getUserCommand.getExecutionEvents().contains(HystrixEventType.FAILURE));
214215
// and will not trigger fallback logic
215216
verify(failoverService, never()).activate();
216217
}
217218
}
218219

220+
@Test
221+
public void testCommandWithNotWrappedExceptionAndFallback() {
222+
try {
223+
userService.throwNotWrappedCheckedExceptionWithFallback();
224+
} catch (NotWrappedCheckedException e) {
225+
fail();
226+
} finally {
227+
assertEquals(1, HystrixRequestLog.getCurrentRequest().getAllExecutedCommands().size());
228+
HystrixInvokableInfo getUserCommand = getHystrixCommandByKey("throwNotWrappedCheckedExceptionWithFallback");
229+
assertTrue(getUserCommand.getExecutionEvents().contains(HystrixEventType.FAILURE));
230+
assertTrue(getUserCommand.getExecutionEvents().contains(HystrixEventType.FALLBACK_SUCCESS));
231+
verify(failoverService).activate();
232+
}
233+
}
234+
219235
public static class UserService {
220236

221237
private FailoverService failoverService;
@@ -288,8 +304,13 @@ private void validate(String val) throws BadRequestException {
288304
}
289305
}
290306

307+
@HystrixCommand
308+
void throwNotWrappedCheckedExceptionWithoutFallback() throws NotWrappedCheckedException {
309+
throw new NotWrappedCheckedException();
310+
}
311+
291312
@HystrixCommand(fallbackMethod = "voidFallback")
292-
void throwNotWrappedCheckedException() throws NotWrappedCheckedException {
313+
void throwNotWrappedCheckedExceptionWithFallback() throws NotWrappedCheckedException {
293314
throw new NotWrappedCheckedException();
294315
}
295316

‎hystrix-core/src/main/java/com/netflix/hystrix/AbstractCommand.java‎

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -761,11 +761,7 @@ private Observable<R> getFallbackOrThrowException(final AbstractCommand<R> _cmd,
761761
// do this before executing fallback so it can be queried from within getFallback (see See https://github.com/Netflix/Hystrix/pull/144)
762762
executionResult = executionResult.addEvent((int) latency, eventType);
763763

764-
if (shouldNotBeWrapped(originalException)){
765-
/* executionHook for all errors */
766-
Exception e = wrapWithOnErrorHook(failureType, originalException);
767-
return Observable.error(e);
768-
} else if (isUnrecoverable(originalException)) {
764+
if (isUnrecoverable(originalException)) {
769765
logger.error("Unrecoverable Error for HystrixCommand so will throw HystrixRuntimeException and not apply fallback. ", originalException);
770766

771767
/* executionHook for all errors */
@@ -808,30 +804,33 @@ public void call() {
808804
final Func1<Throwable, Observable<R>> handleFallbackError = new Func1<Throwable, Observable<R>>() {
809805
@Override
810806
public Observable<R> call(Throwable t) {
811-
Exception e = originalException;
807+
/* executionHook for all errors */
808+
Exception e = wrapWithOnErrorHook(failureType, originalException);
812809
Exception fe = getExceptionFromThrowable(t);
813810

811+
long latency = System.currentTimeMillis() - executionResult.getStartTimestamp();
812+
Exception toEmit;
813+
814814
if (fe instanceof UnsupportedOperationException) {
815-
long latency = System.currentTimeMillis() - executionResult.getStartTimestamp();
816815
logger.debug("No fallback for HystrixCommand. ", fe); // debug only since we're throwing the exception and someone higher will do something with it
817816
eventNotifier.markEvent(HystrixEventType.FALLBACK_MISSING, commandKey);
818817
executionResult = executionResult.addEvent((int) latency, HystrixEventType.FALLBACK_MISSING);
819818

820-
/* executionHook for all errors */
821-
e = wrapWithOnErrorHook(failureType, e);
822-
823-
return Observable.error(new HystrixRuntimeException(failureType, _cmd.getClass(), getLogMessagePrefix() + " " + message + " and no fallback available.", e, fe));
819+
toEmit = new HystrixRuntimeException(failureType, _cmd.getClass(), getLogMessagePrefix() + " " + message + " and no fallback available.", e, fe);
824820
} else {
825-
long latency = System.currentTimeMillis() - executionResult.getStartTimestamp();
826821
logger.debug("HystrixCommand execution " + failureType.name() + " and fallback failed.", fe);
827822
eventNotifier.markEvent(HystrixEventType.FALLBACK_FAILURE, commandKey);
828823
executionResult = executionResult.addEvent((int) latency, HystrixEventType.FALLBACK_FAILURE);
829824

830-
/* executionHook for all errors */
831-
e = wrapWithOnErrorHook(failureType, e);
825+
toEmit = new HystrixRuntimeException(failureType, _cmd.getClass(), getLogMessagePrefix() + " " + message + " and fallback failed.", e, fe);
826+
}
832827

833-
return Observable.error(new HystrixRuntimeException(failureType, _cmd.getClass(), getLogMessagePrefix() + " " + message + " and fallback failed.", e, fe));
828+
// NOTE: we're suppressing fallback exception here
829+
if (shouldNotBeWrapped(originalException)) {
830+
return Observable.error(e);
834831
}
832+
833+
return Observable.error(toEmit);
835834
}
836835
};
837836

‎hystrix-core/src/test/java/com/netflix/hystrix/HystrixCommandTest.java‎

Lines changed: 27 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -190,7 +190,7 @@ public void testNotWrappedExceptionWithNoFallback() {
190190

191191
assertTrue(command.getExecutionTimeInMilliseconds() > -1);
192192
assertTrue(command.isFailedExecution());
193-
assertCommandExecutionEvents(command, HystrixEventType.FAILURE);
193+
assertCommandExecutionEvents(command, HystrixEventType.FAILURE, HystrixEventType.FALLBACK_MISSING);
194194
assertNotNull(command.getExecutionException());
195195
assertTrue(command.getExecutionException() instanceof NotWrappedByHystrixTestRuntimeException);
196196
assertEquals(0, command.getBuilder().metrics.getCurrentConcurrentExecutionCount());
@@ -223,6 +223,28 @@ public void testNotWrappedBadRequestWithNoFallback() {
223223
assertSaneHystrixRequestLog(1);
224224
}
225225

226+
@Test
227+
public void testNotWrappedBadRequestWithFallback() throws Exception {
228+
TestHystrixCommand<Integer> command = getCommand(ExecutionIsolationStrategy.THREAD, AbstractTestHystrixCommand.ExecutionResult.BAD_REQUEST_NOT_WRAPPED, AbstractTestHystrixCommand.FallbackResult.SUCCESS);
229+
try {
230+
command.execute();
231+
fail("we shouldn't get here");
232+
} catch (HystrixRuntimeException e) {
233+
e.printStackTrace();
234+
fail("we shouldn't get a HystrixRuntimeException");
235+
} catch (RuntimeException e) {
236+
assertTrue(e instanceof NotWrappedByHystrixTestRuntimeException);
237+
}
238+
239+
assertTrue(command.getExecutionTimeInMilliseconds() > -1);
240+
assertTrue(command.getEventCounts().contains(HystrixEventType.BAD_REQUEST));
241+
assertCommandExecutionEvents(command, HystrixEventType.BAD_REQUEST);
242+
assertNotNull(command.getExecutionException());
243+
assertTrue(command.getExecutionException() instanceof HystrixBadRequestException);
244+
assertTrue(command.getExecutionException().getCause() instanceof NotWrappedByHystrixTestRuntimeException);
245+
assertEquals(0, command.getBuilder().metrics.getCurrentConcurrentExecutionCount());
246+
assertSaneHystrixRequestLog(1);
247+
}
226248

227249
/**
228250
* Test a command execution that fails but has a fallback.
@@ -246,20 +268,12 @@ public void testExecutionFailureWithFallback() {
246268
@Test
247269
public void testNotWrappedExceptionWithFallback() {
248270
TestHystrixCommand<Integer> command = getCommand(ExecutionIsolationStrategy.THREAD, AbstractTestHystrixCommand.ExecutionResult.NOT_WRAPPED_FAILURE, AbstractTestHystrixCommand.FallbackResult.SUCCESS);
249-
try {
250-
command.execute();
251-
fail("we shouldn't get here");
252-
} catch (HystrixRuntimeException e) {
253-
e.printStackTrace();
254-
fail("we shouldn't get a HystrixRuntimeException");
255-
} catch (RuntimeException e) {
256-
assertTrue(e instanceof NotWrappedByHystrixTestRuntimeException);
257-
}
271+
assertEquals(FlexibleTestHystrixCommand.FALLBACK_VALUE, command.execute());
272+
assertEquals("Raw exception for TestHystrixCommand", command.getFailedExecutionException().getMessage());
258273
assertTrue(command.getExecutionTimeInMilliseconds() > -1);
259274
assertTrue(command.isFailedExecution());
260-
assertCommandExecutionEvents(command, HystrixEventType.FAILURE);
275+
assertCommandExecutionEvents(command, HystrixEventType.FAILURE, HystrixEventType.FALLBACK_SUCCESS);
261276
assertNotNull(command.getExecutionException());
262-
assertTrue(command.getExecutionException() instanceof NotWrappedByHystrixTestRuntimeException);
263277
assertEquals(0, command.getBuilder().metrics.getCurrentConcurrentExecutionCount());
264278
assertSaneHystrixRequestLog(1);
265279
}
@@ -2377,7 +2391,7 @@ public void onNext(Boolean args) {
23772391
assertTrue(t.get() instanceof NotWrappedByHystrixTestException);
23782392
assertTrue(command.getExecutionTimeInMilliseconds() > -1);
23792393
assertTrue(command.isFailedExecution());
2380-
assertCommandExecutionEvents(command, HystrixEventType.FAILURE);
2394+
assertCommandExecutionEvents(command, HystrixEventType.FAILURE, HystrixEventType.FALLBACK_MISSING);
23812395
assertNotNull(command.getExecutionException());
23822396
assertTrue(command.getExecutionException() instanceof NotWrappedByHystrixTestException);
23832397
assertEquals(0, command.getBuilder().metrics.getCurrentConcurrentExecutionCount());

‎hystrix-core/src/test/java/com/netflix/hystrix/NotWrappedByHystrixTestRuntimeException.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,4 +5,7 @@
55
public class NotWrappedByHystrixTestRuntimeException extends RuntimeException implements ExceptionNotWrappedByHystrix {
66
private static final long serialVersionUID = 1L;
77

8+
public NotWrappedByHystrixTestRuntimeException() {
9+
super("Raw exception for TestHystrixCommand");
10+
}
811
}

0 commit comments

Comments
 (0)