GH-3454: From MQTT conversion error - to error ch #3456
Conversation
Fixes spring-projects#3454 The message converter may return null when we try to covert from the MQTT message. The thrown exception may also reset the client connect. * Fix `MqttPahoMessageDrivenChannelAdapter` to catch any conversion errors (including `null` result) and try to send an `ErrorMessage` with that info into the provided `errorChannel`. Otherwise re-throw it as as **Cherry-pick to `5.4.x` & `5.3.x`**
|
I will do back-port to Thanks |
...n/java/org/springframework/integration/mqtt/inbound/MqttPahoMessageDrivenChannelAdapter.java
Outdated
Show resolved
Hide resolved
| conversionException = new MessageConversionException(message, "Failed to convert from MQTT Message", | ||
| conversionError); | ||
| } | ||
| throw conversionException; |
There was a problem hiding this comment.
Should we still throw here? Or just log as ERROR?
There was a problem hiding this comment.
Well, I tried to make it consistent with what we have so far in the MessageProducerSupport.sendMessage():
try {
this.messagingTemplate.send(getRequiredOutputChannel(), message);
}
catch (RuntimeException ex) {
if (!sendErrorMessageIfNecessary(message, ex)) {
throw ex;
}
}
We probably may consider some general ErrorHandler strategy. However it looks like we have it already for this kind of channel adapters via an ErrorHandlingTaskExecutor impl. Nevertheless I don't see that we have some way to inject it into the MqttAsyncClient...
Probably the matter for a separate issue and discussion over there...
There was a problem hiding this comment.
Right, but the problem here is that throwing an exception causes the client to close. Conversion exceptions are generally fatal anyway (no point in retrying) since a redelivery will fail again.
However, it would be a behavior change so we'd probably have to add a flag.
Or, we can just leave it as you have it; up to you.
There was a problem hiding this comment.
Well, we have a flag - errorChannel and no exceptions are thrown back to the client!
I would stay with this behavior for a while: probably everybody is going to be happy to swallow and process those wrong messages in the error flow instead of just silent (log) drop to satisfy the client connection stability.
|
let me know if anything else you'd like to see fixed with this PR! |
* GH-3454: From MQTT conversion error - to error ch Fixes #3454 The message converter may return null when we try to covert from the MQTT message. The thrown exception may also reset the client connect. * Fix `MqttPahoMessageDrivenChannelAdapter` to catch any conversion errors (including `null` result) and try to send an `ErrorMessage` with that info into the provided `errorChannel`. Otherwise re-throw it as as **Cherry-pick to `5.4.x` & `5.3.x`** * * Apply review language-specific changes
* GH-3454: From MQTT conversion error - to error ch Fixes #3454 The message converter may return null when we try to covert from the MQTT message. The thrown exception may also reset the client connect. * Fix `MqttPahoMessageDrivenChannelAdapter` to catch any conversion errors (including `null` result) and try to send an `ErrorMessage` with that info into the provided `errorChannel`. Otherwise re-throw it as as **Cherry-pick to `5.4.x` & `5.3.x`** * * Apply review language-specific changes
|
Cherry-picked to 5.4.x and to 5.3.x after resolving conflicts. Ready for 5.2.x backport. |
* GH-3454: From MQTT conversion error - to error ch Fixes #3454 The message converter may return null when we try to covert from the MQTT message. The thrown exception may also reset the client connect. * Fix `MqttPahoMessageDrivenChannelAdapter` to catch any conversion errors (including `null` result) and try to send an `ErrorMessage` with that info into the provided `errorChannel`. Otherwise re-throw it as as **Cherry-pick to `5.4.x` & `5.3.x`** * * Apply review language-specific changes # Conflicts: # spring-integration-mqtt/src/main/java/org/springframework/integration/mqtt/inbound/MqttPahoMessageDrivenChannelAdapter.java # spring-integration-mqtt/src/test/java/org/springframework/integration/mqtt/MqttAdapterTests.java # src/reference/asciidoc/mqtt.adoc
|
Back-ported to |
Fixes #3454
The message converter may return null when we try to covert from the
MQTT message.
The thrown exception may also reset the client connect.
MqttPahoMessageDrivenChannelAdapterto catch any conversion errors(including
nullresult) and try to send anErrorMessagewith that infointo the provided
errorChannel.Otherwise re-throw it as as
Cherry-pick to
5.4.x&5.3.x