diff --git a/app/jobs/bill_subscription_job.rb b/app/jobs/bill_subscription_job.rb index 33a8ff482f1..8b500947407 100644 --- a/app/jobs/bill_subscription_job.rb +++ b/app/jobs/bill_subscription_job.rb @@ -19,10 +19,10 @@ def perform(subscriptions, timestamp, invoicing_reason:, invoice: nil, skip_char return if tax_error?(result) # If the invoice was passed as an argument, it means the job was already retried (see end of function) - result.raise_if_error! if invoice - - # If the invoice is in a retryable state, we'll re-enqueue the job manually, otherwise the job fails - result.raise_if_error! unless result.invoice&.generating? + if invoice || !result.invoice&.generating? + InvoiceError.create_for(invoice: result.invoice, error: result.error) + return result.raise_if_error! + end # On billing day, we'll retry the job further in the future because the system is typically under heavy load is_billing_date = invoicing_reason.to_sym == :subscription_periodic diff --git a/app/jobs/clock/retry_generating_subscription_invoices_job.rb b/app/jobs/clock/retry_generating_subscription_invoices_job.rb new file mode 100644 index 00000000000..51ab54b19e5 --- /dev/null +++ b/app/jobs/clock/retry_generating_subscription_invoices_job.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module Clock + class RetryGeneratingSubscriptionInvoicesJob < ApplicationJob + include SentryCronConcern + + queue_as 'clock' + + THRESHOLD = -> { 1.day.ago } + + def perform + Invoice.subscription.generating.where.not(id: InvoiceError.select(:id)).where('created_at < ?', THRESHOLD.call).find_each do |invoice| + next unless invoice.invoice_subscriptions.any? + invoicing_reasons = invoice.invoice_subscriptions.pluck(:invoicing_reason).uniq + invoicing_reason = (invoicing_reasons.size == 1) ? invoicing_reasons.first : :upgrading + BillSubscriptionJob.perform_later( + subscriptions: invoice.subscriptions.to_a, + timestamp: invoice.invoice_subscriptions.first.timestamp, + invoicing_reason:, + invoice:, + skip_charges: invoice.skip_charges + ) + end + end + end +end diff --git a/app/models/clickhouse/events_raw.rb b/app/models/clickhouse/events_raw.rb index ca229a240cd..4673fbe9733 100644 --- a/app/models/clickhouse/events_raw.rb +++ b/app/models/clickhouse/events_raw.rb @@ -50,13 +50,12 @@ def organization # # Table name: events_raw # -# code :string not null, primary key -# ingested_at :datetime not null +# code :string not null # precise_total_amount_cents :decimal(40, 15) # properties :string not null -# timestamp :datetime not null, primary key +# timestamp :datetime not null # external_customer_id :string not null -# external_subscription_id :string not null, primary key -# organization_id :string not null, primary key -# transaction_id :string not null, primary key +# external_subscription_id :string not null +# organization_id :string not null +# transaction_id :string not null # diff --git a/app/models/invoice_error.rb b/app/models/invoice_error.rb new file mode 100644 index 00000000000..35bf30fd1fe --- /dev/null +++ b/app/models/invoice_error.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class InvoiceError < ApplicationRecord + # NOTE! Invoice errors will have the same id as the invoice they belong to. + def self.create_for(invoice:, error:) + return unless invoice + + create(id: invoice.id, + backtrace: error.backtrace, + error: error.inspect.to_json, + invoice: invoice.to_json(except: :file), + subscriptions: invoice.subscriptions.to_json) + end +end + +# == Schema Information +# +# Table name: invoice_errors +# +# id :uuid not null, primary key +# backtrace :text +# error :json +# invoice :json +# subscriptions :json +# created_at :datetime not null +# updated_at :datetime not null +# diff --git a/clock.rb b/clock.rb index f32116ad8f7..b9e47fa27f0 100644 --- a/clock.rb +++ b/clock.rb @@ -58,6 +58,11 @@ module Clockwork .set(sentry: {"slug" => 'lago_bill_customers', "cron" => '10 */1 * * *'}) .perform_later end + every(1.hour, 'schedule:retry_generating_subscription_invoices', at: '*:30') do + Clock::RetryGeneratingSubscriptionInvoicesJob + .set(sentry: {"slug" => 'lago_retry_invoices', "cron" => '30 */1 * * *'}) + .perform_later + end every(1.hour, 'schedule:finalize_invoices', at: '*:20') do Clock::FinalizeInvoicesJob diff --git a/db/migrate/20241031102231_create_invoice_errors.rb b/db/migrate/20241031102231_create_invoice_errors.rb new file mode 100644 index 00000000000..163c01e43ce --- /dev/null +++ b/db/migrate/20241031102231_create_invoice_errors.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +class CreateInvoiceErrors < ActiveRecord::Migration[7.1] + def change + create_table :invoice_errors, id: :uuid do |t| + t.text :backtrace + t.json :invoice + t.json :subscriptions + t.json :error + + t.timestamps + end + end +end diff --git a/db/schema.rb b/db/schema.rb index d1000861bef..569b3e6d5be 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_10_25_081408) do +ActiveRecord::Schema[7.1].define(version: 2024_10_31_102231) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -779,6 +779,15 @@ t.index ["token"], name: "index_invites_on_token", unique: true end + create_table "invoice_errors", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| + t.text "backtrace" + t.json "invoice" + t.json "subscriptions" + t.json "error" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + create_table "invoice_metadata", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| t.uuid "invoice_id", null: false t.string "key", null: false diff --git a/spec/clockwork_spec.rb b/spec/clockwork_spec.rb index 170604c2e3b..74efa5e1e73 100644 --- a/spec/clockwork_spec.rb +++ b/spec/clockwork_spec.rb @@ -115,6 +115,27 @@ end end + describe 'schedule:retry_generating_subscription_invoices' do + let(:job) { 'schedule:retry_generating_subscription_invoices' } + let(:start_time) { Time.zone.parse('1 Apr 2022 00:01:00') } + let(:end_time) { Time.zone.parse('1 Apr 2022 01:01:00') } + + it 'enqueues a Clock::RetryGeneratingSubscriptionInvoiceJob' do + Clockwork::Test.run( + file: clock_file, + start_time:, + end_time:, + tick_speed: 1.second + ) + + expect(Clockwork::Test).to be_ran_job(job) + expect(Clockwork::Test.times_run(job)).to eq(1) + + Clockwork::Test.block_for(job).call + expect(Clock::RetryGeneratingSubscriptionInvoicesJob).to have_been_enqueued + end + end + describe 'schedule:compute_daily_usage' do let(:job) { 'schedule:compute_daily_usage' } let(:start_time) { Time.zone.parse('1 Apr 2022 00:01:00') } diff --git a/spec/jobs/bill_subscription_job_spec.rb b/spec/jobs/bill_subscription_job_spec.rb index e9d99abfed0..81ef36a04f3 100644 --- a/spec/jobs/bill_subscription_job_spec.rb +++ b/spec/jobs/bill_subscription_job_spec.rb @@ -24,7 +24,9 @@ context 'when result is a failure' do let(:result) do - BaseService::Result.new.single_validation_failure!(error_code: 'error') + result = BaseService::Result.new + result.invoice = invoice + result.single_validation_failure!(error_code: 'error') end it 'raises an error' do @@ -57,6 +59,15 @@ expect(Invoices::SubscriptionService).to have_received(:call) end + + it 'creates an InvoiceError' do + expect do + described_class.perform_now(subscriptions, timestamp, invoicing_reason:, invoice:) + end.to raise_error(BaseService::FailedResult) + + expect(InvoiceError.all.size).to eq(1) + expect(InvoiceError.first.id).to eq(invoice.id) + end end context 'when a generating invoice is attached to the result' do @@ -86,6 +97,15 @@ expect(Invoices::SubscriptionService).to have_received(:call) end + + it 'creates an InvoiceError' do + expect do + described_class.perform_now(subscriptions, timestamp, invoicing_reason:) + end.to raise_error(BaseService::FailedResult) + + expect(InvoiceError.all.size).to eq(1) + expect(InvoiceError.first.id).to eq(result_invoice.id) + end end end end diff --git a/spec/jobs/clock/retry_generating_subscription_invoices_job_spec.rb b/spec/jobs/clock/retry_generating_subscription_invoices_job_spec.rb new file mode 100644 index 00000000000..3a0591d50b8 --- /dev/null +++ b/spec/jobs/clock/retry_generating_subscription_invoices_job_spec.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Clock::RetryGeneratingSubscriptionInvoicesJob, job: true do + subject { described_class } + + describe '.perform' do + let(:old_generating_invoice) { create(:invoice, :generating, created_at: 5.days.ago) } + + before do + old_generating_invoice + end + + it "does not enqueue a BillSubscriptionJob for this invoice (missing subscriptions)" do + expect do + described_class.perform_now + end.not_to have_enqueued_job(BillSubscriptionJob) + end + + context "with an actual invoice that should be retried" do + let(:old_generating_invoice) { create(:invoice, :subscription, created_at: 5.days.ago) } + + before do + old_generating_invoice.update(status: :generating) + end + + it "does enqueue a BillSubscriptionJob for this invoice " do + expect do + described_class.perform_now + end.to have_enqueued_job(BillSubscriptionJob) + end + + context "with an existing invoice error" do + let(:invoice_error) { InvoiceError.create(id: old_generating_invoice.id) } + + before do + invoice_error + end + + it "does not enqueue a BillSubscriptionJob for this invoice" do + expect do + described_class.perform_now + end.not_to have_enqueued_job(BillSubscriptionJob) + end + end + end + + context "with an addon" do + let(:old_generating_invoice) { create(:invoice, :add_on, created_at: 5.days.ago) } + + before do + old_generating_invoice.update(status: :generating) + end + + it "does not enqueue a BillSubscriptionJob for this invoice" do + expect do + described_class.perform_now + end.not_to have_enqueued_job(BillSubscriptionJob) + end + end + end +end diff --git a/spec/models/invoice_error_spec.rb b/spec/models/invoice_error_spec.rb new file mode 100644 index 00000000000..2b234c763b5 --- /dev/null +++ b/spec/models/invoice_error_spec.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe InvoiceError, type: :model do + let(:invoice) { create(:invoice, :generating) } + let(:result) { BaseService::Result.new } + let(:error) { BaseService::ValidationFailure.new(result, messages: messages) } + let(:messages) { ["message1", "message2"] } + + let(:error_with_backtrace) do + error = OpenStruct.new + error.backtrace = "backtrace" + error + end + + describe ".create_for" do + it "does nothing if the invoice is nil" do + expect(described_class.create_for(invoice: nil, error:)).to eq(nil) + end + + it "creates an invoice error with the same id as the invoice" do + invoice_error = described_class.create_for(invoice:, error:) + expect(invoice_error.id).to eq(invoice.id) + end + + it "stores the error in the error field" do + invoice_error = described_class.create_for(invoice:, error:) + expect(invoice_error.error).to eq(error.inspect.to_json) + end + + it "stores the backtrace in the backtrace field" do + invoice_error = described_class.create_for(invoice:, error: error_with_backtrace) + expect(invoice_error.backtrace).to eq("backtrace") + end + + it "stores the subscriptions in the subscriptions field" do + invoice_error = described_class.create_for(invoice:, error:) + expect(invoice_error.subscriptions).to eq("[]") + end + end +end