Skip to content

SetRESTBucketState is missing the Math.Max(0, ...) clamp that SetGraphQLBucketState has #1256

Description

@TimSirmovics

Got this in production on 6.28.0 during a burst of concurrent REST calls:

System.ArgumentOutOfRangeException: Specified argument was out of the range of valid values.
(Parameter 'currentlyAvailable (-0.113575999999171) must be positive or zero.')
   at ShopifySharp.Infrastructure.Policies.LeakyBucket.LeakyBucket.SetState(Int32 maximumAvailable, Int32 restoreRatePerSecond, Double currentlyAvailable)
   at ShopifySharp.LeakyBucketExecutionPolicy.ExecuteRestAdminRequest[T](...)

MultiShopifyApiBucket.SetRESTBucketState passes Math.Min(RESTBucket.ComputedCurrentlyAvailable, currentlyAvailable) straight into SetState, but ComputedCurrentlyAvailable extrapolates from DateTime.UtcNow, which isn't monotonic — so when the bucket is saturated it can come out slightly negative (a tiny backwards clock adjustment is enough) and SetState's guard throws. SetGraphQLBucketState right below it already wraps the same expression in Math.Max(0.0, ...), so it looks like this was fixed for GraphQL and the REST path got missed.

Because it throws after the request already succeeded, and it's an ArgumentOutOfRangeException rather than a ShopifyException, the retry policy never catches it.

Fix would be the same clamp as the GraphQL path:

currentlyAvailable = Math.Max(0.0, Math.Min(RESTBucket.ComputedCurrentlyAvailable, currentlyAvailable));

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions