Skip to content

Commit

Permalink
Better handle API failures
Browse files Browse the repository at this point in the history
The previous implementation just let the error bubble up. This means
we might loose a sale due to sales tax. Most stores would rather just
pay the sales tax out-of-pocket in these rare events over loosing
a sale.

It might be worth making this an option if there are stores that prefer
to loose the sale but I'll leave it to those stores to implement that.

The system basically treats it as if the order it outside the nexus
(no tax collected) if it cannot contact the API.

To ensure accurate reporting it leaves a log message that is prefixed
with "Taxjar Failure". THe logs should be searched before reporting
sales tax to see if there are any of these.

If a more active notification is desired a log service can be used to
look for that string and send an e-mail to the right person when it
happens.
  • Loading branch information
eric1234 committed Nov 28, 2017
1 parent 41e9d77 commit 15b7981
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 0 deletions.
12 changes: 12 additions & 0 deletions app/models/spree/taxjar.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ def create_transaction_for_order
Rails.logger.debug order: {id: @order.id, number: @order.number}, api_response: api_response
api_response
end
rescue HTTP::Error
Rails.logger.error "Taxjar Failure: Failed to create transaction for order #{@order.number}"
# Silently ignore
end

def delete_transaction_for_order
Expand All @@ -52,6 +55,9 @@ def calculate_tax_for_shipment
else
0
end
rescue HTTP::Error
Rails.logger.error "Taxjar Failure: Failed to calculate tax for shipment #{@shipment.id} (#{@shipment.order.number})"
0
end

def has_nexus?
Expand All @@ -65,6 +71,9 @@ def has_nexus?
else
false
end
rescue HTTP::Error
Rails.logger.error "Taxjar Failure: Failed to determine nexus for #{@order.number}"
false
end

def calculate_tax_for_order
Expand All @@ -76,6 +85,9 @@ def calculate_tax_for_order
Rails.logger.debug order: {id: @order.id, number: @order.number}, api_response: api_response
api_response
end
rescue HTTP::Error
Rails.logger.error "Taxjar Failure: Failed to calcuate tax for #{@order.number}"
nil
end

private
Expand Down
33 changes: 33 additions & 0 deletions spec/models/spree/taxjar_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,17 @@
end
end

context 'api is down' do
before :each do
allow(::Taxjar::Client).to receive(:new).with(api_key: taxjar_api_key).and_return(client)
allow( client ).to receive(:nexus_regions).and_raise HTTP::ConnectionError
end

it 'should return true' do
expect( spree_taxjar.has_nexus? ).to eq false
end
end

context 'nexus_regions is present' do
context 'tax_address is present in nexus regions' do
it 'should return true' do
Expand Down Expand Up @@ -85,6 +96,17 @@
end
end

context 'api is down' do
before :each do
allow(::Taxjar::Client).to receive(:new).with(api_key: taxjar_api_key).and_return(client)
allow( client ).to receive(:nexus_regions).and_raise HTTP::ConnectionError
end

it 'should return nil' do
expect(spree_taxjar.create_transaction_for_order).to eq nil
end
end

context 'when has_nexus? returns true' do
before do
allow(::Taxjar::Client).to receive(:new).with(api_key: taxjar_api_key).and_return(client)
Expand Down Expand Up @@ -152,6 +174,17 @@
after { spree_taxjar.calculate_tax_for_order }

end

context 'api is down' do
before :each do
allow(::Taxjar::Client).to receive(:new).with(api_key: taxjar_api_key).and_return(client)
allow( client ).to receive(:nexus_regions).and_raise HTTP::ConnectionError
end

it 'should return nil' do
expect(spree_taxjar.calculate_tax_for_order).to eq nil
end
end
end

end
Expand Down

0 comments on commit 15b7981

Please sign in to comment.