GoogleCloudPlatform / GoogleCloudPlatform/spring-cloud-gcp

Give users of PubSubTemplate control over conversion exception handling

Open
#887 8 comments 7 reactions 0 assignees View on GitHub
priority: p3 pubsub type: enhancement
Dominant language
Java
Stars
551
Forks
349
Avg merge
1d 13h
Merged PRs (30d)
14

Description

We are currently using _spring-cloud-gcp-pubsub-2.0.2_ but as far as we could see, this functionality has not changed in _3.0.0_.
In our setup, we use the _PubSubTemplate_ with a _JacksonPubSubMessageConverter_ to let our _Consumers_ directly work with deserialized POJOs which is working great.

But we figured that the handling of erroneous messages is quite problematic. Per default, every message that could not be processed successfully (or even not fast enough) will be retried automatically by the subscription. And for us, most cases of unprocessable messages are caused by simply invalid messages that will always be unprecessable, no matter how often they are retried.
For these that are transformed into Java objects, we can simply handle that by logging the error (to trigger an investigation and resolution of the root cause) and returning the ACK signal in our _Consumer_.
But when the conversion of the message fails, the framework will automatically send NACK and thus trigger the retry mechanism.

We thought about using a deal-letter-policy, but we figured that it should not be necessary to have messages been tossed around multiple services if we already know that they cannot be processed at all.

We identified the cause located in _PubSubSubscriberTemplate#subscribeAndConvert_ where a lambda is registered as _MessageReceiver_ that calls the _Consumer_ with the converted payload. This _MessageReceiver_ is called by the _MessageDispatcher_ which will NACK the message on any failure. We found a "solution" by customizing the _SubscriberFactory_ and having it wrap the registered _MessageReceiver_ to catch the _PubSubMessageConversionException_ and marking the message as ACKed. But the structure of the respective code in the library classes seem to mediate the notion that such tweeks are not intendet at that point and certainly not recommended.

Most of all, this does work for _subscribeAndConvert_ but not for _pullAndConvert_, since there the caller will get the _PubSubMessageConversionException_ and gets no chance to mark the message with ACK nonetheless.

So we are worried that our approach might not be a good idea and we have maybe overseen something important here.
We would have thought that our use case should be quite common, but everything we find (or do not find) seems to contradict that.

**Describe the solution you'd like**
If you agree with us that there is no gain in retrying non-convertible messages, then there should be an easier way to differentiate the handling of conversion errors and processing errors. Especially, there should be a means to have conversion errors logged (or given to a customizable handler) and the message to be ACKed, applicable both to _subscribeAndConvert_ and _pullAndConvert_.

**Describe alternatives you've considered**
If there is another recommended way to handle this use case, we would really appreciate some advise.

Contributor guide

Open the contributing guide

Research direction

Start with PubSubSubscriberTemplate#subscribeAndConvert and the MessageReceiver/MessageDispatcher flow, then compare how pullAndConvert exposes PubSubMessageConversionException. Review the SubscriberFactory customization described in the issue and define a consistent, customizable outcome for conversion failures in both paths, with conversion errors handled separately from processing errors.

Written by the indexing model from the issue text.

Assessment

Tech stack
google-cloud, java, spring
Domain
backend, distributed-systems
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
38/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.