Automatic retry with exponential backoff #58
Conversation
|
oh, i just screw the other http errors, i will be fixing it |
|
cool thanks... let me know when you are ready to review |
|
it's fixed, just removed the error raise middleware, it was causing more problems than helping, refactored to use just retryable_statuses. |
| @@ -34,6 +33,7 @@ def initialize(api_key, client_options = {}) | |||
| def send_notification(registration_ids, options = {}) | |||
| post_body = build_post_body(registration_ids, options) | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
👇here why not just using execute_notifications(post_body) instead
lib/fcm.rb
Outdated
| @@ -49,8 +49,8 @@ def create_notification_key(key_name, project_id, registration_ids = []) | |||
| 'project_id' => project_id | |||
| } | |||
|
|
|||
| for_uri(GROUP_NOTIFICATION_BASE_URI, extra_headers) do |connection| | |||
| response = connection.post('/gcm/notification', post_body.to_json) | |||
| for_uri(BASE_URI, extra_headers) do |connection| | |||
There was a problem hiding this comment.
since there are four places where we need to call /fcm/notification
- create_notification_key(post)
- add_registration_ids(post)
- remove_registration_ids(post)
- recover_notification_key(get)
maybe it would be better if we refactor this api call into a separate private method, like what execute_notification method does..
def update_group_messaging_setting(body)
for_uri(BASE_URI, extra_headers) do |connection|
response = connection.post('/fcm/notification', body.to_json)
build_response(response)
end
endThere was a problem hiding this comment.
done, except for recover_notification_key, because it's a get request
|
Hi - great work on the PR. Is there a plan to get this merged soon? We're using the gem and are trying to work out if we can wait for this PR or if we need to create our own fork. Thanks! |
| connection = ::Faraday.new(:url => uri) do |faraday| | ||
| faraday.request :retry, max: 5, interval: 0.1, interval_randomness: 0.5, backoff_factor: 2, | ||
| exceptions: retryable_exceptions, retry_statuses: [200, *(500..599)], methods: [], | ||
| retry_if: retry_if_func | ||
| faraday.adapter Faraday.default_adapter | ||
| faraday.headers["Content-Type"] = "application/json" | ||
| faraday.headers["Authorization"] = "key=#{api_key}" |
There was a problem hiding this comment.
Before
Authorization: key=AIzaSyZ-1u...0GBYzPu7Udno5aA
After
Authorization: Bearer ya29.ElqKBGN2Ri_Uz...HnS_uNreA
Implemented it using the recently added gem Faraday, when it is status 5xx or 200 and any of the results form body is error:Unavaible, it will retry the whole request, with an exponential interval.
passed on simple rspec test with webmock stubs.
I'm not sure if those interval params where good the exponential backoff.
Also updated group notification url to fcm url. #56
ToDo
Implement a Retry faraday middleware that allow editing the body of the request before resend it, so when it is 200 + error:unavaible, we can resend it only for the registration_ids that failed, instead to sending for all.