Skip to content
This repository has been archived by the owner on Jan 19, 2022. It is now read-only.

PubsubInboundChannelAdapter swallows error when connected to non-existent subscription #82

Closed
balopat opened this issue Jul 28, 2017 · 5 comments
Assignees
Milestone

Comments

@balopat
Copy link
Contributor

balopat commented Jul 28, 2017

Currently if I connect a PubsubInboundChannelAdapter to a non-existent subscription, it silently fails.
We could change this behaviour by changing this line:

    protected void doStart() {
        super.doStart();
        this.subscriber = this.subscriberFactory.getSubscriber(this.subscriptionName, this::receiveMessage);
        this.subscriber.startAsync(); // <---- this fails silently. 
    }

We could add an .awaitRunning() to ensure that when the subscriber started that it is actually running.

@artembilan
Copy link
Contributor

The .awaitRunning() is not good. It eliminates the purpose of the startAsync().

Would be better to use void addListener(ApiService.Listener var1, Executor var2); to log in case of
failed(ApiService.State from, Throwable failure)

@joaoandremartins
Copy link
Contributor

joaoandremartins commented Jul 28, 2017

Another (more expensive, but more explicit) alternative is to make an explicit check for the existence of the Subscription using the getSubscription() method. We could also create the Subscription if it doesn't exist.

This behaviour should be parameterised.

@balopat
Copy link
Contributor Author

balopat commented Jul 28, 2017

@artembilan logging sounds good, however, that still doesn't solve the need from the client code to have an explicit way to inspect/handle subscription errors and have the option to fail/retry/ignore.
@joaoandremartins I like the idea of having the option to auto-create the subscription.

I guess we could combine the two suggested behaviors: we listen on the lifecycle events and "on failure" we can inspect if the error is due to missing subscription ("NOT_FOUND: Resource not found (resource=[subscription])." and if that's the case we can have a configurable behavior + a status accessible from the client side probably via a status listener?

@artembilan
Copy link
Contributor

Well, in other places we expose the ApplicationEvent architecture to let end-user to do what is the best for him there. For example ListenerContainerConsumerFailedEvent in the Spring AMQP: http://docs.spring.io/spring-amqp/reference/html/_reference.html#consumer-events. Or MqttIntegrationEvent hierarchy: http://docs.spring.io/spring-integration/reference/html/mqtt.html#mqtt-inbound

@meltsufin meltsufin modified the milestone: 1.0.0.M1 Aug 10, 2017
@saturnism saturnism modified the milestones: 1.0.0.M1, 1.0.0.M2 Oct 13, 2017
@joaoandremartins joaoandremartins self-assigned this Oct 18, 2017
@joaoandremartins
Copy link
Contributor

This was apparently fixed in a recent version of the google-cloud-pubsub library, as I'm now seeing errors logged when trying to subscribe to a non-existing subscription.

meltsufin pushed a commit that referenced this issue Nov 11, 2020
* Pivotal --> Google in pom files, capitalize GCP
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

5 participants