Fixed exceptions when fetching webhook list "sticking" in the cache

This happened because we cache Task instances, not the values, and a failed task is still a *valid* task that can repeatedly be awaited. This lead to the Task being saved even if it failed, and the same exception constantly being re-thrown.

This fix invalidates the cache if it finds a failed Task, and lets it retry fetching as normal (hopefully successfully this time).
This commit is contained in:
Ske 2020-06-11 16:13:18 +02:00
parent e7191def02
commit 3d69a00716

View File

@ -40,9 +40,26 @@ namespace PluralKit.Bot
// We cache the webhook through a Lazy<Task<T>>, this way we make sure to only create one webhook per channel // We cache the webhook through a Lazy<Task<T>>, this way we make sure to only create one webhook per channel
// If the webhook is requested twice before it's actually been found, the Lazy<T> wrapper will stop the // If the webhook is requested twice before it's actually been found, the Lazy<T> wrapper will stop the
// webhook from being created twice. // webhook from being created twice.
var lazyWebhookValue = Lazy<Task<DiscordWebhook>> GetWebhookTaskInner()
_webhooks.GetOrAdd(channel.Id, new Lazy<Task<DiscordWebhook>>(() => GetOrCreateWebhook(channel))); {
Task<DiscordWebhook> Factory() => GetOrCreateWebhook(channel);
return _webhooks.GetOrAdd(channel.Id, new Lazy<Task<DiscordWebhook>>(Factory));
}
var lazyWebhookValue = GetWebhookTaskInner();
// If we've cached a failed Task, we need to clear it and try again
// This is so errors don't become "sticky" and *they* in turn get cached (not good)
// although, keep in mind this block gets hit the call *after* the task failed (since we only await it below)
if (lazyWebhookValue.IsValueCreated && lazyWebhookValue.Value.IsFaulted)
{
_logger.Warning(lazyWebhookValue.Value.Exception, "Cached webhook task for {Channel} faulted with below exception");
// Specifically don't recurse here so we don't infinite-loop - if this one errors too, it'll "stick"
// until next time this function gets hit (which is okay, probably).
_webhooks.TryRemove(channel.Id, out _);
lazyWebhookValue = GetWebhookTaskInner();
}
// It's possible to "move" a webhook to a different channel after creation // It's possible to "move" a webhook to a different channel after creation
// Here, we ensure it's actually still pointing towards the proper channel, and if not, wipe and refetch one. // Here, we ensure it's actually still pointing towards the proper channel, and if not, wipe and refetch one.
var webhook = await lazyWebhookValue.Value; var webhook = await lazyWebhookValue.Value;