Retry Policy

[georgehemmings]

It would be nice if retry policies could be specified. Currently it's up to each provider to come up with it's own way of handling retries. There are some scenarios that could be handled better if the plugin handled retries rather than the provider.

For example if an endpoint cannot be reached it would be sensible to not attempt to send any other events until the first event succeeds or fails completely (after all available retries are exhausted).

Moving the retry policy into the plugin would allow reuse of common webhook logic. For example try every minute for 10 minutes then every hour for 10 hours. The retries would also be visible via the subscription history. This may help the receiver debug their server app.

I've not thought too much about how this could be implemented generically yet. I'm opening this issue to start the discussion.

[jezzsantos]

Nice one @georgehemmings, just wanted to add some coordinates here for those participating in this discussion. (I support what @georgehemmings is after, and I think it does help for others to understand how things work presently - so all the pieces in play are fully disclosed.)

Currently, the 'Relay' plugins use the following flow to POST events to subscribers: IEventSink.Sink() -> (some store and forward mechanism) -> IEventServiceClient.Relay() -> IServiceClient.Post(). Then the result of the POST are collected as a SubscriptionDeliveryResult.

Most of the heavy lifting is done by the EventServiceClient and the Relay method, that has a configurable Retries and Timeout values.

EventServiceClient works like this (see Tests) :

It will try to POST the event to the subscribers URL. If it succeeds first time, it returns. If it fails, see below for reasons why, then it will try up to the number of Retries and wait for the 'Timeout' before reporting back.

Successes:

• If it gets a 200-300 it considers this success and records the result as the actual StatusCode.

Failures:

• If it receives a 4XX status code (i.e. BadRequest, NotFound, Unauthorized, Forbidden etc.), it does not retry any longer, it fails immediately and records that result in a SubscriptionDeliveryResult.
• If it gets a 5XX (i.e. InternalServerError, ServerUnavailable), it will immediately retry again up to the number of 'Retries', then fail and records the last result (using last StatusCode).
• If it gets a timeout, it will immediately retry again up to the Retries, then fail and record the result (using last StatusCode)

All this is already built-into the EventServiceClient, and can be used (or not) by any 'Relay'. If would recommend that any new pattern we come up with has at least this minimum set of rudimentary behavior, or similar.

[grexican]

Just throwing a quick comment into the mix. It sounds like removing the retry attempt count + retry timespan and replacing it with the Strategy pattern would get people what they need. ImmediateRetryStrategy(int maxAttempts, int timeoutSeconds) would be the default to fit the requirements above. Then other ones like: RetryEveryMinuteStrategy(int maxAttempts); ExponentialBackupStrategy(bool forever = true); etc. Then you could custom cook your own retry strategies, and even swap them out at runtime with different strategies for different subscriptions -- though I'm reading the docs here for the first time... I'm not sure where to 'hook' (hah!) that up.