Skip to content

off_quota? and Fbe::Middleware::RateLimit track remaining counts in two places that can drift #450

@igor-belousov

Description

@igor-belousov

What

After #447 / PR #448, the GitHub Search API remaining quota is tracked in two independent places:

  1. Fbe::Middleware::RateLimit#@searchleft — decremented per request when env.url.path.start_with?('/search/').
  2. Fbe.octo's off_quota? reads @origin.last_response.body['resources']['search']['remaining'] from whatever response the Octokit client most recently received.

The same is true for the core remaining count: middleware tracks @remaining, while off_quota? reads via @origin.rate_limit!.remaining.

Why it matters

Today the two counters happen to agree because:

  • off_quota? calls @origin.rate_limit! first, which hits /rate_limit through Faraday, which goes through the middleware, which serves a cached response patched with the latest @searchleft.
  • The patched body reaches off_quota? via last_response.body.

But this correctness depends on call ordering: rate_limit! must run before any code reads last_response.body for the search remaining. If a future caller reads last_response.body directly, or if anything between the middleware and the Octokit client mutates the response, the two counters can drift silently.

The off_quota? fallback warning ("Search-quota check fell back to core remaining...") is the only signal that something went wrong, and it only fires on the :search resource.

Proposed fix

Make Fbe::Middleware::RateLimit the single source of truth and have off_quota? query it directly, eliminating the last_response.body plumbing.

Sketch:

class Fbe::Middleware::RateLimit
  def remaining(resource = :core)
    resource == :search ? @searchleft : @remaining
  end
end

# in off_quota?:
mw = @origin.connection.builder.handlers.find { |h| h.klass == Fbe::Middleware::RateLimit }
left = mw.instance.remaining(resource)

(Exact API needs design — exposing middleware state to the calling code is a bit awkward in Faraday; one option is to stash a reference to the middleware instance on the Octokit client during Fbe.octo setup.)

Why this wasn't done in #447

The PR was already substantive (two-layer fix, 11 new tests, ~370 LOC). Adding a refactor that touches both the middleware API and off_quota?'s internals would have doubled the surface area and made review harder. The current design works; it's just fragile.

Filed during PR #448.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions