Skip to content

Commit 8e1ecec

Browse files
committed
Fine-tuned JCA MessageEndpoint exception logging and propagation
Issue: SPR-16717
1 parent 7ee6130 commit 8e1ecec

File tree

3 files changed

+26
-17
lines changed

3 files changed

+26
-17
lines changed

Diff for: spring-jms/src/main/java/org/springframework/jms/listener/endpoint/JmsMessageEndpointFactory.java

+6-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 the original author or authors.
2+
* Copyright 2002-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -84,6 +84,7 @@ private class JmsMessageEndpoint extends AbstractMessageEndpoint implements Mess
8484

8585
@Override
8686
public void onMessage(Message message) {
87+
Throwable endpointEx = null;
8788
boolean applyDeliveryCalls = !hasBeforeDeliveryBeenCalled();
8889
if (applyDeliveryCalls) {
8990
try {
@@ -97,6 +98,7 @@ public void onMessage(Message message) {
9798
getMessageListener().onMessage(message);
9899
}
99100
catch (RuntimeException | Error ex) {
101+
endpointEx = ex;
100102
onEndpointException(ex);
101103
throw ex;
102104
}
@@ -106,7 +108,9 @@ public void onMessage(Message message) {
106108
afterDelivery();
107109
}
108110
catch (ResourceException ex) {
109-
throw new JmsResourceException(ex);
111+
if (endpointEx == null) {
112+
throw new JmsResourceException(ex);
113+
}
110114
}
111115
}
112116
}

Diff for: spring-tx/src/main/java/org/springframework/jca/endpoint/AbstractMessageEndpointFactory.java

+4-2
Original file line numberDiff line numberDiff line change
@@ -269,9 +269,10 @@ protected final boolean hasBeforeDeliveryBeenCalled() {
269269
* endpoint throwing an exception.
270270
* @param ex the exception thrown from the concrete endpoint
271271
*/
272-
protected final void onEndpointException(Throwable ex) {
272+
protected void onEndpointException(Throwable ex) {
273273
Assert.state(this.transactionDelegate != null, "Not initialized");
274274
this.transactionDelegate.setRollbackOnly();
275+
logger.debug("Transaction marked as rollback-only after endpoint exception", ex);
275276
}
276277

277278
/**
@@ -291,6 +292,7 @@ public void afterDelivery() throws ResourceException {
291292
this.transactionDelegate.endTransaction();
292293
}
293294
catch (Throwable ex) {
295+
logger.warn("Failed to complete transaction after endpoint delivery", ex);
294296
throw new ApplicationServerInternalException("Failed to complete transaction", ex);
295297
}
296298
}
@@ -303,7 +305,7 @@ public void release() {
303305
this.transactionDelegate.endTransaction();
304306
}
305307
catch (Throwable ex) {
306-
logger.error("Could not complete unfinished transaction on endpoint release", ex);
308+
logger.warn("Could not complete unfinished transaction on endpoint release", ex);
307309
}
308310
}
309311
}

Diff for: spring-tx/src/main/java/org/springframework/jca/endpoint/GenericMessageEndpointFactory.java

+16-13
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2017 the original author or authors.
2+
* Copyright 2002-2018 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -108,24 +108,21 @@ private class GenericMessageEndpoint extends AbstractMessageEndpoint implements
108108

109109
@Override
110110
public Object invoke(MethodInvocation methodInvocation) throws Throwable {
111+
Throwable endpointEx = null;
111112
boolean applyDeliveryCalls = !hasBeforeDeliveryBeenCalled();
112113
if (applyDeliveryCalls) {
113114
try {
114115
beforeDelivery(null);
115116
}
116117
catch (ResourceException ex) {
117-
if (ReflectionUtils.declaresException(methodInvocation.getMethod(), ex.getClass())) {
118-
throw ex;
119-
}
120-
else {
121-
throw new InternalResourceException(ex);
122-
}
118+
throw adaptExceptionIfNecessary(methodInvocation, ex);
123119
}
124120
}
125121
try {
126122
return methodInvocation.proceed();
127123
}
128124
catch (Throwable ex) {
125+
endpointEx = ex;
129126
onEndpointException(ex);
130127
throw ex;
131128
}
@@ -135,17 +132,23 @@ public Object invoke(MethodInvocation methodInvocation) throws Throwable {
135132
afterDelivery();
136133
}
137134
catch (ResourceException ex) {
138-
if (ReflectionUtils.declaresException(methodInvocation.getMethod(), ex.getClass())) {
139-
throw ex;
140-
}
141-
else {
142-
throw new InternalResourceException(ex);
135+
if (endpointEx == null) {
136+
throw adaptExceptionIfNecessary(methodInvocation, ex);
143137
}
144138
}
145139
}
146140
}
147141
}
148142

143+
private Exception adaptExceptionIfNecessary(MethodInvocation methodInvocation, ResourceException ex) {
144+
if (ReflectionUtils.declaresException(methodInvocation.getMethod(), ex.getClass())) {
145+
return ex;
146+
}
147+
else {
148+
return new InternalResourceException(ex);
149+
}
150+
}
151+
149152
@Override
150153
protected ClassLoader getEndpointClassLoader() {
151154
return getMessageListener().getClass().getClassLoader();
@@ -164,7 +167,7 @@ protected ClassLoader getEndpointClassLoader() {
164167
@SuppressWarnings("serial")
165168
public static class InternalResourceException extends RuntimeException {
166169

167-
protected InternalResourceException(ResourceException cause) {
170+
public InternalResourceException(ResourceException cause) {
168171
super(cause);
169172
}
170173
}

0 commit comments

Comments
 (0)