Skip to content

Warn about dropped message in filter #8579

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

Merged
merged 3 commits into from
Mar 20, 2023
Merged

Warn about dropped message in filter #8579

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
16f81bc
Warn about dropped message in filter
artembilan Mar 20, 2023
571543a
* Mention `nullChannel` variant to ignore even warn
artembilan Mar 20, 2023
6a436fb
Fix language in docs
artembilan Mar 20, 2023
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
24 changes: 14 additions & 10 deletions ...-integration-core/src/main/java/org/springframework/integration/filter/MessageFilter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2002-2020 the original author or authors.
* Copyright 2002-2023 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 @@ -20,6 +20,7 @@
import org.springframework.beans.factory.BeanFactoryAware;
import org.springframework.context.Lifecycle;
import org.springframework.core.convert.ConversionService;
import org.springframework.core.log.LogMessage;
import org.springframework.integration.IntegrationPatternType;
import org.springframework.integration.MessageRejectedException;
import org.springframework.integration.core.MessageSelector;
Expand All @@ -33,11 +34,11 @@
/**
* Message Handler that delegates to a {@link MessageSelector}. If and only if
* the selector {@link MessageSelector#accept(Message) accepts} the Message, it
* will be passed to this filter's output channel. Otherwise the message will
* either be silently dropped (the default) or will trigger the throwing of a
* {@link MessageRejectedException} depending on the value of its
* {@link #throwExceptionOnRejection} property. If a discard channel is
* provided, the rejected Messages will be sent to that channel.
* will be passed to this filter's output channel. Otherwise, the message will
* either be silently dropped (the default) with a warning into logs,
* or will trigger the throwing of a {@link MessageRejectedException}
* depending on the value of its {@link #throwExceptionOnRejection} property.
* If a discard channel is provided, the rejected Messages will be sent to that channel.
*
* @author Mark Fisher
* @author Oleg Zhurakousky
Expand Down Expand Up @@ -71,7 +72,7 @@ public MessageFilter(MessageSelector selector) {
* {@link MessageRejectedException} when its selector does not accept a
* Message. The default value is <code>false</code> meaning that rejected
* Messages will be quietly dropped or sent to the discard channel if
* available. Typically this value would not be <code>true</code> when
* available. Typically, this value would not be <code>true</code> when
* a discard channel is provided, but if so, it will still apply
* (in such a case, the Message will be sent to the discard channel,
* and <em>then</em> the exception will be thrown).
Expand Down Expand Up @@ -179,13 +180,16 @@ protected Object doHandleRequestMessage(Message<?> message) {
@Override
public Object postProcess(Message<?> message, Object result) {
if (result == null) {
MessageChannel channel = getDiscardChannel();
if (channel != null) {
this.messagingTemplate.send(channel, message);
MessageChannel channelToDiscard = getDiscardChannel();
if (channelToDiscard != null) {
this.messagingTemplate.send(channelToDiscard, message);
}
if (this.throwExceptionOnRejection) {
throw new MessageRejectedException(message, "message has been rejected in filter: " + this);
}
else if (channelToDiscard == null) {
logger.warn(LogMessage.format("The message [%s] has been rejected in filter: %s", message, this));
Copy link
Contributor

Choose a reason for hiding this comment

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

WARN seems too strong; this might be the desired behavior; I would suggest DEBUG, perhaps with an option to increase the log level.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK. As a middle group I can agree for an INFO, which seems a default one for Spring Boot.

Sure! It can be desired behavior when you know what you do.
But if your expectation is request-reply and you miss the fact that simple filter() in the middle of the flow just drops messages silently, it would be great if framework notifies you some way what is going on instead of silence.
Another way I'd suggest is to configure a nullChannel as a discardChannel if you'd like to ignore everything silently.

I also can go more aggressive path and make a throwExceptionOnRejection as default behavior.
So, when we develop everything in default, there is no any silence.

FYI: the next my change will be to fix gateway for its infinite default reply timeout (After you merge that doc PR).
The recommendation in modern design is to never block forever.
This is going to be another signal in the mentioned request-reply and filter scenario that something is not OK or misconfigured 😄 .

Copy link
Contributor

Choose a reason for hiding this comment

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

I have no objection to requiring nullChannel, if it's only a 6.1 change.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it is. Let me add such a sentence into doc and you are feel free to merge thereafter!

}
}
return result;
}
Expand Down
4 changes: 4 additions & 0 deletions src/reference/asciidoc/filter.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,10 @@ If you want rejected messages to be routed to a specific channel, provide that r
----
====

If the `throwExceptionOnRejection == false` and no `discardChannel` is provided, the message is silently dropped and an `o.s.i.filter.MessageFilter` instance just emits a warning log message (starting with version 6.1) about this discarded message.
To drop the message with no warning in the logs, a `NullChannel` can be configured as the `discardChannel` on the filter.
The goal of the framework is to not be completely silent, by default, requiring an explicit option to be set, if that is the desired behavior.

See also <<./handler-advice.adoc#advising-filters,Advising Filters>>.

NOTE: Message filters are commonly used in conjunction with a publish-subscribe channel.
Expand Down
2 changes: 2 additions & 0 deletions src/reference/asciidoc/whats-new.adoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ See <<./zip.adoc#zip,Zip Support>> for more information.
- Added support for transforming to/from Protocol Buffers.
See <<./transformer.adoc#Protobuf-transformers, Protocol Buffers Transformers>> for more information.

- The `MessageFilter` now emits a warning into logs when message is silently discarded and dropped.
See <<./filter.adoc#filter, Filter>> for more information.

[[x6.1-web-sockets]]
=== Web Sockets Changes
Expand Down