Add refresh buffer

[shime]

We had a case when the payment option was attempted to be deleted one second before the token expiry time raising 401 when the network request reached Booqable. This adds buffer for expiry.

---

[Copilot]

Pull request overview

This PR hardens OAuth request authentication by refreshing tokens slightly before expiry and retrying once on a 401, addressing edge cases where a token expires between the client-side freshness check and server-side validation.

Changes:

• Add an expiry “refresh buffer” to proactively refresh near-expired tokens.
• Retry a request once on 401 by refreshing/reloading the stored token and re-sending the request.
• Update and expand specs to cover buffer behavior, 401 retry behavior, and concurrent refresh reuse.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
spec/booqable/oauth_client_spec.rb	Reworks middleware integration specs to cover refresh buffer, single-retry-on-401, and concurrent refresh behavior.
spec/booqable/client_spec.rb	Updates client specs to align with the new “expires_at-based” refresh behavior (no reliance on expired?).
lib/booqable/middleware/auth/oauth.rb	Implements refresh buffer logic and a single 401-triggered refresh+retry flow in the OAuth Faraday middleware.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (2)

README.md:115

• The around_refresh_token callable is invoked around the read + expiry-check sequence even when no refresh happens (and then an additional time on the 401 retry path). The current wording "around each refresh attempt" is misleading about when the callback runs.

serialize the read + expiry-check + refresh sequence. The middleware yields
to the callable around each refresh attempt — normally once per request, plus
one more time if a request is retried after the server rejects its token with a
401. The host application decides how to lock.

CHANGELOG.md:5

• Grammar: "refresh the access token a short buffer" is missing a preposition and reads incorrectly in the changelog entry.

- OAuth middleware: refresh the access token a short buffer
  (`REFRESH_BUFFER_SECONDS`, 60s) before it expires, and retry a request once if
  the server still rejects the token with a 401 (refreshing first). This avoids

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

lib/booqable/middleware/auth/oauth.rb:92

• call loads/refreshes the stored OAuth token even when the request already has a caller-supplied Authorization header (i.e., when manages_authorization? is false). This can unexpectedly take locks via around_refresh_token, refresh/write tokens, and do extra work for requests that explicitly opted out of OAuth handling.

        def call(env)
          token = fresh_token
          managed = manages_authorization?(env)
          # (Re)apply our token on every managed pass — including a Faraday::Retry
          # re-send, which re-enters with our previous attempt's header still set.
          env.request_headers["Authorization"] = bearer_header(token) if managed

          request_body = env[:body]
          response = @app.call(env)

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (2)

lib/booqable/middleware/auth/oauth.rb:18

• The usage example sets api_endpoint to a token URL (.../oauth/token), but OAuthClient already appends /oauth/token internally (see lib/booqable/oauth_client.rb:38). Using the token URL here would produce an incorrect token_url like .../oauth/token/oauth/token.

      #     api_endpoint: "https://company.booqable.com/api/v4/oauth/token",
      #     read_token: -> { stored_token },
      #     write_token: ->(token) { store_token(token) }
      class OAuth < Base
        # Refresh the token slightly before it actually expires. A request that

lib/booqable/middleware/auth/oauth.rb:104

• On the 401 retry path the request body is restored, but if the body is an IO/stream it may have been consumed by the first attempt. Rewinding when possible avoids retrying with an empty/partial body.

            # flight, and refreshing again could fail on the rotated refresh token.
            # A refresh that itself fails raises, surfacing the genuine auth error
            # rather than retrying with a doomed token.
            rejected = token.token
            token = fresh_token { |reloaded| reloaded.token == rejected }

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (2)

lib/booqable/middleware/auth/oauth.rb:104

• Use env.body = ... rather than writing env[:body] directly, for consistency with Faraday's public API (and to match how other middleware in this repo reads the body).

            env[:body] = request_body

lib/booqable/middleware/auth/oauth.rb:93

• Use Faraday::Env's body accessor instead of env[:body] to align with the public API and existing middleware usage (e.g. lib/booqable/middleware/auth/single_use.rb:119).

This issue also appears on line 104 of the same file.

          request_body = env[:body]

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (1)

lib/booqable/middleware/auth/oauth.rb:74

• Use Faraday::Env's body accessor consistently (as done in other middleware) instead of env[:body]. It’s clearer and avoids relying on hash-style access semantics when saving/restoring the request body for the 401 retry.

          body = env[:body]
          env.request_headers["Authorization"] = "Bearer #{token.token}"

[devin-ai-integration]

Devin Review found 1 new potential issue.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (3)

lib/booqable/middleware/auth/oauth.rb:85

• expires_at.to_i can produce incorrect results when expires_at is a String (e.g., ISO8601) since String#to_i does not parse timestamps (it often becomes 0 or the leading year). That can cause premature refreshes or missed refreshes. Consider explicitly handling Time, Integer, and common string formats (e.g., parse ISO8601), or validating/normalizing expires_at when reading the token hash so this comparison is reliable.

        def expires_soon?(token)
          token.expires_at.nil? || Time.now.to_i >= token.expires_at.to_i - REFRESH_BUFFER_SECONDS
        end

spec/booqable/client_spec.rb:421

• WebMock header matching can be sensitive to how headers are normalized. To reduce brittleness, match the canonical header name (e.g., \"Authorization\") rather than authorization, or use a more flexible matcher for the header key. This makes the test less dependent on adapter/header normalization details.

        orders_request = stub_request(:get, booqable_url("/orders")).with(headers: { authorization: "Bearer renewed_token" })

spec/booqable/client_spec.rb:412

• Using Time.now directly in specs can introduce timing-related flakiness and makes intent harder to reason about. Consider freezing time (e.g., ActiveSupport::Testing::TimeHelpers#freeze_time / travel_to, or Timecop if used in the repo) and basing expires_at on the frozen time to keep this deterministic.

              expires_at: Time.now + 30 # not yet expired, but inside the buffer

[devin-ai-integration]

Devin Review found 1 new potential issue.

[pbalaban]

Looks good for me, much better then the initial version 👍

Claude found this issue, I did not verify it myself:

refresh_token! rescue crashes with NoMethodError, and the buffer makes it fire on requests that used to work (oauth.rb:96)

e.response.response.env assumes e.response is a Faraday response. oauth2 also raises OAuth2::Error.new({...}) with a plain Hash — when the stored token has no refresh token, or when the token endpoint answers without an access token.

It looks like it's not a regression and we did not see such an exceptions in AppSignal yet.

[shime]

@pbalaban will be addressed in a separate PR