Parsing non-standard rate limit reset header value

[zavan]

The SendGrid API responses have a X-RateLimit-Reset header on 429 rate limit errors, but the value of that header is a integer unix timestamp (e.g. 1685544756), not a rfc2822 date or a number of seconds like the RFC and this gem expects.

I went around that by extending the middleware and overriding the method:

class RetryMiddleware < Faraday::Retry::Middleware
  private

  def parse_retry_header(env, header)
    response_headers = env[:response_headers]
    return unless response_headers

    retry_after_value = env[:response_headers][header]
    return super unless retry_after_value

    begin
      Time.zone.at(retry_after_value.to_i) - Time.zone.now
    rescue
      super
    end
  end
end

Faraday.new do |conn|
  conn.use RetryMiddleware, retry_statuses: [429], rate_limit_reset_header: 'X-RateLimit-Reset'
  conn.adapter Faraday.default_adapter
end

Is there a better way of doing this? If not, could we implement it?

[iMacTia]

Hi @zavan, thank you for raising this issue.
This is indeed a tricky, since SendGrid are effectively disregarding the RFC standard.
And even if we wanted to try and make the middleware smarter, it would still be tricky because a numeric value IS a valid value, but it has a different meaning from the one they convey.
Overriding the method is also a bad idea because that would change the behaviour with ALL APIs, making it impossible for this solution to work if your application needs to deal with 2 APIs, one following the standard and the other not.

I have a couple of potential solutions in mind:

1. Automatically assume the value is a unix timestamp if it's above a certain threshold
2. Make the middleware accept a parse_header_block option upon initialization

Solution 1

This is based on the assumption that a unix timestamp is a really long amount of time, so if we were to find a value above let's say 1,600,000,000 that would be a good indicator that the value is a timestamp, rather than the service asking us to wait 50 years 😅.
What I don't like about this solution though:

• It makes the middleware more complicated (maintenance)
• It brings extra complexity due to an external party not following the RFC standard
• It won't work if another API in future will decide to send back a different value (e.g. milliseconds instead of seconds)
  Overall, this solution is automatic but is based on some fragile assumptions and it brings the burden of maintenance on our side.

Solution 2

This would me a much more elegant solution, IMO.
The usage would be something like the following:

Faraday.new do |conn|
  conn.use RetryMiddleware,
    retry_statuses: [429],
    rate_limit_reset_header: 'X-RateLimit-Reset',
    header_parser_block: ->(value) { Time.zone.at(value.to_i) - Time.zone.now }
  # ...
end

This way, when you have to deal with an API that doesn't follow the standard, you can pass a custom block to parse the value correctly. This solution has multiple advantages:

• It's flexible and can cover any possible use-case
• It does not increase complexity or maintenance every time we discover a new non-conforming API
• Can be customized on a per-connection basis

Conclusion

Although the SendGrid API is ultimately responsible for the issue, we could implement "Solution 2" to allow users to deal with such use-cases in a maintainable way.
I'd be open to reviewing a PR that implements such solution or hearing about other ideas on how to approach this problem.

[zavan]

Thanks @iMacTia!

Yeah, I don't understand why SG went that way, maybe they implemented it before the RFC was accepted, which seems to be a recent thing.

Solution 2 sounds great and it's what I had in mind, I didn't implement it because it requires adding the option, checking if was passed etc, so it was simpler to just extend and override the method in my case (if other APIs need the original middleware they can just use that, I'm not monkey-patching the middleware, just extending into a new class).

I'll write a PR for solution 2 next week.

[zavan]

Actually, found a minute and implemented in #28. Tests pass. Please let me know if you want any changes.