[event-hubs] Error handling changes for SDK consistency

[richardpark-msft]

During our recent review for error handling in EventHubs a few spots showed up as being different than we'd like to change:

(will do)

1. When claiming ownership do NOT eat errors unless they are specifically related to fighting for ownership over a blob (ie, etag related). Eating them can conceal underlying problems (storage account accessibility, etc..). This should be the standard for the CheckpointStore contract overall.
2. User-thrown errors from the event handlers (except for processError) will be routed to the processError callback.

(might do)
There was an additional discussion about whether this should also terminate the processing loop - some languages do this. That is a larger issue that should be discussed with our architect (@bterlson)

[chradek]

Yes this looks correct to me. Reiterating...all our checkpointstore implementations should throw errors instead of eating them when calling claimOwnership, unless the failure is due to an etag mismatch (since that will get indirectly handled as EventProcessor should see an OwnershipLost eventually)

[bterlson]

I'll need more context to weigh in on the processing loop termination. Come find me to discuss 😀

[richardpark-msft]

I've got a PR out for the first two points since those seem universal.

[richardpark-msft]

Closing this now that the PR is complete.

After talking with @bterlson we weren't able to come up with a good reason in the current JS API why user errors would terminate the processing loop.

There isn't a great way for us to know which user errors might be considered fatal, which might be recoverable, etc.. and the cost of spinning down the loop and starting it up should be made by the user themselves.

PR#6555 addresses the first two points and the fact that processError is mandatory and their errors get routed to it should be enough to for the user to not mistakenly be failing in their event handlers and not notice.