Skip to content

AMQP-200: Listener Exceptions Should be Logged at at Least WARN level #15

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jamescarr wants to merge 4 commits into from
Closed

AMQP-200: Listener Exceptions Should be Logged at at Least WARN level #15

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5546ff5
changes log messages from debug to warn
jamescarr Nov 15, 2011
c7801d2
added @author tag as per contribution guidelines
jamescarr Nov 15, 2011
b75318e
changed isDebugEnabled and isInfoEnabled to isWarnEnabled.
jamescarr Nov 15, 2011
294a8b9
removed else if that never gets called now.
jamescarr Nov 15, 2011
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions .../main/java/org/springframework/amqp/rabbit/listener/AbstractMessageListenerContainer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
* @author Mark Pollack
* @author Mark Fisher
* @author Dave Syer
* @author James Carr
*/
public abstract class AbstractMessageListenerContainer extends RabbitAccessor implements BeanNameAware, DisposableBean,
SmartLifecycle {
Expand Down Expand Up @@ -423,9 +424,9 @@ protected void invokeErrorHandler(Throwable ex) {
if (this.errorHandler != null) {
this.errorHandler.handleError(ex);
} else if (logger.isDebugEnabled()) {
logger.debug("Execution of Rabbit message listener failed, and no ErrorHandler has been set.", ex);
logger.warn("Execution of Rabbit message listener failed, and no ErrorHandler has been set.", ex);
} else if (logger.isInfoEnabled()) {
Copy link
Contributor

@garyrussell garyrussell Nov 15, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should not log under warn after testing for isInfoEnabled (or isDebugEnabled). Always log at the same level as if test.

simply replace both with a single

if (logger.isWarnEnabled() {
logger.warn(...., ex);
}

(See JMS AMLC)

Copy link
Contributor Author

@jamescarr jamescarr Nov 15, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Just updated the commit range.

logger.info("Execution of Rabbit message listener failed, and no ErrorHandler has been set: "
logger.warn("Execution of Rabbit message listener failed, and no ErrorHandler has been set: "
+ ex.getClass() + ": " + ex.getMessage());
}
Copy link
Contributor

@garyrussell garyrussell Nov 15, 2011

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great!

You should remove this last else if{...} stanza; it will never be executed because the one above it will run.

Thanks.

}
Expand Down