Skip to content

Retry-After is honored uncapped on 429s, with no total retry deadline and no rate-limit signal to the caller #23

Description

@ToxicOrca

Summary

src/bookstack-client.ts's 429 retry logic (introduced in v3.3.2, untouched
by PR #13 or PR #14, confirmed unchanged on current main at v5.4.0) has
three related gaps:

  1. The computed exponential backoff is capped at 30 seconds
    (Math.min(30000, ...)), but when BookStack's response includes a
    Retry-After header, that value is used directly with no ceiling at
    all.
  2. MAX_RETRIES_429 is 5, and nothing bounds the total time across all
    retries. Production logs from PR fix: eliminate book-slug cache stampede that OOM-crashed search (v5.2.0) #13's original bug report show real
    Retry-After values of 12-47 seconds. Five retries at the higher end of
    that range alone approaches four minutes for a single request, before
    accounting for any call that makes more than one request internally.
  3. When this happens, the only signal is a console.error on the server
    side. The MCP caller gets no indication a rate limit was hit, only
    silence until it eventually times out.

Why this matters now

A search_content call was observed hanging for 300 seconds with no
response, then eventually resolving. Three retries of the identical call
afterward all returned instantly, so this is intermittent, consistent with
a rate-limit-driven stall, not a deterministic bug. No server-side logs
were available to confirm a 429/Retry-After sequence at that exact
timestamp, so this incident is a plausible trigger, not a confirmed one.
The code gap itself is confirmed regardless of whether it explains this
specific incident: any sustained 429 pressure on the shared, rate-limited
BookStack token can currently cause a multi-minute silent stall on any
tool call, not just search.

Proposed fix

  • Cap the Retry-After-derived delay the same way the computed backoff is
    already capped.
  • Add a total deadline across the whole retry chain, not just a per-attempt
    timeout.
  • Surface a clear, distinct error to the MCP caller when a request is
    being rate-limited, instead of silence.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions