Deferred retry of hardware_info handling when device not found

Since we deployed the devices refactor, a slightly subtle bug has
emerged: if a device sends the hardware_info packet before the user has
completed registration, the device is not found, the info is not saved,
and therefore the device information in the UI is incomplete. This
(hopefully, I like to test on staging with a real device) fixes this by
relying on Sidekiq's built in error handling functionality to retry the
hardware_info message (with backoff) in the event that no device
corresponds to the given key. Sidekiq defaults to 25 retries with
backoff (corresponding to about 3 weeks of elapsed time between the
first and the last try), which should be sufficient:
https://docs.gitlab.com/ee/development/sidekiq/.

app/jobs/retry_mqtt_message_job.rb

@@ -0,0 +1,10 @@
+class RetryMQTTMessageJob < ApplicationJob
+  queue_as :default
+
+  def perform(topic, message)
+    result = MqttMessagesHandler.handle_topic(topic, message, false)
+    raise "Message handler returned nil, retrying" if result.nil?
+  end
+end
+
+

app/lib/mqtt_messages_handler.rb

@@ -1,5 +1,5 @@
 class MqttMessagesHandler
-  def self.handle_topic(topic, message)
+  def self.handle_topic(topic, message, retry_on_nil_device=true)
     Sentry.set_tags('mqtt-topic': topic)
 
     crumb = Sentry::Breadcrumb.new(
@@ -19,7 +19,10 @@ def self.handle_topic(topic, message)
     end
 
     device = Device.find_by(device_token: device_token(topic))
-    return if device.nil?
+    if device.nil?
+      handle_nil_device(topic, message, retry_on_nil_device)
+      return nil
+    end
 
     if topic.to_s.include?('raw')
       handle_readings(device, parse_raw_readings(message, device.id))
@@ -42,6 +45,16 @@ def self.handle_topic(topic, message)
     end
   end
 
+  def self.handle_nil_device(topic, message, retry_on_nil_device)
+    if topic.to_s.include?("info")
+      retry_later(topic, message) if retry_on_nil_device
+    end
+  end
+
+  def self.retry_later(topic, message)
+    RetryMQTTMessageJob.perform_later(topic, message)
+  end
+
   # takes a packet and stores data
   def self.handle_readings(device, message)
     data = self.data(message)

spec/jobs/retry_mqtt_message_job_spec.rb

@@ -0,0 +1,19 @@
+require 'rails_helper'
+
+RSpec.describe RetryMQTTMessageJob, type: :job do
+  it "retries the mqtt ingest with the given topic and message, and with automatic retries disabled" do
+    topic = "topic/1/2/3"
+    message = '{"foo": "bar", "test": "message"}'
+    expect(MqttMessagesHandler).to receive(:handle_topic).with(topic, message, false).and_return(true)
+    RetryMQTTMessageJob.perform_now(topic, message)
+  end
+
+  it "raises an error if the handler returns nil" do
+    topic = "topic/1/2/3"
+    message = '{"foo": "bar", "test": "message"}'
+    expect(MqttMessagesHandler).to receive(:handle_topic).with(topic, message, false).and_return(nil)
+    expect {
+      RetryMQTTMessageJob.perform_now(topic, message)
+    }.to raise_error
+  end
+end

spec/lib/mqtt_messages_handler_spec.rb

@@ -243,12 +243,22 @@
       expect(orphan_device.reload.device_handshake).to be true
     end
 
-    it 'does not handle bad topic' do
+    it 'defers messages with unknown device tokens if retry flag is true' do
       expect(device.hardware_info["id"]).to eq(47)
+      expect(RetryMQTTMessageJob).to receive(:perform_later).with(@hardware_info_packet_bad.topic, @hardware_info_packet_bad.payload)
       MqttMessagesHandler.handle_topic(@hardware_info_packet_bad.topic, @hardware_info_packet_bad.payload)
       device.reload
       expect(device.hardware_info["id"]).to eq(47)
       expect(@hardware_info_packet_bad.payload).to_not eq((device.hardware_info.to_json))
     end
+
+    it 'does not defer messages with unknown device tokens if retry flag is false' do
+      expect(device.hardware_info["id"]).to eq(47)
+      expect(RetryMQTTMessageJob).not_to receive(:perform_later).with(@hardware_info_packet_bad.topic, @hardware_info_packet_bad.payload)
+      MqttMessagesHandler.handle_topic(@hardware_info_packet_bad.topic, @hardware_info_packet_bad.payload, false)
+      device.reload
+      expect(device.hardware_info["id"]).to eq(47)
+      expect(@hardware_info_packet_bad.payload).to_not eq((device.hardware_info.to_json))
+    end
   end
 end