Make Service Bus attempt to reconnect forever in case of issues

[ItalyPaleAle]

Description

With exponential back-off configurable between min and max time. Fixes #1612

Also includes fixes:

• Binding: make sure it actually retries to connect forever
• Binding: add delay (exponential backoff) before reconnecting
• PubSub: better handling of failures such as topics disabled or other non-connection issues

Issue reference

Fixes #1612

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

• Code compiles correctly
• Created/updated tests
• Extended the documentation Updated metadata for Service Bus binding/pubsub docs#2512

[berndverst]

This is a breaking change - see https://docs.dapr.io/reference/components-reference/supported-pubsub/setup-azure-servicebus/

[berndverst]

This is a breaking change as the metadata options has changed and existing metadata options no longer take effect. That needs to be handled.

[ItalyPaleAle]

It sort if is, and sort of isn't.

The additional metadata keys are ignored so that won't cause crashes.

Now, it's true that it changes the behavior, but I don't believe we should be particularly concerned with that, because the current behavior is incorrect/undesirable.

Per linked issue (user-reported), the pubsub component would only try to reconnect up to maxReconnectionAttempts. After that, it would enter in a state in which nothing is happening. The runtime doesn't crash, but it doesn't receive any message. This is arguably not a desirable behavior and it should be ok to remove it.

For connectionRecoveryInSec, with the change to make the component retry forever, it makes sense to have a min and max instead. Although users that have set this value before may see it ignored and see a different behavior, it shouldn't be a significant impact, as it just regulates the reconnection interval (and before it was only used up to the maxReconnectionAttempts).

[berndverst]

Behavior change is a breaking change @ItalyPaleAle. Of course I know that metadata keys which don't exist are ignored.

I wonder if there is any situation where a user previously benefited from retries and would not be broken as a result. Is there something better we can do here?

Maybe at a minimum we just write a log message that the metadata option is ignored and other metadata options (the new ones you have added) should be used instead?

[ItalyPaleAle]

Maybe at a minimum we just write a log message that the metadata option is ignored and other metadata options (the new ones you have added) should be used instead?

that's a good suggestion

[ItalyPaleAle]

Changed as requested

[codecov]

Codecov Report

Merging #1783 (d8724af) into master (4322a22) will decrease coverage by 0.08%.
The diff coverage is 20.17%.

@@            Coverage Diff             @@
##           master    #1783      +/-   ##
==========================================
- Coverage   36.59%   36.51%   -0.09%     
==========================================
  Files         177      177              
  Lines       16222    16278      +56     
==========================================
+ Hits         5937     5944       +7     
- Misses       9617     9661      +44     
- Partials      668      673       +5     

Impacted Files	Coverage Δ
pubsub/azure/servicebus/subscription.go	0.00% <0.00%> (ø)
pubsub/rabbitmq/rabbitmq.go	55.39% <0.00%> (-2.31%)	⬇️
state/postgresql/postgresdbaccess.go	37.05% <0.00%> (-1.56%)	⬇️
...indings/azure/servicebusqueues/servicebusqueues.go	11.59% <14.28%> (+0.89%)	⬆️
pubsub/azure/servicebus/servicebus.go	27.03% <17.77%> (-0.65%)	⬇️
pubsub/rabbitmq/metadata.go	93.93% <100.00%> (+0.39%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1db4173...d8724af. Read the comment docs.