[service-bus] Error handling needs work to be consistent and reliable

[richardpark-msft]

As part of a recent refactor it's become more obvious that the error handling in Service Bus needs to be made more consistent. There are some good classification functions in the older code (which needs to be brought forward) and basically we just need to have enough information to say:

• This link is dead and needs to restart
• There is a retryable issue here (transient) and we should just try it again

This affects the Receiver and Sender a bit more since the Processor doesn't really make any distinctions about what's retryable vs not.

Tagging @jhendrixMSFT on this as we've had discussion about how we might provide some interfaces or similar to make the classification easier for client code.

[richardpark-msft]

I'm taking a first stab at this for beta 1 (October release).

The big things:

• The processor type brings in built-in infinite retrying, which should be more durable than the previous APIs.
• Our error classification has been improved - much thanks to the go-shuttle project (and @serbrech). I've added in some further checking and also allow for link and connection level recovery (we fallback to connection more than I'd like, but it's a first go).

As a first beta release bar I've got a stress test running, with network outages every minute for 5 seconds. This gives us some decent recovery testing, while also getting a good ballpark on our longevity.

There are a few spots that are being string-matched rather than doing a type/interface check. We need to work to eliminate these.

[richardpark-msft]

Some of this work has been incorporated by not requiring the user to intervene. We still report the error but the next call to Receiver.ReceiveMessages() will repair it.

We still want to add in some classification, as that will help customers if they want to decide to make that next call or not,.

[richardpark-msft]

Note, another interesting discussion was here:
Azure/azure-service-bus-go#96

When applicable, we should return a structured error. The equivalent of a 404 here would be useful.

[ryepup]

My team is working on a servicebus integration, and running into a problem differentiating recoverable and unrecoverable errors.

Specifically we're writing a function to ensure a topic is defined, calling GetTopic to see if it exists, and a CreateTopic if needed. We're not seeing an official way to differentiate between recoverable "topic not found" errors and unrecoverable network errors.

resp, err := client.GetTopic(ctx, "topic", &admin.GetTopicOptions{})
if err != nil {
  // err could be an unrecoverable auth/network/etc problem
  // or a recoverable 404 because the topic is not defined
}

There's an internal atom.NotFound(err) helper to differentiate these error cases, but that's not exported. Currently we're using string comparisons on err.Error(), but that feels brittle; the specific error messages are not part of the exported API and we'd like to only rely on documented behavior. We have a similar situation with GetSubscription/CreateSubscription.

It'd be great if y'all could export something like feedEmptyError, NotFound(err), or any other mechanism to enable this logic.

[richardpark-msft]

@ryepup, I'll need to look at it a bit more. The admin client could use some more work to get it consistent with how the rest of the Azure SDK's http based clients work, which should involve exporting a standard error type to look at the HTTP resp and status code, directly.

[ryepup]

@richardpark-msft including a status code would be great

[ryepup]

Thanks for the fixes, definitely will solve my problem!

Any idea when the changes from #16831 will get a release?