fix(enable-banking): refactor error handling and add missing GIN index (#1432)

* fix(enable-banking): refactor error handling and add missing GIN index

* fix(enable-banking): handle wrapped network errors and fix concurrent index migration

* fix(enable-banking): extract network errors to frozen constant

* fix(enable-banking): consolidate error handling and enforce strict localization

* fix(enable-banking): improve sync error handling and fix invalid test status

* test(enable_banking): use OpenStruct instead of mock for provider

app/models/enable_banking_item/importer.rb

@@ -3,6 +3,14 @@ class EnableBankingItem::Importer
   # Enable Banking typically returns ~100 transactions per page, so 100 pages = ~10,000 transactions
   MAX_PAGINATION_PAGES = 100
 
+  NETWORK_ERRORS = [
+    ::SocketError,
+    ::Errno::ECONNREFUSED,
+    ::Timeout::Error,
+    ::Net::ReadTimeout,
+    ::Net::OpenTimeout
+  ].freeze
+
   attr_reader :enable_banking_item, :enable_banking_provider
 
   def initialize(enable_banking_item, enable_banking_provider:)
@@ -13,12 +21,12 @@ class EnableBankingItem::Importer
   def import
     unless enable_banking_item.session_valid?
       enable_banking_item.update!(status: :requires_update)
-      return { success: false, error: "Session expired or invalid", accounts_updated: 0, transactions_imported: 0 }
+      return { success: false, error: I18n.t("enable_banking_items.errors.session_invalid"), accounts_updated: 0, transactions_imported: 0 }
     end
 
     session_data = fetch_session_data
     unless session_data
-      error_msg = @session_error || "Failed to fetch session data"
+      error_msg = @session_error || I18n.t("enable_banking_items.errors.unexpected")
       return { success: false, error: error_msg, accounts_updated: 0, transactions_imported: 0 }
     end
 
@@ -68,6 +76,7 @@ class EnableBankingItem::Importer
           end
         rescue => e
           accounts_failed += 1
+          @sync_error = promote_session_invalid(@sync_error, handle_sync_error(e))
           Rails.logger.error "EnableBankingItem::Importer - Failed to update account #{uid}: #{e.message}"
         end
       end
@@ -81,16 +90,22 @@ class EnableBankingItem::Importer
 
     linked_accounts_query.each do |enable_banking_account|
       begin
-        fetch_and_update_balance(enable_banking_account)
+        unless fetch_and_update_balance(enable_banking_account)
+          transactions_failed += 1
+          # @sync_error already set in fetch_and_update_balance
+          next
+        end
 
         result = fetch_and_store_transactions(enable_banking_account)
         if result[:success]
           transactions_imported += result[:transactions_count]
         else
           transactions_failed += 1
+          @sync_error = promote_session_invalid(@sync_error, result[:error])
         end
       rescue => e
         transactions_failed += 1
+        @sync_error = promote_session_invalid(@sync_error, handle_sync_error(e))
         Rails.logger.error "EnableBankingItem::Importer - Failed to process account #{enable_banking_account.uid}: #{e.message}"
       end
     end
@@ -102,46 +117,48 @@ class EnableBankingItem::Importer
       transactions_imported: transactions_imported,
       transactions_failed: transactions_failed
     }
-    if !result[:success] && (accounts_failed > 0 || transactions_failed > 0)
-      parts = []
-      parts << "#{accounts_failed} #{'account'.pluralize(accounts_failed)} failed" if accounts_failed > 0
-      parts << "#{transactions_failed} #{'transaction'.pluralize(transactions_failed)} failed" if transactions_failed > 0
-      result[:error] = parts.join(", ")
-    end
+
+    result[:error] = @sync_error || I18n.t("enable_banking_items.errors.unexpected") if !result[:success]
     result
   end
 
   private
 
-    def extract_friendly_error_message(exception)
-      [ exception, exception.cause ].compact.each do |ex|
-        case ex
-        when SocketError then return "DNS resolution failed: check your network/DNS configuration"
-        when Net::OpenTimeout, Net::ReadTimeout then return "Connection timed out: the Enable Banking API may be unreachable"
-        when Errno::ECONNREFUSED then return "Connection refused: the Enable Banking API is unreachable"
-        end
+    def handle_sync_error(exception)
+      # Check the underlying cause first, then the exception itself
+      exceptions = [ exception.cause, exception ].compact
+
+      provider_error = exceptions.find { |ex| ex.is_a?(Provider::EnableBanking::EnableBankingError) }
+
+      # Handle session expiration status update
+      if provider_error && [ :unauthorized, :not_found ].include?(provider_error.error_type)
+        enable_banking_item.update!(status: :requires_update)
+        return I18n.t("enable_banking_items.errors.session_invalid")
       end
 
-      msg = exception.message.to_s
-      return "DNS resolution failed: check your network/DNS configuration" if msg.include?("getaddrinfo") || msg.match?(/name or service not known/i)
-      return "Connection timed out: the Enable Banking API may be unreachable" if msg.include?("execution expired") || msg.include?("timeout") || msg.match?(/timed out/i)
-      return "Connection refused: the Enable Banking API is unreachable" if msg.include?("ECONNREFUSED") || msg.match?(/connection refused/i)
+      is_network_error = exceptions.any? do |ex|
+        NETWORK_ERRORS.any? { |err| ex.is_a?(err) } ||
+          (ex.is_a?(Provider::EnableBanking::EnableBankingError) && [ :request_failed, :timeout ].include?(ex.error_type))
+      end
 
-      msg
+      if is_network_error
+        I18n.t("enable_banking_items.errors.network_unreachable")
+      elsif provider_error
+        I18n.t("enable_banking_items.errors.api_error")
+      else
+        I18n.t("enable_banking_items.errors.unexpected")
+      end
     end
 
     def fetch_session_data
       enable_banking_provider.get_session(session_id: enable_banking_item.session_id)
     rescue Provider::EnableBanking::EnableBankingError => e
-      if e.error_type == :unauthorized || e.error_type == :not_found
-        enable_banking_item.update!(status: :requires_update)
-      end
       Rails.logger.error "EnableBankingItem::Importer - Enable Banking API error: #{e.message}"
-      @session_error = extract_friendly_error_message(e)
+      @session_error = handle_sync_error(e)
       nil
     rescue => e
       Rails.logger.error "EnableBankingItem::Importer - Unexpected error fetching session: #{e.class} - #{e.message}"
-      @session_error = extract_friendly_error_message(e)
+      @session_error = handle_sync_error(e)
       nil
     end
 
@@ -165,7 +182,7 @@ class EnableBankingItem::Importer
       # Enable Banking returns an array of balances. We prioritize types based on reliability.
       # closingBooked (CLBD) > interimAvailable (ITAV) > expected (XPCD)
       balances = balance_data[:balances] || []
-      return if balances.empty?
+      return true if balances.empty?
 
       priority_types = [ "CLBD", "ITAV", "XPCD", "CLAV", "ITBD" ]
       balance = nil
@@ -195,8 +212,17 @@ class EnableBankingItem::Importer
           )
         end
       end
+      true
     rescue Provider::EnableBanking::EnableBankingError => e
+      @sync_error = promote_session_invalid(@sync_error, handle_sync_error(e))
       Rails.logger.error "EnableBankingItem::Importer - Error fetching balance for account #{enable_banking_account.uid}: #{e.message}"
+      false
+    end
+
+    def promote_session_invalid(existing, new)
+      return new if existing.nil?
+      return new if new == I18n.t("enable_banking_items.errors.session_invalid")
+      existing
     end
 
     def fetch_and_store_transactions(enable_banking_account)
@@ -286,10 +312,10 @@ class EnableBankingItem::Importer
       { success: true, transactions_count: transactions_count }
     rescue Provider::EnableBanking::EnableBankingError => e
       Rails.logger.error "EnableBankingItem::Importer - Error fetching transactions for account #{enable_banking_account.uid}: #{e.message}"
-      { success: false, transactions_count: 0, error: e.message }
+      { success: false, transactions_count: 0, error: handle_sync_error(e) }
     rescue => e
       Rails.logger.error "EnableBankingItem::Importer - Unexpected error fetching transactions for account #{enable_banking_account.uid}: #{e.class} - #{e.message}"
-      { success: false, transactions_count: 0, error: e.message }
+      { success: false, transactions_count: 0, error: handle_sync_error(e) }
     end
 
     # Deduplicate transactions from the Enable Banking API response.

config/locales/views/enable_banking_items/en.yml

@@ -1,6 +1,11 @@
 ---
 en:
   enable_banking_items:
+    errors:
+      api_error: "A communication error occurred with the bank."
+      network_unreachable: "The banking service is temporarily unreachable. Please try again later."
+      session_invalid: "Session expired. Please reconnect your bank."
+      unexpected: "An unexpected error occurred during synchronization."
     authorize:
       authorization_failed: "Failed to initiate authorization: %{message}"
       bank_required: Please select a bank.

config/locales/views/enable_banking_items/fr.yml

@@ -1,6 +1,11 @@
 ---
 fr:
   enable_banking_items:
+    errors:
+      api_error: "Erreur de communication avec la banque."
+      network_unreachable: "Le service bancaire est temporairement injoignable. Veuillez réessayer plus tard."
+      session_invalid: "Session expirée. Veuillez reconnecter votre banque."
+      unexpected: "Une erreur inattendue est survenue lors de la synchronisation."
     authorize:
       authorization_failed: Échec de l'initiation de l'autorisation
       bank_required: Veuillez sélectionner une banque.

db/migrate/20260411082125_add_gin_index_to_enable_banking_accounts_identification_hashes.rb

@@ -0,0 +1,7 @@
+class AddGinIndexToEnableBankingAccountsIdentificationHashes < ActiveRecord::Migration[7.2]
+  disable_ddl_transaction!
+
+  def change
+    add_index :enable_banking_accounts, :identification_hashes, using: :gin, algorithm: :concurrently
+  end
+end

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: 2026_04_10_114435) do
+ActiveRecord::Schema[7.2].define(version: 2026_04_11_082125) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pgcrypto"
   enable_extension "plpgsql"
@@ -408,6 +408,7 @@ ActiveRecord::Schema[7.2].define(version: 2026_04_10_114435) do
     t.jsonb "identification_hashes", default: []
     t.index ["account_id"], name: "index_enable_banking_accounts_on_account_id"
     t.index ["enable_banking_item_id"], name: "index_enable_banking_accounts_on_enable_banking_item_id"
+    t.index ["identification_hashes"], name: "index_enable_banking_accounts_on_identification_hashes", using: :gin
   end
 
   create_table "enable_banking_items", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|

test/models/enable_banking_item/importer_error_handling_test.rb

@@ -0,0 +1,76 @@
+require "test_helper"
+require "ostruct"
+
+class EnableBankingItem::ImporterErrorHandlingTest < ActiveSupport::TestCase
+  setup do
+    @family = families(:dylan_family)
+    @enable_banking_item = EnableBankingItem.create!(
+      family: @family,
+      name: "Test Enable Banking",
+      country_code: "AT",
+      application_id: "test_app_id",
+      client_certificate: "test_cert",
+      session_id: "test_session",
+      session_expires_at: 1.day.from_now,
+      status: :good
+    )
+
+    @mock_provider = OpenStruct.new
+    @importer = EnableBankingItem::Importer.new(@enable_banking_item, enable_banking_provider: @mock_provider)
+  end
+
+  test "handle_sync_error handles unauthorized EnableBankingError" do
+    error = Provider::EnableBanking::EnableBankingError.new("Unauthorized", :unauthorized)
+    message = @importer.send(:handle_sync_error, error)
+
+    assert_equal I18n.t("enable_banking_items.errors.session_invalid"), message
+    assert @enable_banking_item.reload.requires_update?
+  end
+
+  test "handle_sync_error handles not_found EnableBankingError" do
+    error = Provider::EnableBanking::EnableBankingError.new("Not Found", :not_found)
+    message = @importer.send(:handle_sync_error, error)
+
+    assert_equal I18n.t("enable_banking_items.errors.session_invalid"), message
+    assert @enable_banking_item.reload.requires_update?
+  end
+
+  test "handle_sync_error handles other EnableBankingError as api_error" do
+    error = Provider::EnableBanking::EnableBankingError.new("Some API error", :internal_server_error)
+    message = @importer.send(:handle_sync_error, error)
+
+    assert_equal I18n.t("enable_banking_items.errors.api_error"), message
+    assert_not @enable_banking_item.reload.requires_update?
+  end
+
+  test "fetch_session_data updates status to requires_update on unauthorized error" do
+    def @mock_provider.get_session(**args)
+      raise Provider::EnableBanking::EnableBankingError.new("Unauthorized", :unauthorized)
+    end
+
+    @importer.send(:fetch_session_data)
+
+    assert @enable_banking_item.reload.requires_update?
+  end
+
+  test "fetch_and_store_transactions updates status to requires_update on unauthorized error" do
+    enable_banking_account = EnableBankingAccount.new(uid: "test_uid")
+    @importer.stubs(:determine_sync_start_date).returns(Date.today)
+    @importer.expects(:fetch_paginated_transactions).raises(Provider::EnableBanking::EnableBankingError.new("Unauthorized", :unauthorized))
+
+    @importer.send(:fetch_and_store_transactions, enable_banking_account)
+
+    assert @enable_banking_item.reload.requires_update?
+  end
+
+  test "fetch_and_update_balance updates status to requires_update on unauthorized error" do
+    enable_banking_account = EnableBankingAccount.new(uid: "test_uid")
+    def @mock_provider.get_account_balances(**args)
+      raise Provider::EnableBanking::EnableBankingError.new("Unauthorized", :unauthorized)
+    end
+
+    @importer.send(:fetch_and_update_balance, enable_banking_account)
+
+    assert @enable_banking_item.reload.requires_update?
+  end
+end