From 9fdc66a87cd3842a6bf8102c9dd0ebe4694d8311 Mon Sep 17 00:00:00 2001 From: Alan Michel Thomaz Date: Thu, 28 Mar 2019 09:40:15 -0300 Subject: [PATCH 1/8] Retry with Exponential backoff --- lib/fcm.rb | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/fcm.rb b/lib/fcm.rb index 87a3cfd..64e3450 100644 --- a/lib/fcm.rb +++ b/lib/fcm.rb @@ -94,7 +94,7 @@ def recover_notification_key(key_name, project_id) notification_key_name: key_name } } - + extra_headers = { 'project_id' => project_id } @@ -144,7 +144,7 @@ def get_instance_id_info iid_token, options={} params = { query: options } - + for_uri(INSTANCE_ID_API) do |connection| response = connection.get('/iid/info/'+iid_token, params) build_response(response) @@ -177,7 +177,14 @@ def send_to_topic_condition(condition, options = {}) private def for_uri(uri, extra_headers = {}) + retryable_exceptions = Faraday::Request::Retry::DEFAULT_EXCEPTIONS +[ServerError] 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], methods: [], + retry_if: Proc.new do |env, exception| + return true unless exception.is_a?(Faraday::RetriableResponse) + return true if exception.response.body[:results].any? { |result| result[:error] == "Unavailable" } + end faraday.adapter Faraday.default_adapter faraday.headers["Content-Type"] = "application/json" faraday.headers["Authorization"] = "key=#{api_key}" From ad2e977cd0fb5c088e6c7bab190680d368988c92 Mon Sep 17 00:00:00 2001 From: Alan Michel Thomaz Date: Thu, 28 Mar 2019 09:48:35 -0300 Subject: [PATCH 2/8] Update to FCM URLs for device groups features --- lib/fcm.rb | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/lib/fcm.rb b/lib/fcm.rb index 64e3450..4ee662b 100644 --- a/lib/fcm.rb +++ b/lib/fcm.rb @@ -8,7 +8,6 @@ class FCM FORMAT = :json # constants - GROUP_NOTIFICATION_BASE_URI = 'https://android.googleapis.com' INSTANCE_ID_API = 'https://iid.googleapis.com' TOPIC_REGEX = /[a-zA-Z0-9\-_.~%]+/ @@ -49,8 +48,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| + response = connection.post('/fcm/notification', post_body.to_json) build_response(response) end end @@ -65,8 +64,8 @@ def add_registration_ids(key_name, project_id, notification_key, registration_id '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| + response = connection.post('/fcm/notification', post_body.to_json) build_response(response) end end @@ -81,8 +80,8 @@ def remove_registration_ids(key_name, project_id, notification_key, registration '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| + response = connection.post('/fcm/notification', post_body.to_json) build_response(response) end end @@ -99,8 +98,8 @@ def recover_notification_key(key_name, project_id) 'project_id' => project_id } - for_uri(GROUP_NOTIFICATION_BASE_URI, extra_headers) do |connection| - response = connection.get('/gcm/notification', params) + for_uri(BASE_URI, extra_headers) do |connection| + response = connection.get('/fcm/notification', params) build_response(response) end end From f99a4c3cfce16cea1f4400140bd1b5f8fdb7cdcf Mon Sep 17 00:00:00 2001 From: Alan Michel Thomaz Date: Fri, 29 Mar 2019 11:36:21 -0300 Subject: [PATCH 3/8] fix tests to use FCM device group URL --- spec/fcm_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/fcm_spec.rb b/spec/fcm_spec.rb index d0341b6..3f93e20 100644 --- a/spec/fcm_spec.rb +++ b/spec/fcm_spec.rb @@ -2,7 +2,7 @@ describe FCM do let(:send_url) { "#{FCM::BASE_URI}/fcm/send" } - let(:group_notification_base_uri) { "#{FCM::GROUP_NOTIFICATION_BASE_URI}/gcm/notification" } + let(:group_notification_base_uri) { "#{FCM::BASE_URI}/fcm/notification" } let(:api_key) { 'AIzaSyB-1uEai2WiUapxCs2Q0GZYzPu7Udno5aA' } let(:registration_id) { '42' } let(:registration_ids) { ['42'] } From 3607446f8f8e017a4ef4d9a5a7d357939161ebb7 Mon Sep 17 00:00:00 2001 From: Alan Michel Thomaz Date: Fri, 29 Mar 2019 11:40:28 -0300 Subject: [PATCH 4/8] fix,refactor and added tests for retry --- lib/fcm.rb | 22 +++++++++++++------ spec/fcm_spec.rb | 56 ++++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 66 insertions(+), 12 deletions(-) diff --git a/lib/fcm.rb b/lib/fcm.rb index 4ee662b..a0b451e 100644 --- a/lib/fcm.rb +++ b/lib/fcm.rb @@ -33,6 +33,7 @@ def initialize(api_key, client_options = {}) def send_notification(registration_ids, options = {}) post_body = build_post_body(registration_ids, options) + for_uri(BASE_URI) do |connection| response = connection.post('/fcm/send', post_body.to_json) build_response(response, registration_ids) @@ -176,14 +177,23 @@ def send_to_topic_condition(condition, options = {}) private def for_uri(uri, extra_headers = {}) - retryable_exceptions = Faraday::Request::Retry::DEFAULT_EXCEPTIONS +[ServerError] + retry_if_func = lambda do |env, exception| + case (exception.response[:status] || exception.response.status) + when (500..599) + true + when (400..499) + false + when 200 + body = JSON.parse(exception.response.body) + body["results"] != nil && body["results"].any? { |result| result["error"] == "Unavailable" } + end + end + retryable_exceptions = Faraday::Request::Retry::DEFAULT_EXCEPTIONS + [ Faraday::ClientError] connection = ::Faraday.new(:url => uri) do |faraday| - faraday.request :retry, max: 5, interval: 0.1, interval_randomness: 0.5, backoff_factor: 2, + faraday.request :retry, max: 5, interval: 0.1, interval_randomness: 0.5, backoff_factor: 2, exceptions: retryable_exceptions, retry_statuses: [200], methods: [], - retry_if: Proc.new do |env, exception| - return true unless exception.is_a?(Faraday::RetriableResponse) - return true if exception.response.body[:results].any? { |result| result[:error] == "Unavailable" } - end + retry_if: retry_if_func + faraday.response :raise_error faraday.adapter Faraday.default_adapter faraday.headers["Content-Type"] = "application/json" faraday.headers["Authorization"] = "key=#{api_key}" diff --git a/spec/fcm_spec.rb b/spec/fcm_spec.rb index 3f93e20..269b177 100644 --- a/spec/fcm_spec.rb +++ b/spec/fcm_spec.rb @@ -90,7 +90,7 @@ stub_request(:post, send_url) .with(body: '{"registration_ids":["42"],"data":{"score":"5x1","time":"15:10"}}', headers: valid_request_headers) - .to_return(status: 200, body: '', headers: {}) + .to_return(status: 200, body: '{}', headers: {}) end before do end @@ -106,13 +106,13 @@ stub_request(:post, send_url) .with(body: '{"to":"/topics/TopicA","data":{"score":"5x1","time":"15:10"}}', headers: valid_request_headers) - .to_return(status: 200, body: '', headers: {}) + .to_return(status: 200, body: '{}', headers: {}) end let!(:stub_with_invalid_topic) do stub_request(:post, send_url) .with(body: '{"condition":"/topics/TopicA$","data":{"score":"5x1","time":"15:10"}}', headers: valid_request_headers) - .to_return(status: 200, body: '', headers: {}) + .to_return(status: 200, body: '{}', headers: {}) end describe "#send_to_topic" do @@ -135,19 +135,19 @@ stub_request(:post, send_url) .with(body: '{"condition":"\'TopicA\' in topics && (\'TopicB\' in topics || \'TopicC\' in topics)","data":{"score":"5x1","time":"15:10"}}', headers: valid_request_headers) - .to_return(status: 200, body: '', headers: {}) + .to_return(status: 200, body: '{}', headers: {}) end let!(:stub_with_invalid_condition) do stub_request(:post, send_url) .with(body: '{"condition":"\'TopicA\' in topics and some other text (\'TopicB\' in topics || \'TopicC\' in topics)","data":{"score":"5x1","time":"15:10"}}', headers: valid_request_headers) - .to_return(status: 200, body: '', headers: {}) + .to_return(status: 200, body: '{}', headers: {}) end let!(:stub_with_invalid_condition_topic) do stub_request(:post, send_url) .with(body: '{"condition":"\'TopicA$\' in topics","data":{"score":"5x1","time":"15:10"}}', headers: valid_request_headers) - .to_return(status: 200, body: '', headers: {}) + .to_return(status: 200, body: '{}', headers: {}) end describe "#send_to_topic_condition" do @@ -338,6 +338,50 @@ ) end end + + context 'when send_notification responds with 5XX or 200+error:unavailable' do + subject { FCM.new(api_key) } + + let(:valid_response_body) do + { + failure: 0, canonical_ids: 0, results: [{ message_id: '0:1385025861956342%572c22801bb3' }] + } + end + + let(:error_response_body) do + { + failure: 1, canonical_ids: 0, results: [{ error: 'Unavailable' }] + } + end + + + let(:stub_fcm_send_request_unavaible_server) do + i = 0 + stub_request(:post, send_url).with( + body: "{\"registration_ids\":[\"42\"]}", + headers: { + 'Accept'=>'*/*', + 'Accept-Encoding'=>'gzip;q=1.0,deflate;q=0.6,identity;q=0.3', + 'User-Agent'=>'Faraday v0.15.4' + }).to_return do |stub| + body = {} + status = ( i == 0) ? 501 : 200 + body = error_response_body if i == 1 + body = valid_response_body if i == 0 + i += 1 + {status: status, headers: { "Retry-After": "1"}, body: body.to_json} + end + end + + before(:each) do + stub_fcm_send_request_unavaible_server + end + + it 'should retry 2 times' do + subject.send( registration_ids) + stub_fcm_send_request_unavaible_server.should have_been_made.times(3) + end + end end describe 'sending group notifications' do From 46079de109a48f91b5c6688bef20a65aa9e21cc2 Mon Sep 17 00:00:00 2001 From: Alan Michel Thomaz Date: Fri, 29 Mar 2019 12:09:37 -0300 Subject: [PATCH 5/8] cover all retryable errors --- lib/fcm.rb | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lib/fcm.rb b/lib/fcm.rb index a0b451e..9abff3d 100644 --- a/lib/fcm.rb +++ b/lib/fcm.rb @@ -178,6 +178,9 @@ def send_to_topic_condition(condition, options = {}) def for_uri(uri, extra_headers = {}) retry_if_func = lambda do |env, exception| + retryable_errors = [ + "Unavailable", "InternalServerError", + "DeviceMessageRateExceeded", "TopicsMessageRateExceeded"] case (exception.response[:status] || exception.response.status) when (500..599) true @@ -185,7 +188,7 @@ def for_uri(uri, extra_headers = {}) false when 200 body = JSON.parse(exception.response.body) - body["results"] != nil && body["results"].any? { |result| result["error"] == "Unavailable" } + body["results"] != nil && body["results"].any? { |result| retryable_errors.include? result["error"]} end end retryable_exceptions = Faraday::Request::Retry::DEFAULT_EXCEPTIONS + [ Faraday::ClientError] From f86baf665bcda7f8d14d0546726187fa85b2f857 Mon Sep 17 00:00:00 2001 From: Alan Michel Thomaz Date: Mon, 1 Apr 2019 09:36:01 -0300 Subject: [PATCH 6/8] removing Faraday raise_error middleware. --- lib/fcm.rb | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/lib/fcm.rb b/lib/fcm.rb index 9abff3d..c65d89a 100644 --- a/lib/fcm.rb +++ b/lib/fcm.rb @@ -181,22 +181,18 @@ def for_uri(uri, extra_headers = {}) retryable_errors = [ "Unavailable", "InternalServerError", "DeviceMessageRateExceeded", "TopicsMessageRateExceeded"] - case (exception.response[:status] || exception.response.status) - when (500..599) - true - when (400..499) - false - when 200 + if defined?(exception.response) && defined?(exception.response.status) && exception.response.status == 200 body = JSON.parse(exception.response.body) body["results"] != nil && body["results"].any? { |result| retryable_errors.include? result["error"]} + else + true end end - retryable_exceptions = Faraday::Request::Retry::DEFAULT_EXCEPTIONS + [ Faraday::ClientError] + retryable_exceptions = Faraday::Request::Retry::DEFAULT_EXCEPTIONS 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], methods: [], + exceptions: retryable_exceptions, retry_statuses: [200, *(500..599)], methods: [], retry_if: retry_if_func - faraday.response :raise_error faraday.adapter Faraday.default_adapter faraday.headers["Content-Type"] = "application/json" faraday.headers["Authorization"] = "key=#{api_key}" From a8742e005df568eda4a8f4e72cae6851ee230183 Mon Sep 17 00:00:00 2001 From: Alan Michel Thomaz Date: Mon, 1 Apr 2019 09:44:01 -0300 Subject: [PATCH 7/8] fix syntax in specs for ruby version 2.1 or lower --- spec/fcm_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/fcm_spec.rb b/spec/fcm_spec.rb index 269b177..60c9527 100644 --- a/spec/fcm_spec.rb +++ b/spec/fcm_spec.rb @@ -369,7 +369,7 @@ body = error_response_body if i == 1 body = valid_response_body if i == 0 i += 1 - {status: status, headers: { "Retry-After": "1"}, body: body.to_json} + {status: status, headers: { :"Retry-After" => "1"}, body: body.to_json} end end From 86202624e6958c3f0c8dc80f50d1197383821192 Mon Sep 17 00:00:00 2001 From: Alan Michel Thomaz Date: Mon, 15 Apr 2019 09:54:04 -0300 Subject: [PATCH 8/8] refactor notification_key related methods --- lib/fcm.rb | 38 ++++++++++++++------------------------ 1 file changed, 14 insertions(+), 24 deletions(-) diff --git a/lib/fcm.rb b/lib/fcm.rb index c65d89a..bcf421e 100644 --- a/lib/fcm.rb +++ b/lib/fcm.rb @@ -45,14 +45,7 @@ def create_notification_key(key_name, project_id, registration_ids = []) post_body = build_post_body(registration_ids, operation: 'create', notification_key_name: key_name) - extra_headers = { - 'project_id' => project_id - } - - for_uri(BASE_URI, extra_headers) do |connection| - response = connection.post('/fcm/notification', post_body.to_json) - build_response(response) - end + update_group_messaging_setting(post_body, project_id) end alias create create_notification_key @@ -61,14 +54,7 @@ def add_registration_ids(key_name, project_id, notification_key, registration_id notification_key_name: key_name, notification_key: notification_key) - extra_headers = { - 'project_id' => project_id - } - - for_uri(BASE_URI, extra_headers) do |connection| - response = connection.post('/fcm/notification', post_body.to_json) - build_response(response) - end + update_group_messaging_setting(post_body, project_id) end alias add add_registration_ids @@ -77,14 +63,7 @@ def remove_registration_ids(key_name, project_id, notification_key, registration notification_key_name: key_name, notification_key: notification_key) - extra_headers = { - 'project_id' => project_id - } - - for_uri(BASE_URI, extra_headers) do |connection| - response = connection.post('/fcm/notification', post_body.to_json) - build_response(response) - end + update_group_messaging_setting(post_body, project_id) end alias remove remove_registration_ids @@ -253,6 +232,17 @@ def build_not_registered_ids(body, registration_id) not_registered_ids end + def update_group_messaging_setting(body, project_id) + extra_headers = { + 'project_id' => project_id + } + + for_uri(BASE_URI, extra_headers) do |connection| + response = connection.post('/fcm/notification', body.to_json) + build_response(response) + end + end + def execute_notification(body) for_uri(BASE_URI) do |connection| response = connection.post('/fcm/send', body.to_json)