Skip to content

Commit

Permalink
Exposes all root causes to ExceptionHandler methods
Browse files Browse the repository at this point in the history
Closes gh-28155
  • Loading branch information
rstoyanchev committed Apr 28, 2022
1 parent b6b03f3 commit b30f4d7
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 14 deletions.
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2020 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand All @@ -16,6 +16,7 @@

package org.springframework.web.reactive.result.method.annotation;

import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.function.Function;
Expand Down Expand Up @@ -213,21 +214,30 @@ private Mono<HandlerResult> handleException(Throwable exception, HandlerMethod h

InvocableHandlerMethod invocable = this.methodResolver.getExceptionHandlerMethod(exception, handlerMethod);
if (invocable != null) {
ArrayList<Throwable> exceptions = new ArrayList<>();
try {
if (logger.isDebugEnabled()) {
logger.debug(exchange.getLogPrefix() + "Using @ExceptionHandler " + invocable);
}
bindingContext.getModel().asMap().clear();
Throwable cause = exception.getCause();
if (cause != null) {
return invocable.invoke(exchange, bindingContext, exception, cause, handlerMethod);
}
else {
return invocable.invoke(exchange, bindingContext, exception, handlerMethod);

// Expose causes as provided arguments as well
Throwable exToExpose = exception;
while (exToExpose != null) {

This comment has been minimized.

Copy link
@vy

vy Apr 29, 2022

Shouldn't we be protecting this causal chain retrieval against recursion? For inspiration, check out how Log4j tackles this.

exceptions.add(exToExpose);
Throwable cause = exToExpose.getCause();
exToExpose = (cause != exToExpose ? cause : null);
}
Object[] arguments = new Object[exceptions.size() + 1];
exceptions.toArray(arguments); // efficient arraycopy call in ArrayList
arguments[arguments.length - 1] = handlerMethod;

return invocable.invoke(exchange, bindingContext, arguments);
}
catch (Throwable invocationEx) {
if (logger.isWarnEnabled()) {
// Any other than the original exception (or a cause) is unintended here,
// probably an accident (e.g. failed assertion or the like).
if (!exceptions.contains(invocationEx) && logger.isWarnEnabled()) {
logger.warn(exchange.getLogPrefix() + "Failure in @ExceptionHandler " + invocable, invocationEx);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -82,9 +82,10 @@ public void resolveExceptionWithAssertionError() throws Exception {

@Test
public void resolveExceptionWithAssertionErrorAsRootCause() throws Exception {
AssertionError cause = new AssertionError("argh");
FatalBeanException exception = new FatalBeanException("wrapped", cause);
testException(exception, cause.toString());
AssertionError rootCause = new AssertionError("argh");
FatalBeanException cause = new FatalBeanException("wrapped", rootCause);
Exception exception = new Exception(cause);
testException(exception, rootCause.toString());
}

private void testException(Throwable exception, String expected) throws Exception {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2022 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -138,7 +138,7 @@ public Publisher<String> handleAndThrowExceptionWithCause() {

@GetMapping("/thrown-exception-with-cause-to-handle")
public Publisher<String> handleAndThrowExceptionWithCauseToHandle() {
throw new RuntimeException("State", new IOException("IO"));
throw new RuntimeException("State1", new RuntimeException("State2", new IOException("IO")));
}

@GetMapping(path = "/mono-error")
Expand Down

0 comments on commit b30f4d7

Please sign in to comment.