From 30beecd3e7b7d9b1080a8ac927190de2a0685385 Mon Sep 17 00:00:00 2001 From: Ryan Lue Date: Fri, 14 Dec 2018 02:59:19 +0100 Subject: [PATCH] Improve Zendesk import when DNS resolution fails etc. --- lib/import/zendesk/object_attribute/base.rb | 2 +- .../unit/import/zendesk/sub_sequence/base.rb | 13 ++- .../zendesk/object_attribute/base_examples.rb | 46 ++++++++ .../zendesk/object_attribute/base_spec.rb | 34 ------ .../zendesk/object_attribute/checkbox_spec.rb | 2 + .../zendesk/object_attribute/date_spec.rb | 2 + .../zendesk/object_attribute/decimal_spec.rb | 2 + .../zendesk/object_attribute/dropdown_spec.rb | 2 + .../zendesk/object_attribute/integer_spec.rb | 2 + .../zendesk/object_attribute/regexp_spec.rb | 2 + .../zendesk/object_attribute/tagger_spec.rb | 2 + .../zendesk/object_attribute/text_spec.rb | 2 + .../zendesk/object_attribute/textarea_spec.rb | 2 + .../zendesk/sub_sequence/base_examples.rb | 108 +++++++++++------- 14 files changed, 142 insertions(+), 79 deletions(-) create mode 100644 spec/lib/import/zendesk/object_attribute/base_examples.rb delete mode 100644 spec/lib/import/zendesk/object_attribute/base_spec.rb diff --git a/lib/import/zendesk/object_attribute/base.rb b/lib/import/zendesk/object_attribute/base.rb index b1ae02c9d..1858fb2d1 100644 --- a/lib/import/zendesk/object_attribute/base.rb +++ b/lib/import/zendesk/object_attribute/base.rb @@ -26,7 +26,7 @@ module Import def attribute_config(object, name, attribute) { - object: object, + object: object.to_s, name: name, display: attribute.title, data_type: data_type(attribute), diff --git a/lib/sequencer/unit/import/zendesk/sub_sequence/base.rb b/lib/sequencer/unit/import/zendesk/sub_sequence/base.rb index fc97a1a63..1da87f1ee 100644 --- a/lib/sequencer/unit/import/zendesk/sub_sequence/base.rb +++ b/lib/sequencer/unit/import/zendesk/sub_sequence/base.rb @@ -40,7 +40,18 @@ class Sequencer def resource_iteration(&block) resource_collection.public_send(resource_iteration_method, &block) rescue ZendeskAPI::Error::NetworkError => e - return if e.response.status.to_s == '403' && resource_klass.in?(%w[UserField OrganizationField]) + case e.response.status.to_s + when '403' + return if resource_klass.in?(%w[UserField OrganizationField]) + when /^5\d\d$/ + raise if (fail_count ||= 1) > 10 + + logger.error e + logger.info "Sleeping 10 seconds after ZendeskAPI::Error::NetworkError and retry (##{fail_count}/10)." + sleep 10 + + (fail_count += 1) && retry + end raise end diff --git a/spec/lib/import/zendesk/object_attribute/base_examples.rb b/spec/lib/import/zendesk/object_attribute/base_examples.rb new file mode 100644 index 000000000..5291ad522 --- /dev/null +++ b/spec/lib/import/zendesk/object_attribute/base_examples.rb @@ -0,0 +1,46 @@ +require 'rails_helper' + +RSpec.shared_examples Import::Zendesk::ObjectAttribute::Base do + let(:attribute) do + double( + title: 'Example attribute', + description: 'Example attribute description', + removable: false, + active: true, + position: 12, + visible_in_portal: true, + required_in_portal: true, + required: true, + type: 'input', + custom_field_options: [], + regexp_for_validation: '', + ) + end + + describe 'exception handling' do + let(:error_text) { Faker::Lorem.sentence } + it 'extends ObjectManager Attribute exception message' do + expect(ObjectManager::Attribute).to receive(:add).and_raise(RuntimeError, error_text) + + expect do + described_class.new('Ticket', 'example_field', attribute) + end.to raise_error(RuntimeError, /'example_field': #{error_text}$/) + end + end + + describe 'argument handling' do + it 'takes an ObjectLookup name as the first argument' do + expect(ObjectManager::Attribute) + .to receive(:add).with(hash_including(object: 'Ticket')) + + described_class.new('Ticket', 'example_field', attribute) + end + + it 'accepts a constant ObjectLookup name' do + expect(ObjectManager::Attribute) + .to receive(:add).with(hash_including(object: 'Ticket')) + + described_class.new(Ticket, 'example_field', attribute) + end + end +end diff --git a/spec/lib/import/zendesk/object_attribute/base_spec.rb b/spec/lib/import/zendesk/object_attribute/base_spec.rb deleted file mode 100644 index a253f0ce0..000000000 --- a/spec/lib/import/zendesk/object_attribute/base_spec.rb +++ /dev/null @@ -1,34 +0,0 @@ -require 'rails_helper' - -RSpec.describe Import::Zendesk::ObjectAttribute::Base do - - it 'extends ObjectManager Attribute exception text' do - - attribute = double( - title: 'Example attribute', - description: 'Example attribute description', - removable: false, - active: true, - position: 12, - visible_in_portal: true, - required_in_portal: true, - required: true, - type: 'input', - ) - - error_text = 'some error' - expect(ObjectManager::Attribute).to receive(:add).and_raise(RuntimeError, error_text) - - exception = nil - begin - described_class.new('Ticket', 'example_field', attribute) - rescue => e - exception = e - end - - expect(exception).not_to be nil - expect(exception).to be_a(RuntimeError) - expect(exception.message).to include(error_text) - expect(exception.message).not_to eq(error_text) - end -end diff --git a/spec/lib/import/zendesk/object_attribute/checkbox_spec.rb b/spec/lib/import/zendesk/object_attribute/checkbox_spec.rb index be31043b0..4592557ba 100644 --- a/spec/lib/import/zendesk/object_attribute/checkbox_spec.rb +++ b/spec/lib/import/zendesk/object_attribute/checkbox_spec.rb @@ -1,6 +1,8 @@ require 'rails_helper' +require 'lib/import/zendesk/object_attribute/base_examples' RSpec.describe Import::Zendesk::ObjectAttribute::Checkbox do + it_behaves_like Import::Zendesk::ObjectAttribute::Base it 'imports boolean object attribute from checkbox object field' do diff --git a/spec/lib/import/zendesk/object_attribute/date_spec.rb b/spec/lib/import/zendesk/object_attribute/date_spec.rb index fe19ba525..8aaba3854 100644 --- a/spec/lib/import/zendesk/object_attribute/date_spec.rb +++ b/spec/lib/import/zendesk/object_attribute/date_spec.rb @@ -1,9 +1,11 @@ require 'rails_helper' +require 'lib/import/zendesk/object_attribute/base_examples' # required due to some of rails autoloading issues require 'import/zendesk/object_attribute/date' RSpec.describe Import::Zendesk::ObjectAttribute::Date do + it_behaves_like Import::Zendesk::ObjectAttribute::Base it 'imports date object attribute from date object field' do diff --git a/spec/lib/import/zendesk/object_attribute/decimal_spec.rb b/spec/lib/import/zendesk/object_attribute/decimal_spec.rb index 37014e780..e289e1bc4 100644 --- a/spec/lib/import/zendesk/object_attribute/decimal_spec.rb +++ b/spec/lib/import/zendesk/object_attribute/decimal_spec.rb @@ -1,6 +1,8 @@ require 'rails_helper' +require 'lib/import/zendesk/object_attribute/base_examples' RSpec.describe Import::Zendesk::ObjectAttribute::Decimal do + it_behaves_like Import::Zendesk::ObjectAttribute::Base it 'imports input object attribute from decimal object field' do diff --git a/spec/lib/import/zendesk/object_attribute/dropdown_spec.rb b/spec/lib/import/zendesk/object_attribute/dropdown_spec.rb index 6bc10fba9..03ca51b1d 100644 --- a/spec/lib/import/zendesk/object_attribute/dropdown_spec.rb +++ b/spec/lib/import/zendesk/object_attribute/dropdown_spec.rb @@ -1,6 +1,8 @@ require 'rails_helper' +require 'lib/import/zendesk/object_attribute/base_examples' RSpec.describe Import::Zendesk::ObjectAttribute::Dropdown do + it_behaves_like Import::Zendesk::ObjectAttribute::Base it 'imports select object attribute from dropdown object field' do diff --git a/spec/lib/import/zendesk/object_attribute/integer_spec.rb b/spec/lib/import/zendesk/object_attribute/integer_spec.rb index 964d1e7a6..4e60f0d0a 100644 --- a/spec/lib/import/zendesk/object_attribute/integer_spec.rb +++ b/spec/lib/import/zendesk/object_attribute/integer_spec.rb @@ -1,9 +1,11 @@ require 'rails_helper' +require 'lib/import/zendesk/object_attribute/base_examples' # required due to some of rails autoloading issues require 'import/zendesk/object_attribute/integer' RSpec.describe Import::Zendesk::ObjectAttribute::Integer do + it_behaves_like Import::Zendesk::ObjectAttribute::Base it 'imports integer object attribute from integer object field' do diff --git a/spec/lib/import/zendesk/object_attribute/regexp_spec.rb b/spec/lib/import/zendesk/object_attribute/regexp_spec.rb index 85b30908c..14aae8d86 100644 --- a/spec/lib/import/zendesk/object_attribute/regexp_spec.rb +++ b/spec/lib/import/zendesk/object_attribute/regexp_spec.rb @@ -1,9 +1,11 @@ require 'rails_helper' +require 'lib/import/zendesk/object_attribute/base_examples' # required due to some of rails autoloading issues require 'import/zendesk/object_attribute/regexp' RSpec.describe Import::Zendesk::ObjectAttribute::Regexp do + it_behaves_like Import::Zendesk::ObjectAttribute::Base it 'imports input object attribute from regexp object field' do diff --git a/spec/lib/import/zendesk/object_attribute/tagger_spec.rb b/spec/lib/import/zendesk/object_attribute/tagger_spec.rb index f5952f47b..be40f3fa9 100644 --- a/spec/lib/import/zendesk/object_attribute/tagger_spec.rb +++ b/spec/lib/import/zendesk/object_attribute/tagger_spec.rb @@ -1,6 +1,8 @@ require 'rails_helper' +require 'lib/import/zendesk/object_attribute/base_examples' RSpec.describe Import::Zendesk::ObjectAttribute::Tagger do + it_behaves_like Import::Zendesk::ObjectAttribute::Base it 'imports select object attribute from tagger object field' do diff --git a/spec/lib/import/zendesk/object_attribute/text_spec.rb b/spec/lib/import/zendesk/object_attribute/text_spec.rb index 5648a65a0..6841c9eaa 100644 --- a/spec/lib/import/zendesk/object_attribute/text_spec.rb +++ b/spec/lib/import/zendesk/object_attribute/text_spec.rb @@ -1,6 +1,8 @@ require 'rails_helper' +require 'lib/import/zendesk/object_attribute/base_examples' RSpec.describe Import::Zendesk::ObjectAttribute::Text do + it_behaves_like Import::Zendesk::ObjectAttribute::Base it 'imports input object attribute from text object field' do diff --git a/spec/lib/import/zendesk/object_attribute/textarea_spec.rb b/spec/lib/import/zendesk/object_attribute/textarea_spec.rb index f66209599..351523216 100644 --- a/spec/lib/import/zendesk/object_attribute/textarea_spec.rb +++ b/spec/lib/import/zendesk/object_attribute/textarea_spec.rb @@ -1,6 +1,8 @@ require 'rails_helper' +require 'lib/import/zendesk/object_attribute/base_examples' RSpec.describe Import::Zendesk::ObjectAttribute::Textarea do + it_behaves_like Import::Zendesk::ObjectAttribute::Base it 'imports input object attribute from textarea object field' do diff --git a/spec/lib/sequencer/unit/import/zendesk/sub_sequence/base_examples.rb b/spec/lib/sequencer/unit/import/zendesk/sub_sequence/base_examples.rb index 40c88b07d..3949caafc 100644 --- a/spec/lib/sequencer/unit/import/zendesk/sub_sequence/base_examples.rb +++ b/spec/lib/sequencer/unit/import/zendesk/sub_sequence/base_examples.rb @@ -1,61 +1,83 @@ RSpec.shared_examples 'Sequencer::Unit::Import::Zendesk::SubSequence::Base' do - before do - allow(params[:client]).to receive(collection_name).and_return(client_collection) - allow(client_collection).to receive(:all!).and_raise(api_error) - end + describe 'error handling' do + before do + allow(params[:client]).to receive(collection_name).and_return(client_collection) + allow(client_collection).to receive(:all!).and_raise(api_error) + end - let(:params) do - { - dry_run: false, - import_job: instance_double(ImportJob), - client: double('ZendeskAPI'), - group_map: {}, # required by Tickets - organization_map: {}, # required by Tickets - ticket_field_map: {}, # required by Tickets - user_map: {}, # required by Tickets - } - end + let(:params) do + { + dry_run: false, + import_job: instance_double(ImportJob), + client: double('ZendeskAPI'), + group_map: {}, # required by Tickets + organization_map: {}, # required by Tickets + ticket_field_map: {}, # required by Tickets + user_map: {}, # required by Tickets + } + end - let(:collection_name) { described_class.name.demodulize.snakecase.to_sym } - let(:client_collection) { double('ZendeskAPI::Collection') } - let(:api_error) { ZendeskAPI::Error::NetworkError.new('Mock err msg', response_obj) } - let(:response_obj) { double('Faraday::Response') } + let(:collection_name) { described_class.name.demodulize.snakecase.to_sym } + let(:client_collection) { double('ZendeskAPI::Collection') } + let(:api_error) { ZendeskAPI::Error::NetworkError.new('Mock err msg', response_obj) } - # https://github.com/zammad/zammad/issues/2262 - context 'for lowest-tier Zendesk subscriptions ("Essential")' do - shared_examples 'Zendesk import data (only available on Team tier and up)' do - context 'when API returns 403 forbidden during sync' do - before { allow(response_obj).to receive(:status).and_return(403) } + let(:response_obj) do + # stubbed methods required for ZendeskAPI::Error::ClientError#to_s + double('Faraday::Response', method: :get, url: 'https://example.com', status: 500) + end - it 'rescues the resulting exception' do - expect { process(params) }.not_to raise_error + # https://github.com/zammad/zammad/issues/2262 + context 'for lowest-tier Zendesk subscriptions ("Essential")' do + shared_examples 'Zendesk import data (only available on Team tier and up)' do + context 'when API returns 403 forbidden during sync' do + before { allow(response_obj).to receive(:status).and_return(403) } + + it 'rescues the resulting exception' do + expect { process(params) }.not_to raise_error + end + end + + context 'when API returns other errors' do + # https://github.com/zammad/zammad/issues/2262 + it 'does not rescue the resulting exception' do + expect do + process(params) do |unit| + allow(unit).to receive(:sleep) # stub out this method to speed up retry cycle + end + end + .to raise_error(api_error) + end end end - context 'when API returns other errors' do - before { allow(response_obj).to receive(:status).and_return(500) } + shared_examples 'Zendesk import data (available on all tiers)' do + context 'if API returns 403 forbidden during sync' do + before { allow(response_obj).to receive(:status).and_return(403) } - # https://github.com/zammad/zammad/issues/2262 - it 'does not rescue the resulting exception' do - expect { process(params) }.to raise_error(api_error) + it 'does not rescue the resulting exception' do + expect { process(params) }.to raise_error(api_error) + end end end + + if described_class.name.demodulize.in?(%w[UserFields OrganizationFields]) + include_examples 'Zendesk import data (only available on Team tier and up)' + else + include_examples 'Zendesk import data (available on all tiers)' + end end - shared_examples 'Zendesk import data (available on all tiers)' do - context 'if API returns 403 forbidden during sync' do - before { allow(response_obj).to receive(:status).and_return(403) } + context 'when DNS resolution fails (getaddrinfo: nodename nor servname provided, or not known)' do + it 'retries ten times, in 10s intervals' do + expect(client_collection) + .to receive(:all!).exactly(11).times - it 'does not rescue the resulting exception' do - expect { process(params) }.to raise_error(api_error) - end + expect do + process(params) do |unit| + expect(unit).to receive(:sleep).with(10).exactly(10).times + end + end.to raise_error(api_error) end end - - if described_class.name.demodulize.in?(%w[UserFields OrganizationFields]) - include_examples 'Zendesk import data (only available on Team tier and up)' - else - include_examples 'Zendesk import data (available on all tiers)' - end end end