diff --git a/modules/va_notify/lib/va_notify/default_callback.rb b/modules/va_notify/lib/va_notify/default_callback.rb index 3aa2efaa35c..fa48268675b 100644 --- a/modules/va_notify/lib/va_notify/default_callback.rb +++ b/modules/va_notify/lib/va_notify/default_callback.rb @@ -19,9 +19,18 @@ def call private + # rubocop:disable Metrics/MethodLength def call_with_metadata notification_type = metadata['notification_type'] - tags = validate_and_normalize_statsd_tags + + tags = if Flipper.enabled?(:va_notify_metadata_statsd_tags) + validate_and_normalize_statsd_tags + else + statsd_tags = metadata['statsd_tags'] + service = statsd_tags['service'] + function = statsd_tags['function'] + ["service:#{service}", "function:#{function}"] + end case notification_record.status when 'delivered' @@ -30,7 +39,15 @@ def call_with_metadata # 'temporary-failure' is an end state for the notification; VANotify API does not auto-retry these. permanent_failure(tags) if notification_type == 'error' end + rescue TypeError, KeyError => e + Rails.logger.error( + "VANotify: Invalid metadata format: #{e.message}", + notification_record_id: notification_record.id + ) + # Invalid metadata is treated as if no metadata were provided. + call_without_metadata end + # rubocop:enable Metrics/MethodLength def call_without_metadata case notification_record.status diff --git a/modules/va_notify/spec/lib/default_callback_spec.rb b/modules/va_notify/spec/lib/default_callback_spec.rb index e3f56931b1d..ef3d60eb098 100644 --- a/modules/va_notify/spec/lib/default_callback_spec.rb +++ b/modules/va_notify/spec/lib/default_callback_spec.rb @@ -73,11 +73,21 @@ build(:notification, status: 'delivered', callback_metadata:) end - it 'raises KeyError' do - expect do - VANotify::DefaultCallback.new(notification_record).call - end.to raise_error(KeyError, - 'Missing required keys in default_callback metadata statsd_tags: service, function') + it 'logs error and falls back to call_without_metadata' do + allow(Rails.logger).to receive(:error) + allow(StatsD).to receive(:increment) + + VANotify::DefaultCallback.new(notification_record).call + + expect(Rails.logger).to have_received(:error).with( + 'VANotify: Invalid metadata format: Missing required keys in default_callback metadata ' \ + 'statsd_tags: service, function', + { notification_record_id: notification_record.id } + ) + expect(StatsD).to have_received(:increment).with( + 'silent_failure_avoided', + tags: ['service:none-provided', 'function:none-provided'] + ) end end end @@ -143,28 +153,45 @@ build(:notification, status: 'delivered', callback_metadata:) end - it 'raises KeyError' do - expect do - VANotify::DefaultCallback.new(notification_record).call - end.to raise_error(KeyError, - 'Missing required keys in default_callback metadata statsd_tags: service, function') + it 'logs error and falls back to call_without_metadata' do + allow(Rails.logger).to receive(:error) + allow(StatsD).to receive(:increment) + + VANotify::DefaultCallback.new(notification_record).call + + expect(Rails.logger).to have_received(:error).with( + 'VANotify: Invalid metadata format: Missing required keys in default_callback metadata ' \ + 'statsd_tags: service, function', + { notification_record_id: notification_record.id } + ) + expect(StatsD).to have_received(:increment).with( + 'silent_failure_avoided', + tags: ['service:none-provided', 'function:none-provided'] + ) end end end context 'invalid metadata format is provided' do - context 'missing required statsd_tag keys' do - let(:callback_metadata) { 'this is not how we should pass metadata' } - let(:notification_record) do - build(:notification, status: 'delivered', callback_metadata:) - end + let(:callback_metadata) { 'this is not how we should pass metadata' } + let(:notification_record) do + build(:notification, status: 'delivered', callback_metadata:) + end - it 'raises KeyError' do - expect do - VANotify::DefaultCallback.new(notification_record).call - end.to raise_error(TypeError, - 'Invalid metadata statsd_tags format: must be a Hash or Array') - end + it 'logs error and falls back to call_without_metadata' do + allow(Rails.logger).to receive(:error) + allow(StatsD).to receive(:increment) + + VANotify::DefaultCallback.new(notification_record).call + + expect(Rails.logger).to have_received(:error).with( + 'VANotify: Invalid metadata format: Invalid metadata statsd_tags format: must be a Hash or Array', + { notification_record_id: notification_record.id } + ) + expect(StatsD).to have_received(:increment).with( + 'silent_failure_avoided', + tags: ['service:none-provided', 'function:none-provided'] + ) end end end