Update - handling metadata with incorrect keys

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

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