Skip to content

Commit 80d679a

Browse files
artembilangaryrussell
authored andcommitted
GH-2748: More bean definitions into exceptions
Fixes #2748 * Refactor more `MessageHandlingException`s to include `this` into an exception message * Revert using `MessagingException` in some places which really are not about messaging. This helps to wrap them into `MessageHandlingException` later in the `MessageHandler` for the `BeanDefinition` reference * Remove `volatile` from configuration properties in the affected classes * Remove already deprecated `JmsOutboundGateway.setPriority()` * Add `resource` and `source` for `BeanDefinition` in the `AbstractChannelAdapterParser` & `AbstractInboundGatewayParser` * Document the feature
1 parent 0ed88c3 commit 80d679a

File tree

46 files changed

+352
-370
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

46 files changed

+352
-370
lines changed

spring-integration-amqp/src/main/java/org/springframework/integration/amqp/outbound/AsyncAmqpOutboundGateway.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public String getComponentType() {
6464

6565
@Override
6666
protected RabbitTemplate getRabbitTemplate() {
67-
return this.template.getRabbitTemplate();
67+
return this.template.getRabbitTemplate();
6868
}
6969

7070
@Override
@@ -112,7 +112,8 @@ public void onSuccess(org.springframework.amqp.core.Message result) {
112112
catch (Exception e) {
113113
Exception exceptionToLogAndSend = e;
114114
if (!(e instanceof MessagingException)) {
115-
exceptionToLogAndSend = new MessageHandlingException(this.requestMessage, e);
115+
exceptionToLogAndSend = new MessageHandlingException(this.requestMessage,
116+
"failed to handle a message in the [" + AsyncAmqpOutboundGateway.this + ']', e);
116117
if (replyMessageBuilder != null) {
117118
exceptionToLogAndSend =
118119
new MessagingException(replyMessageBuilder.build(), exceptionToLogAndSend);

spring-integration-core/src/main/java/org/springframework/integration/aggregator/AbstractCorrelatingMessageHandler.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,7 @@ protected void handleMessageInternal(Message<?> message) {
454454
}
455455
catch (InterruptedException e) {
456456
Thread.currentThread().interrupt();
457-
throw new MessageHandlingException(message, "Interrupted getting lock", e);
457+
throw new MessageHandlingException(message, "Interrupted getting lock in the [" + this + ']', e);
458458
}
459459
try {
460460
noOutput = processMessageForGroup(message, correlationKey, groupIdUuid, lock);

spring-integration-core/src/main/java/org/springframework/integration/aggregator/BarrierMessageHandler.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -169,7 +169,8 @@ protected Object handleRequestMessage(Message<?> requestMessage) {
169169
}
170170
catch (InterruptedException e) {
171171
Thread.currentThread().interrupt();
172-
throw new MessageHandlingException(requestMessage, "Interrupted while waiting for release", e);
172+
throw new MessageHandlingException(requestMessage,
173+
"Interrupted while waiting for release in the [" + this + ']', e);
173174
}
174175
finally {
175176
this.inProcess.remove(key);

spring-integration-core/src/main/java/org/springframework/integration/config/xml/AbstractChannelAdapterParser.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ public abstract class AbstractChannelAdapterParser extends AbstractBeanDefinitio
4848
@Override
4949
protected final String resolveId(Element element, AbstractBeanDefinition definition, ParserContext parserContext)
5050
throws BeanDefinitionStoreException {
51+
5152
String id = element.getAttribute(ID_ATTRIBUTE);
5253
if (!element.hasAttribute("channel")) {
5354
// the created channel will get the 'id', so the adapter's bean name includes a suffix
@@ -82,21 +83,21 @@ protected final AbstractBeanDefinition parseInternal(Element element, ParserCont
8283
propertyValues.add("role", new TypedStringValue(role));
8384
}
8485
}
86+
beanDefinition.setResource(parserContext.getReaderContext().getResource());
87+
beanDefinition.setSource(IntegrationNamespaceUtils.createElementDescription(element));
8588
return beanDefinition;
8689
}
8790

8891
private String createDirectChannel(Element element, ParserContext parserContext) {
8992
if (parserContext.isNested()) {
9093
return null;
9194
}
92-
9395
return IntegrationNamespaceUtils.createDirectChannel(element, parserContext);
9496
}
9597

9698
/**
9799
* Subclasses must implement this method to parse the adapter element.
98100
* The name of the MessageChannel bean is provided.
99-
*
100101
* @param element The element.
101102
* @param parserContext The parser context.
102103
* @param channelName The channel name.

spring-integration-core/src/main/java/org/springframework/integration/config/xml/AbstractInboundGatewayParser.java

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,14 @@ protected boolean isEligibleAttribute(String attributeName) {
5656
&& !attributeName.equals("reply-channel") && super.isEligibleAttribute(attributeName);
5757
}
5858

59+
@Override
60+
protected void doParse(Element element, ParserContext parserContext, BeanDefinitionBuilder builder) {
61+
super.doParse(element, parserContext, builder);
62+
AbstractBeanDefinition beanDefinition = builder.getRawBeanDefinition();
63+
beanDefinition.setResource(parserContext.getReaderContext().getResource());
64+
beanDefinition.setSource(IntegrationNamespaceUtils.createElementDescription(element));
65+
}
66+
5967
@Override
6068
protected final void postProcess(BeanDefinitionBuilder builder, Element element) {
6169
String requestChannelRef = element.getAttribute("request-channel");
@@ -69,7 +77,7 @@ protected final void postProcess(BeanDefinitionBuilder builder, Element element)
6977
if (StringUtils.hasText(errorChannel)) {
7078
builder.addPropertyValue("errorChannelName", errorChannel);
7179
}
72-
this.doPostProcess(builder, element);
80+
doPostProcess(builder, element);
7381
}
7482

7583
/**

spring-integration-core/src/main/java/org/springframework/integration/context/IntegrationObjectSupport.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ public String getBeanDescription() {
153153
StringBuilder sb = new StringBuilder("bean '")
154154
.append(this.beanName).append("'");
155155
if (!this.beanName.equals(getComponentName())) {
156-
sb.append("for component '").append(getComponentName()).append("'");
156+
sb.append(" for component '").append(getComponentName()).append("'");
157157
}
158158
if (description != null) {
159159
sb.append("; defined in: '").append(description).append("'");

spring-integration-core/src/main/java/org/springframework/integration/handler/AbstractMessageProducingHandler.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -475,7 +475,8 @@ protected void sendErrorMessage(Message<?> requestMessage, Throwable ex) {
475475
}
476476
catch (Exception e) {
477477
Exception exceptionToLog =
478-
IntegrationUtils.wrapInHandlingExceptionIfNecessary(requestMessage, () -> null, e);
478+
IntegrationUtils.wrapInHandlingExceptionIfNecessary(requestMessage,
479+
() -> "failed to send error message in the [" + this + ']', e);
479480
logger.error("Failed to send async reply", exceptionToLog);
480481
}
481482
}

spring-integration-core/src/main/java/org/springframework/integration/handler/DelayHandler.java

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@
4646
import org.springframework.messaging.Message;
4747
import org.springframework.messaging.MessageChannel;
4848
import org.springframework.messaging.MessageHandler;
49-
import org.springframework.messaging.MessageHandlingException;
5049
import org.springframework.messaging.MessagingException;
5150
import org.springframework.messaging.core.DestinationResolver;
5251
import org.springframework.messaging.support.ErrorMessage;
@@ -381,7 +380,7 @@ else if (delayValue != null) {
381380
}
382381
}
383382
else {
384-
throw new MessageHandlingException(message, "Error occurred during 'delay' value determination",
383+
throw new IllegalStateException("Error occurred during 'delay' value determination",
385384
delayValueException);
386385
}
387386
}

spring-integration-core/src/main/java/org/springframework/integration/handler/MethodInvokingMessageProcessor.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,8 @@ public T processMessage(Message<?> message) {
109109
}
110110
catch (Exception ex) {
111111
throw IntegrationUtils.wrapInHandlingExceptionIfNecessary(message,
112-
() -> "error occurred during processing message in 'MethodInvokingMessageProcessor' [" + this +
113-
"]", ex);
112+
() -> "error occurred during processing message in 'MethodInvokingMessageProcessor' [" + this + ']',
113+
ex);
114114
}
115115
}
116116

spring-integration-core/src/main/java/org/springframework/integration/transformer/ContentEnricher.java

Lines changed: 27 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -62,50 +62,52 @@ public class ContentEnricher extends AbstractReplyProducingMessageHandler implem
6262
*/
6363
private final SpelExpressionParser parser = new SpelExpressionParser(new SpelParserConfiguration(true, true));
6464

65-
private volatile Map<Expression, Expression> nullResultPropertyExpressions = new HashMap<>();
65+
private Map<Expression, Expression> nullResultPropertyExpressions = new HashMap<>();
6666

67-
private volatile Map<String, HeaderValueMessageProcessor<?>> nullResultHeaderExpressions =
68-
new HashMap<>();
67+
private Map<String, HeaderValueMessageProcessor<?>> nullResultHeaderExpressions = new HashMap<>();
6968

70-
private volatile Map<Expression, Expression> propertyExpressions = new HashMap<>();
69+
private Map<Expression, Expression> propertyExpressions = new HashMap<>();
7170

72-
private volatile Map<String, HeaderValueMessageProcessor<?>> headerExpressions =
73-
new HashMap<>();
71+
private Map<String, HeaderValueMessageProcessor<?>> headerExpressions = new HashMap<>();
7472

7573
private EvaluationContext sourceEvaluationContext;
7674

7775
private EvaluationContext targetEvaluationContext;
7876

79-
private volatile boolean shouldClonePayload = false;
77+
private boolean shouldClonePayload = false;
8078

8179
private Expression requestPayloadExpression;
8280

83-
private volatile MessageChannel requestChannel;
81+
private MessageChannel requestChannel;
8482

85-
private volatile String requestChannelName;
83+
private String requestChannelName;
8684

87-
private volatile MessageChannel replyChannel;
85+
private MessageChannel replyChannel;
8886

89-
private volatile String replyChannelName;
87+
private String replyChannelName;
9088

91-
private volatile MessageChannel errorChannel;
89+
private MessageChannel errorChannel;
9290

93-
private volatile String errorChannelName;
91+
private String errorChannelName;
9492

95-
private volatile Gateway gateway = null;
93+
private Gateway gateway;
9694

97-
private volatile Long requestTimeout;
95+
private Long requestTimeout;
9896

99-
private volatile Long replyTimeout;
97+
private Long replyTimeout;
10098

10199
public void setNullResultPropertyExpressions(Map<String, Expression> nullResultPropertyExpressions) {
102-
Map<Expression, Expression> localMap = new HashMap<>(nullResultPropertyExpressions.size());
103-
for (Map.Entry<String, Expression> entry : nullResultPropertyExpressions.entrySet()) {
100+
this.nullResultPropertyExpressions = convertExpressions(nullResultPropertyExpressions);
101+
}
102+
103+
private Map<Expression, Expression> convertExpressions(Map<String, Expression> expressions) {
104+
Map<Expression, Expression> localMap = new HashMap<>(expressions.size());
105+
for (Map.Entry<String, Expression> entry : expressions.entrySet()) {
104106
String key = entry.getKey();
105107
Expression value = entry.getValue();
106108
localMap.put(this.parser.parseExpression(key), value);
107109
}
108-
this.nullResultPropertyExpressions = localMap;
110+
return localMap;
109111
}
110112

111113
public void setNullResultHeaderExpressions(Map<String, HeaderValueMessageProcessor<?>> nullResultHeaderExpressions) {
@@ -122,13 +124,7 @@ public void setPropertyExpressions(Map<String, Expression> propertyExpressions)
122124
Assert.notEmpty(propertyExpressions, "propertyExpressions must not be empty");
123125
Assert.noNullElements(propertyExpressions.keySet().toArray(), "propertyExpressions keys must not be empty");
124126
Assert.noNullElements(propertyExpressions.values().toArray(), "propertyExpressions values must not be empty");
125-
Map<Expression, Expression> localMap = new HashMap<>(propertyExpressions.size());
126-
for (Map.Entry<String, Expression> entry : propertyExpressions.entrySet()) {
127-
String key = entry.getKey();
128-
Expression value = entry.getValue();
129-
localMap.put(this.parser.parseExpression(key), value);
130-
}
131-
this.propertyExpressions = localMap;
127+
this.propertyExpressions = convertExpressions(propertyExpressions);
132128
}
133129

134130
/**
@@ -337,7 +333,8 @@ protected void doInit() {
337333
}
338334
}
339335

340-
for (Map.Entry<String, HeaderValueMessageProcessor<?>> entry : this.nullResultHeaderExpressions.entrySet()) {
336+
for (Map.Entry<String, HeaderValueMessageProcessor<?>> entry :
337+
this.nullResultHeaderExpressions.entrySet()) {
341338
if (checkReadOnlyHeaders &&
342339
(MessageHeaders.ID.equals(entry.getKey()) || MessageHeaders.TIMESTAMP.equals(entry.getKey()))) {
343340
throw new BeanInitializationException(
@@ -362,7 +359,8 @@ protected Object handleRequestMessage(Message<?> requestMessage) {
362359
targetPayload = ReflectionUtils.invokeMethod(cloneMethod, requestPayload);
363360
}
364361
catch (Exception e) {
365-
throw new MessageHandlingException(requestMessage, "Failed to clone payload object", e);
362+
throw new MessageHandlingException(requestMessage,
363+
"Failed to clone payload object in the [" + this + ']', e);
366364
}
367365
}
368366
else {
@@ -400,7 +398,7 @@ protected Object handleRequestMessage(Message<?> requestMessage) {
400398
return targetPayload;
401399
}
402400
else {
403-
Map<String, Object> targetHeaders = new HashMap<String, Object>(
401+
Map<String, Object> targetHeaders = new HashMap<>(
404402
this.nullResultHeaderExpressions.size());
405403
for (Map.Entry<String, HeaderValueMessageProcessor<?>> entry : this.nullResultHeaderExpressions
406404
.entrySet()) {

spring-integration-file/src/main/java/org/springframework/integration/file/FileWritingMessageHandler.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -503,7 +503,8 @@ protected Object handleRequestMessage(Message<?> requestMessage) {
503503
boolean exists = resultFile.exists();
504504
if (exists && FileExistsMode.FAIL.equals(this.fileExistsMode)) {
505505
throw new MessageHandlingException(requestMessage,
506-
"The destination file already exists at '" + resultFile.getAbsolutePath() + "'.");
506+
"Failed to process message in the [" + this
507+
+ "]. The destination file already exists at '" + resultFile.getAbsolutePath() + "'.");
507508
}
508509

509510
Object timestamp = requestMessage.getHeaders().get(FileHeaders.SET_MODIFIED);
@@ -528,7 +529,7 @@ protected Object handleRequestMessage(Message<?> requestMessage) {
528529
}
529530
catch (Exception e) {
530531
throw IntegrationUtils.wrapInHandlingExceptionIfNecessary(requestMessage,
531-
() -> "failed to write Message payload to file", e);
532+
() -> "failed to write Message payload to file in the [" + this + ']', e);
532533
}
533534
}
534535

spring-integration-file/src/main/java/org/springframework/integration/file/remote/gateway/AbstractRemoteFileOutboundGateway.java

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -543,9 +543,9 @@ private Object doGet(final Message<?> requestMessage) {
543543
payload = session.readRaw(remoteFilePath);
544544
}
545545
catch (IOException e) {
546-
throw new MessageHandlingException(requestMessage, "Failed to get the remote file ["
547-
+ remoteFilePath
548-
+ "] as a stream", e);
546+
throw new MessageHandlingException(requestMessage,
547+
"Error handling message in the [" + this
548+
+ "]. Failed to get the remote file [" + remoteFilePath + "] as a stream", e);
549549
}
550550
}
551551
else {
@@ -984,7 +984,8 @@ else if (FileExistsMode.REPLACE_IF_MODIFIED.equals(existsMode)) {
984984
}
985985
}
986986
else if (!FileExistsMode.IGNORE.equals(existsMode)) {
987-
throw new MessageHandlingException(message, "Local file " + localFile + " already exists");
987+
throw new MessageHandlingException(message,
988+
"Error handling message in the [" + this + "]. Local file " + localFile + " already exists");
988989
}
989990
else {
990991
if (logger.isDebugEnabled()) {

spring-integration-file/src/main/java/org/springframework/integration/file/splitter/FileSplitter.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,8 @@ protected Object splitMessage(final Message<?> message) {
180180
filePath = (String) payload;
181181
}
182182
catch (FileNotFoundException e) {
183-
throw new MessageHandlingException(message, "failed to read file [" + payload + "]", e);
183+
throw new MessageHandlingException(message,
184+
"Error handing message in the [" + this + "]. Failed to read file [" + payload + "]", e);
184185
}
185186
}
186187
else if (payload instanceof File) {

spring-integration-file/src/test/java/org/springframework/integration/file/config/FileOutboundGatewayParserTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ public void gatewayWithFailMode() throws Exception {
181181

182182
assertThatExceptionOfType(MessageHandlingException.class)
183183
.isThrownBy(() -> messagingTemplate.sendAndReceive(new GenericMessage<>("String content:")))
184-
.withMessageStartingWith("The destination file already exists at '");
184+
.withMessageContaining("The destination file already exists at '");
185185
}
186186

187187
/**
@@ -208,7 +208,7 @@ public void gatewayWithFailModeLowercase() throws Exception {
208208

209209
assertThatExceptionOfType(MessageHandlingException.class)
210210
.isThrownBy(() -> messagingTemplate.sendAndReceive(new GenericMessage<>("String content:")))
211-
.withMessageStartingWith("The destination file already exists at '");
211+
.withMessageContaining("The destination file already exists at '");
212212
}
213213

214214
/**

spring-integration-file/src/test/java/org/springframework/integration/file/splitter/FileSplitterTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ public void testFileSplitter() throws Exception {
162162
}
163163
catch (Exception e) {
164164
assertThat(e.getCause()).isInstanceOf(FileNotFoundException.class);
165-
assertThat(e.getMessage()).contains("failed to read file [bar]");
165+
assertThat(e.getMessage()).contains("Failed to read file [bar]");
166166
}
167167
this.input2.send(new GenericMessage<>(new Date()));
168168
receive = this.output.receive(10000);

0 commit comments

Comments
 (0)