[rfc] use spree default calculator when taxjar is down

[AdnanTheExcellent]

So..taxjar is currently down. Getting a lot of Rack::Timeout::RequestTimeoutException errors. What do you think would be the best way to handle that? rescuing StandardError does not catch that error.

[AdnanTheExcellent]

Instead of returning no_tax on rescue, what are your thoughts on defaulting to the Spree::TaxCalculator::Default calculator? If not set up, that would return no tax anyway. If a store has it set up enough to capture somewhat accurate tax (such as importing taxjars estimated rates), it would be better than nothing.

[jarednorman]

I'm certainly open to making that an option and it would be very easy to implement: just call the default calculator and return that.

[nvandoorn]

Hey @AdnanTheExcellent thanks for the RFC!

How do you feel about something like this?

# config/initializers/taxjar.rb

SuperGood::SolidusTaxjar.exception_handler = ->(e) {
  Rails.logger.error "An error occurred while fetching TaxJar tax rates - #{e}: #{e.message}"
  # If TaxJar is down, we swap in the default tax calculator.
  if e.is_a?(HTTP::ConnectionError)
    Spree::Config.tax_calculator_class = Spree::TaxCalculator::Default
    # This job is responsible for checking if TaxJar comes back up and then
    # setting `Spree::Config.tax_calculator_class` to
    # `SuperGood::SolidusTaxjar::TaxCalculator`.
    TaxjarPingJob.perform_later
  end
}

We feel it may be undesirable to silently call the default tax calculator as there are lots of ways you could handle TaxJar being down (some stores may have other calculators to fallback on, special logging, etc).

Suppose we did add some behavior around fallback calculators it could look like this:

# config/initializers/taxjar.rb

SuperGood::SolidusTaxjar.fallback_calculator = SomeCustomCalculator

[AdnanTheExcellent]

Hey @AdnanTheExcellent thanks for the RFC!

How do you feel about something like this?

# config/initializers/taxjar.rb

SuperGood::SolidusTaxjar.exception_handler = ->(e) {
  Rails.logger.error "An error occurred while fetching TaxJar tax rates - #{e}: #{e.message}"
  # If TaxJar is down, we swap in the default tax calculator.
  if e.is_a?(HTTP::ConnectionError)
    Spree::Config.tax_calculator_class = Spree::TaxCalculator::Default
    # This job is responsible for checking if TaxJar comes back up and then
    # setting `Spree::Config.tax_calculator_class` to
    # `SuperGood::SolidusTaxjar::TaxCalculator`.
    TaxjarPingJob.perform_later
  end
}

So i don't think this will work. timeouts are not a StandardError, so Taxjar timing out wont be caught by the exception handler as it is now. I had to add Rack::Timeout::RequestTimeoutException (what taxjar throws on timeout) to the rescue for my fallback to work:

      rescue StandardError, Rack::Timeout::RequestTimeoutException => e
        exception_handler.call(e)
        ::SuperGood::SolidusTaxJar.fallback_tax_calculator.new(order).calculate
      end

the fallback calculator could literally be no_tax and this will be the same functionality as now

We feel it may be undesirable to silently call the default tax calculator as there are lots of ways you could handle TaxJar being down (some stores may have other calculators to fallback on, special logging, etc).

Suppose we did add some behavior around fallback calculators it could look like this:

# config/initializers/taxjar.rb

SuperGood::SolidusTaxjar.fallback_calculator = SomeCustomCalculator

this is exactly what i'm trying to do. my fallback calculators look something like:

    self.fallback_tax_calculator = ::Spree::TaxCalculator::Default
    self.fallback_tax_rate_calculator =  -> (address) { ::Spree::TaxRate.for_address(address)&.first&.amount }

[AdnanTheExcellent]

On a slightly related note, i have a custom Zipcode activerecord model that i have extended to be a zone member, so i can make tax rates based off zipcode. This allows the fallback calculator to be ~99% accurate when taxjar is down, or when i want to quote based off a zipcode instead of wasting taxjar API calls to do it.

[Noah-Silvera]

@AdnanTheExcellent we think the request for a fallback calculator is reasonable and valuable. We have added two tickets to capture that work #61 #62 .

Let's continue the discussion on those tickets if you have more thoughts and close this RFC