Update BucketId and redirect requests

src/Discord.Net.Core/Net/BucketId.cs

@@ -5,41 +5,93 @@ using System.Linq;
 
 namespace Discord.Net
 {
+    /// <summary>
+    ///     Represents a ratelimit bucket.
+    /// </summary>
     public class BucketId : IEquatable<BucketId>
     {
+        /// <summary>
+        ///     Gets the http method used to make the request if available.
+        /// </summary>
         public string HttpMethod { get; }
+        /// <summary>
+        ///     Gets the endpoint that is going to be requested if available.
+        /// </summary>
         public string Endpoint { get; }
-        public IOrderedEnumerable<KeyValuePair<string, string>> MajorParams { get; }
+        /// <summary>
+        ///     Gets the major parameters of the route.
+        /// </summary>
+        public IOrderedEnumerable<KeyValuePair<string, string>> MajorParameters { get; }
+        /// <summary>
+        ///     Gets the hash of this bucket.
+        /// </summary>
+        /// <remarks>
+        ///     The hash is provided by Discord to group ratelimits.
+        /// </remarks>
         public string BucketHash { get; }
-
+        /// <summary>
+        ///     Gets if this bucket is a hash type.
+        /// </summary>
         public bool IsHashBucket { get => BucketHash != null; }
 
-        private BucketId(string httpMethod, string endpoint, IEnumerable<KeyValuePair<string, string>> majorParams, string bucketHash)
+        private BucketId(string httpMethod, string endpoint, IEnumerable<KeyValuePair<string, string>> majorParameters, string bucketHash)
         {
             HttpMethod = httpMethod;
             Endpoint = endpoint;
-            MajorParams = majorParams.OrderBy(x => x.Key);
+            MajorParameters = majorParameters.OrderBy(x => x.Key);
             BucketHash = bucketHash;
         }
 
+        /// <summary>
+        ///     Creates a new <see cref="BucketId"/> based on the
+        ///     <see cref="HttpMethod"/> and <see cref="Endpoint"/>.
+        /// </summary>
+        /// <param name="httpMethod">Http method used to make the request.</param>
+        /// <param name="endpoint">Endpoint that is going to receive requests.</param>
+        /// <param name="majorParams">Major parameters of the route of this endpoint.</param>
+        /// <returns>
+        ///     A <see cref="BucketId"/> based on the <see cref="HttpMethod"/>
+        ///     and the <see cref="Endpoint"> with the provided data.
+        /// </returns>
         public static BucketId Create(string httpMethod, string endpoint, Dictionary<string, string> majorParams)
         {
-            Preconditions.NotNullOrWhitespace(httpMethod, nameof(httpMethod));
             Preconditions.NotNullOrWhitespace(endpoint, nameof(endpoint));
             majorParams ??= new Dictionary<string, string>();
             return new BucketId(httpMethod, endpoint, majorParams, null);
         }
 
+        /// <summary>
+        ///     Creates a new <see cref="BucketId"/> based on a
+        ///     <see cref="BucketHash"/> and a previous <see cref="BucketId"/>.
+        /// </summary>
+        /// <param name="hash">Bucket hash provided by Discord.</param>
+        /// <param name="oldBucket"><see cref="BucketId"/> that is going to be upgraded to a hash type.</param>
+        /// <returns>
+        ///     A <see cref="BucketId"/> based on the <see cref="BucketHash"/>
+        ///     and <see cref="MajorParameters"/>.
+        /// </returns>
         public static BucketId Create(string hash, BucketId oldBucket)
         {
             Preconditions.NotNullOrWhitespace(hash, nameof(hash));
             Preconditions.NotNull(oldBucket, nameof(oldBucket));
-            return new BucketId(null, null, oldBucket.MajorParams, hash);
+            return new BucketId(null, null, oldBucket.MajorParameters, hash);
         }
 
+        /// <summary>
+        ///     Gets the string that will define this bucket as a hash based one.
+        /// </summary>
+        /// <returns>
+        ///     A <see cref="string"/> that defines this bucket as a hash based one.
+        /// </returns>
         public string GetBucketHash()
-            => IsHashBucket ? $"{BucketHash}:{string.Join("/", MajorParams.Select(x => x.Value))}" : null;
+            => IsHashBucket ? $"{BucketHash}:{string.Join("/", MajorParameters.Select(x => x.Value))}" : null;
 
+        /// <summary>
+        ///     Gets the string that will define this bucket as an endpoint based one.
+        /// </summary>
+        /// <returns>
+        ///     A <see cref="string"/> that defines this bucket as an endpoint based one.
+        /// </returns>
         public string GetUniqueEndpoint()
             => HttpMethod != null ? $"{HttpMethod} {Endpoint}" : Endpoint;
 
@@ -47,7 +99,7 @@ namespace Discord.Net
             => Equals(obj as BucketId);
 
         public override int GetHashCode()
-            =>  IsHashBucket ? (BucketHash, string.Join("/", MajorParams.Select(x => x.Value))).GetHashCode() : (HttpMethod, Endpoint).GetHashCode();
+            =>  IsHashBucket ? (BucketHash, string.Join("/", MajorParameters.Select(x => x.Value))).GetHashCode() : (HttpMethod, Endpoint).GetHashCode();
 
         public override string ToString()
             => GetBucketHash() ?? GetUniqueEndpoint();

src/Discord.Net.Rest/Net/Queue/RequestQueue.cs

@@ -125,16 +125,16 @@ namespace Discord.Net.Queue
         {
             await RateLimitTriggered(bucketId, info).ConfigureAwait(false);
         }
-        internal BucketId UpdateBucketHash(BucketId id, string discordHash)
+        internal (RequestBucket, BucketId) UpdateBucketHash(BucketId id, string discordHash)
         {
             if (!id.IsHashBucket)
             {
                 var bucket = BucketId.Create(discordHash, id);
-                _buckets.GetOrAdd(bucket, _buckets[id]);
+                var hashReqQueue = (RequestBucket)_buckets.GetOrAdd(bucket, _buckets[id]);
                 _buckets.AddOrUpdate(id, bucket, (oldBucket, oldObj) => bucket);
-                return bucket;
+                return (hashReqQueue, bucket);
             }
-            return null;
+            return (null, null);
         }
 
         private async Task RunCleanup()

src/Discord.Net.Rest/Net/Queue/RequestQueueBucket.cs

@@ -19,6 +19,7 @@ namespace Discord.Net.Queue
         private readonly RequestQueue _queue;
         private int _semaphore;
         private DateTimeOffset? _resetTick;
+        private RequestBucket _redirectBucket;
 
         public BucketId Id { get; private set; }
         public int WindowCount { get; private set; }
@@ -52,6 +53,8 @@ namespace Discord.Net.Queue
             {
                 await _queue.EnterGlobalAsync(id, request).ConfigureAwait(false);
                 await EnterAsync(id, request).ConfigureAwait(false);
+                if (_redirectBucket != null)
+                    return await _redirectBucket.SendAsync(request);
 
 #if DEBUG_LIMITS
                 Debug.WriteLine($"[{id}] Sending...");
@@ -160,6 +163,9 @@ namespace Discord.Net.Queue
 
             while (true)
             {
+                if (_redirectBucket != null)
+                    break;
+
                 if (DateTimeOffset.UtcNow > request.TimeoutAt || request.Options.CancelToken.IsCancellationRequested)
                 {
                     if (!isRateLimited)
@@ -216,13 +222,15 @@ namespace Discord.Net.Queue
             }
         }
 
-        private void UpdateRateLimit(int id, RestRequest request, RateLimitInfo info, bool is429)
+        private void UpdateRateLimit(int id, RestRequest request, RateLimitInfo info, bool is429, bool redirected = false)
         {
             if (WindowCount == 0)
                 return;
 
             lock (_lock)
             {
+                if (redirected)
+                    Interlocked.Decrement(ref _semaphore); //we might still hit a real ratelimit if all tickets were already taken, can't do much about it since we didn't know they were the same
                 bool hasQueuedReset = _resetTick != null;
                 if (info.Limit.HasValue && WindowCount != info.Limit.Value)
                 {
@@ -233,10 +241,20 @@ namespace Discord.Net.Queue
 #endif
                 }
 
-                if (info.Bucket != null)
-                    Id = _queue.UpdateBucketHash(request.Options.BucketId, info.Bucket) ?? Id;
+                if (info.Bucket != null && !redirected)
+                {
+                    (RequestBucket, BucketId) hashBucket = _queue.UpdateBucketHash(request.Options.BucketId, info.Bucket);
+                    if (hashBucket.Item1 is null || hashBucket.Item2 is null)
+                        return;
+                    if (hashBucket.Item1 == this) //this bucket got promoted to a hash queue
+                        Id = hashBucket.Item2;
+                    else
+                    {
+                        _redirectBucket = hashBucket.Item1; //this request should be part of another bucket, this bucket will be disabled, redirect everything
+                        _redirectBucket.UpdateRateLimit(id, request, info, is429, redirected: true); //update the hash bucket ratelimit
+                    }
+                }
 
-                var now = DateTimeOffset.UtcNow.ToUnixTimeSeconds();
                 DateTimeOffset? resetTick = null;
 
                 //Using X-RateLimit-Remaining causes a race condition
@@ -253,16 +271,15 @@ namespace Discord.Net.Queue
                     Debug.WriteLine($"[{id}] Retry-After: {info.RetryAfter.Value} ({info.RetryAfter.Value} ms)");
 #endif
                 }
-				else if (info.ResetAfter.HasValue && (request.Options.UseSystemClock.HasValue ? !request.Options.UseSystemClock.Value : false))
-				{
-					resetTick = DateTimeOffset.UtcNow.Add(info.ResetAfter.Value);
-				}
+                else if (info.ResetAfter.HasValue && (request.Options.UseSystemClock.HasValue ? !request.Options.UseSystemClock.Value : false))
+                {
+                    resetTick = DateTimeOffset.UtcNow.Add(info.ResetAfter.Value);
+                }
                 else if (info.Reset.HasValue)
                 {
                     resetTick = info.Reset.Value.AddSeconds(info.Lag?.TotalSeconds ?? 1.0);
 
-					/* millisecond precision makes this unnecessary, retaining in case of regression
-
+                    /* millisecond precision makes this unnecessary, retaining in case of regression
                     if (request.Options.IsReactionBucket)
                         resetTick = DateTimeOffset.Now.AddMilliseconds(250);
 					*/