intercom-ruby icon indicating copy to clipboard operation
intercom-ruby copied to clipboard

fix bug rate_limit_details returned and a bug for intercom request header

Open Goran1708 opened this issue 4 years ago • 2 comments

Fix bug rate_limit_details returned as nil if net:http failed a request, and bug if intercom request header does not contain X-RateLimit-Reset header

Why?

Why are you making this change?

We have experienced a "bug" when using this library, we implemented our own sidekiq throttle limiter with existing rate_limit_details. It worked fine until at one point(my guess is) NET:HTTP request started failing and rate_limit_details was being returned as nil.

Another potential issue is if intercom does not return X-RateLimit-Reset header, the app will break because it will try to calculate try to subtract some time from Nil.

How?

Technical details on your change

For rate_limit_details:

  1. Not setting request.rate_limit_details if its nil, because it should be empty hash in the initialiser.
  2. Another solution is to pass rate_limit_details to request object. It's kind of an anti-pattern that both classes are initialising the same variable and both of them are using it. We should initialise it in Client, pass it to Request, Request can modify it and send it back if needed.

For bug prone date arithmetic:

  1. Set current time if reset_at is not returned(if its NIL that is) so that the library does not throw used - on NIL object error.
  2. We should not call sleep if the sleep amount is 0.

Goran1708 avatar Dec 10 '21 16:12 Goran1708

Hey @Goran1708

Wondering if this pr is still in progress or if you'd like us to give it a review

SeanHealy33 avatar Mar 01 '22 10:03 SeanHealy33

Hey @Goran1708

Wondering if this pr is still in progress or if you'd like us to give it a review

Heya! Yeah sure, you can give it a review. Sorry I missed your notification.

Goran1708 avatar Mar 15 '22 09:03 Goran1708