Pubsub API should not swallow exceptions by default

[gcbirzan]

Current behaviour, albeit documented, is to ignore errors in publishing. They will, eventually, maybe, bubble up in a different thread, but this behavious is surprising and useless. Most other APIs (in this library) are synchronous, to a fault. If nothing else, an option that doesn't say "only useful for debugging" should be provided (talking about autocommit).

[theacodes]

@lukesneeringer would like your opinion on this as well

[lukesneeringer]

I agree with this, but need to determine what the correct behavior is. One problem is that we may not get errors in all situations.

[gcbirzan]

If you block until a certain timeout, wouldn't that guarantee to either get a timeout/exception or a success?

[lukesneeringer]

Sort of. There is a world where you publish N messages and Pub/Sub only publishes N - 1 (really N - M) of them. The only way you know is because the length of the message IDs does not match (and you have no way of knowing which one failed).

[gcbirzan]

But, if you publish a batch of messages, you know which ones you published, and which ones you got back as being published. If anything, at least return ONE error. However, this is an edge case. In most cases, it's not that "half the messages are published", it's "oops, you screwed up IAM", or "oops pubsub is down".

[theacodes]

The other question is when do we bubble the exceptions?

If we implement #4913 as a context manager, it becomes obvious:

try:
    with publisher.batch():
        ...
except:
    # publish errors bubble here.
    pass

With the current "fire and forget" approach, we only bubble errors on the future returned:

future = publisher.publish(...)

try:
    future.result()
except:
    # publish errors bubble here.
    pass

Like luke said, the only real situation where this happens currently is if pubsub publishes < N messages in a batch (code), which should be truly exceptional.

I think our problem stems that we don't catch and bubble errors from this call. We should do that at a minimum, but that still leaves the question of where and when we should bubble those errors on the main thread.

[theacodes]

you know which ones you published, and which ones you got back as being published

Actually we don't, there's no identifying information on the outgoing messages. If all is good, pubsub returns the message IDs in the same order as the sent messages. We zip those together here.

[brunoais]

Is this discussion about exceptions that may be thrown by response = self._client.api.publish( self._topic, self._messages, ):202?
I'm currently wanting to catch the exception about inexistent topic and I'm being unable to do that because it is being bubbled inside a thread to which I don't have control of instead of being bubbled through the future which was returned at the call of pubsub_v1.PublisherClient(...).publish(...)

[theacodes]

@brunoais #5124 should solve that.

[brunoais]

@jonparrott It is much better now. Unfortunately, it still does fill the logs with a quite considerable stack trace which I can't selectively turn off. I can only have it either "on" or "off" for everything, unfortunately.
Additionally, unfortunately because there's data missing in the log, I have a very hard time tying the fault with the large stack trace it is given to me.
Thankfully, though. I can now add my own callback to the Future which gives me a huge advantage in debugging from production as I can now know which calls fail... Even though I may have to check 5 medium-sized stack traces, 4 potentially of the same and 1 redundant.

Let me know when it goes to a stable version so I can test it. This is already a great step to making PubSub easier to use through this python API. Thanks.