From ab2cec55e77d9e5b1ebb17992ada6dc117b7eefd Mon Sep 17 00:00:00 2001 From: Zach Gollwitzer Date: Fri, 9 May 2025 14:56:49 -0400 Subject: [PATCH] Propagate child sync errors up to parent, fix sync status (#2232) * Propagate child sync errors up to parent, fix sync status * Remove testing error --- app/controllers/concerns/auto_sync.rb | 1 - app/models/account.rb | 2 -- app/models/concerns/syncable.rb | 6 ++++- app/models/family.rb | 2 -- app/models/plaid_item.rb | 2 -- app/models/sync.rb | 23 +++++++++++++------ .../20250509182903_dynamic_last_synced.rb | 7 ++++++ db/schema.rb | 5 +--- test/fixtures/families.yml | 3 --- 9 files changed, 29 insertions(+), 22 deletions(-) create mode 100644 db/migrate/20250509182903_dynamic_last_synced.rb diff --git a/app/controllers/concerns/auto_sync.rb b/app/controllers/concerns/auto_sync.rb index 970eec0a..4e375359 100644 --- a/app/controllers/concerns/auto_sync.rb +++ b/app/controllers/concerns/auto_sync.rb @@ -7,7 +7,6 @@ module AutoSync private def sync_family - Current.family.update!(last_synced_at: Time.current) Current.family.sync_later end diff --git a/app/models/account.rb b/app/models/account.rb index 85167e22..352335e0 100644 --- a/app/models/account.rb +++ b/app/models/account.rb @@ -82,8 +82,6 @@ class Account < ApplicationRecord end def sync_data(sync, start_date: nil) - update!(last_synced_at: Time.current) - Rails.logger.info("Processing balances (#{linked? ? 'reverse' : 'forward'})") sync_balances end diff --git a/app/models/concerns/syncable.rb b/app/models/concerns/syncable.rb index 0ae45122..d804b992 100644 --- a/app/models/concerns/syncable.rb +++ b/app/models/concerns/syncable.rb @@ -27,7 +27,11 @@ module Syncable end def sync_error - latest_sync.error + latest_sync&.error + end + + def last_synced_at + latest_sync&.last_ran_at end private diff --git a/app/models/family.rb b/app/models/family.rb index 3e877dc1..f565d54d 100644 --- a/app/models/family.rb +++ b/app/models/family.rb @@ -66,8 +66,6 @@ class Family < ApplicationRecord end def sync_data(sync, start_date: nil) - update!(last_synced_at: Time.current) - # We don't rely on this value to guard the app, but keep it eventually consistent sync_trial_status! diff --git a/app/models/plaid_item.rb b/app/models/plaid_item.rb index a527dc3e..2226f12f 100644 --- a/app/models/plaid_item.rb +++ b/app/models/plaid_item.rb @@ -38,8 +38,6 @@ class PlaidItem < ApplicationRecord end def sync_data(sync, start_date: nil) - update!(last_synced_at: Time.current) - begin Rails.logger.info("Fetching and loading Plaid data") fetch_and_load_plaid_data(sync) diff --git a/app/models/sync.rb b/app/models/sync.rb index 22b61829..826899ce 100644 --- a/app/models/sync.rb +++ b/app/models/sync.rb @@ -28,8 +28,7 @@ class Sync < ApplicationRecord Rails.logger.info("Post-sync completed") end rescue StandardError => error - fail! error - raise error if Rails.env.development? + fail! error, report_error: true ensure notify_parent_of_completion! if has_parent? end @@ -43,7 +42,11 @@ class Sync < ApplicationRecord self.lock! unless has_pending_child_syncs? - complete! + if has_failed_child_syncs? + fail!(Error.new("One or more child syncs failed")) + else + complete! + end # If this sync is both a child and a parent, we need to notify the parent of completion notify_parent_of_completion! if has_parent? @@ -58,6 +61,10 @@ class Sync < ApplicationRecord children.where(status: [ :pending, :syncing ]).any? end + def has_failed_child_syncs? + children.where(status: :failed).any? + end + def has_parent? parent_id.present? end @@ -76,12 +83,14 @@ class Sync < ApplicationRecord update! status: :completed, last_ran_at: Time.current end - def fail!(error) + def fail!(error, report_error: false) Rails.logger.error("Sync failed: #{error.message}") - Sentry.capture_exception(error) do |scope| - scope.set_context("sync", { id: id, syncable_type: syncable_type, syncable_id: syncable_id }) - scope.set_tags(sync_id: id) + if report_error + Sentry.capture_exception(error) do |scope| + scope.set_context("sync", { id: id, syncable_type: syncable_type, syncable_id: syncable_id }) + scope.set_tags(sync_id: id) + end end update!( diff --git a/db/migrate/20250509182903_dynamic_last_synced.rb b/db/migrate/20250509182903_dynamic_last_synced.rb new file mode 100644 index 00000000..6ade36eb --- /dev/null +++ b/db/migrate/20250509182903_dynamic_last_synced.rb @@ -0,0 +1,7 @@ +class DynamicLastSynced < ActiveRecord::Migration[7.2] + def change + remove_column :plaid_items, :last_synced_at, :datetime + remove_column :accounts, :last_synced_at, :datetime + remove_column :families, :last_synced_at, :datetime + end +end diff --git a/db/schema.rb b/db/schema.rb index 9a7e5fcf..6d76a125 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.2].define(version: 2025_05_02_164951) do +ActiveRecord::Schema[7.2].define(version: 2025_05_09_182903) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -34,7 +34,6 @@ ActiveRecord::Schema[7.2].define(version: 2025_05_02_164951) do t.uuid "import_id" t.uuid "plaid_account_id" t.boolean "scheduled_for_deletion", default: false - t.datetime "last_synced_at" t.decimal "cash_balance", precision: 19, scale: 4, default: "0.0" t.jsonb "locked_attributes", default: {} t.index ["accountable_id", "accountable_type"], name: "index_accounts_on_accountable_id_and_accountable_type" @@ -225,7 +224,6 @@ ActiveRecord::Schema[7.2].define(version: 2025_05_02_164951) do t.string "stripe_customer_id" t.string "date_format", default: "%m-%d-%Y" t.string "country", default: "US" - t.datetime "last_synced_at" t.string "timezone" t.boolean "data_enrichment_enabled", default: false t.boolean "early_access", default: false @@ -445,7 +443,6 @@ ActiveRecord::Schema[7.2].define(version: 2025_05_02_164951) do t.datetime "updated_at", null: false t.string "available_products", default: [], array: true t.string "billed_products", default: [], array: true - t.datetime "last_synced_at" t.string "plaid_region", default: "us", null: false t.string "institution_url" t.string "institution_id" diff --git a/test/fixtures/families.yml b/test/fixtures/families.yml index 9c6790b9..10d5bd18 100644 --- a/test/fixtures/families.yml +++ b/test/fixtures/families.yml @@ -1,8 +1,5 @@ empty: name: Family - last_synced_at: <%= Time.now %> dylan_family: name: The Dylan Family - last_synced_at: <%= Time.now %> -