Retries when punishing plugins

[mariusandra]

New issue for thoughts and future work (retries) on merged PR #380

One change (nit?) I would have done is to pass pluginConfig to disablePlugin directly, instead of via plugin_id and team_id. This would also simplify the SQL query, since we would just need to check for id = pluginConfig.id.

Regarding the points in the PR:

Do we want to allow users to re-enable plugins that failed on setup even though they haven't changed at all? i.e. prevent a user from keeping on enabling a plugin that we know will fail.

If the API my plugin talks to fails for reasons outside of my control (or in control, but external, e.g. not enough credits for an API), I'd like to re-enable the plugin without needing to reinstall it :). So yes.

Do we want a retry strategy for setupPlugin? How many retries?

I think we do. With the merged implementation we will accidentally punish good plugins. Knowing how the system works after that PR, I would be very careful with making any network requests inside setupPlugin, as any accidental network glitch (api is down for 5min) will disable the plugin completely. Basically, if my plugin won't start because "Your account is suspended due to lack of payment method.", I'd like it to try again at a later date. However if my plugin won't start because "invalid api key", I'd like it to be totally disabled.

I think the answer is to put this on plugin authors. You either throw a RetryError (will be retried) or anything else (will be disabled). That's what Segment does at least.

[neilkakkar]

I like the idea of segregating errors and pushing that onto the creators. I'm guessing even with RetryError, we'll have our own upper limit to prevent misuse?

If the API my plugin talks to fails for reasons outside of my control (or in control, but external, e.g. not enough credits for an API), I'd like to re-enable the plugin without needing to reinstall it :). So yes.

What does re-enabling the plugin without needing to re-install it mean?

[mariusandra]

This was for a case when we would prevent the plugin from being re-enabled after it errored, by hashing its source/version/config and tagging that with the failure. However sometimes you just want to try running the same thing again.

[mariusandra]

And yes, for RetryError either here or in for example processEvent, we'll need an upper bound for retries. Something like 24h of retries with exponential backoff.

It might be worth reading what Segment does:

• https://segment.com/docs/connections/functions/destination-functions/#errors-and-error-handling
• https://segment.com/docs/connections/functions/destination-functions/#does-segment-retry-failed-function-invocations