From 3d69a00716d7899e99ae736d28ae0c7396f9d745 Mon Sep 17 00:00:00 2001 From: Ske Date: Thu, 11 Jun 2020 16:13:18 +0200 Subject: [PATCH] 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). --- PluralKit.Bot/Services/WebhookCacheService.cs | 21 +++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/PluralKit.Bot/Services/WebhookCacheService.cs b/PluralKit.Bot/Services/WebhookCacheService.cs index 873f8eba..8967ee61 100644 --- a/PluralKit.Bot/Services/WebhookCacheService.cs +++ b/PluralKit.Bot/Services/WebhookCacheService.cs @@ -40,9 +40,26 @@ namespace PluralKit.Bot // We cache the webhook through a Lazy>, 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 wrapper will stop the // webhook from being created twice. - var lazyWebhookValue = - _webhooks.GetOrAdd(channel.Id, new Lazy>(() => GetOrCreateWebhook(channel))); + Lazy> GetWebhookTaskInner() + { + Task Factory() => GetOrCreateWebhook(channel); + return _webhooks.GetOrAdd(channel.Id, new Lazy>(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 // Here, we ensure it's actually still pointing towards the proper channel, and if not, wipe and refetch one. var webhook = await lazyWebhookValue.Value;