Merge pull request #368 from fablabbcn/device-error-logging

Device error logging

app/lib/mqtt_messages_handler.rb

@@ -7,46 +7,46 @@ def handle_topic(topic, message, retry_on_nil_device=true)
     handshake_device(topic)
 
     if topic.to_s.include?('inventory')
-      handle_inventory(topic, message)
-    elsif topic.to_s.include?('raw')
-      handle_readings(topic, parse_raw_readings(message), retry_on_nil_device)
-    elsif topic.to_s.include?('readings')
-      handle_readings(topic, message, retry_on_nil_device)
-    elsif topic.to_s.include?('info')
-      handle_info(topic, message, retry_on_nil_device)
+      handle_inventory(message)
+      return true
     else
-      true
+      device = find_device_for_topic(topic, message, retry_on_nil_device)
+      return nil if device.nil?
+      with_device_error_handling(device, topic, message) do
+        if topic.to_s.include?('raw')
+          handle_readings(device, parse_raw_readings(message))
+        elsif topic.to_s.include?('readings')
+          handle_readings(device, message)
+        elsif topic.to_s.include?('info')
+          handle_info(device, message)
+        else
+          true
+        end
+      end
     end
   end
 
   private
 
-  def handle_inventory(topic, message)
+  def handle_inventory(message)
     DeviceInventory.create({ report: (message rescue nil) })
     return true
   end
 
-  def handle_readings(topic, message, retry_on_nil_device)
-    device = find_device_for_topic(topic, message, retry_on_nil_device)
-    return nil if device.nil?
-
+  def handle_readings(device, message)
     parsed = JSON.parse(message) if message
     data = parsed["data"] if parsed
     return nil if data.nil? or data&.empty?
-
     data.each do |reading|
       storer.store(device, reading)
     end
-
     return true
   rescue Exception => e
     Sentry.capture_exception(e)
     raise e if Rails.env.test?
   end
 
-  def handle_info(topic, message, retry_on_nil_device)
-    device = find_device_for_topic(topic, message, retry_on_nil_device)
-    return nil if device.nil?
+  def handle_info(device, message)
     json_message = JSON.parse(message)
     device.update_column(:hardware_info, json_message)
     return true
@@ -88,6 +88,41 @@ def handle_nil_device(topic, message, retry_on_nil_device)
     end
   end
 
+  def with_device_error_handling(device, topic, message, reraise=true, &block)
+    begin
+      block.call
+    rescue Exception => e
+      hardware_info = device.hardware_info
+      Sentry.set_tags({
+        "device-id": device.id,
+        "device-hardware-version": hardware_info&.[]("hw_ver"),
+        "device-esp-version": hardware_info&.[]("esp_ver"),
+        "device-sam-version": hardware_info&.[]("sam_ver"),
+      })
+      Sentry.capture_exception(e)
+      last_error = device.ingest_errors.order(created_at: :desc).first
+      ingest_error = device.ingest_errors.create({
+        topic: topic,
+        message: message,
+        error_class: e.class.name,
+        error_message: e.message,
+        error_trace: e.full_message
+      })
+      if send_device_error_warnings && (!last_error || last_error.created_at < device_error_warning_threshold)
+        UserMailer.device_ingest_errors(device.id).deliver_later
+      end
+      raise e if reraise
+    end
+  end
+
+  def send_device_error_warnings
+    ENV.fetch("SEND_DEVICE_ERROR_WARNINGS", false)
+  end
+
+  def device_error_warning_threshold
+    ENV.fetch("DEVICE_ERROR_WARNING_THRESHOLD_HOURS", "6").to_i.hours.ago
+  end
+
   def device_token(topic)
     device_token = topic[/device\/sck\/(.*?)\//m, 1].to_s
   end

app/lib/raw_mqtt_message_parser.rb

@@ -6,7 +6,9 @@ def initialize
   end
 
   def parse(message)
-    parser.parse(self.convert_to_ascii(message.strip))&.to_hash
+    message = parser.parse(self.convert_to_ascii(message.strip))&.to_hash
+    raise "Message not parsed: #{message}" unless message
+    return message
   end
 
   private

app/mailers/user_mailer.rb

@@ -32,4 +32,10 @@ def device_stopped_publishing device_id
     mail to: @user.to_email_s, subject: 'Device stopped publishing', from: "SmartCitizen Notifications - Device <notifications@mailbot.smartcitizen.me>"
   end
 
+  def device_ingest_errors(device_id)
+    @device = Device.find(device_id)
+    @user = @device.owner
+    mail to: @user.to_email_s, subject: "Device has errors", from: "SmartCitizen Notifications - Device <notifications@mailbot.smartcitizen.me>"
+  end
+
 end

app/models/device.rb

@@ -25,6 +25,7 @@ class Device < ActiveRecord::Base
   has_many :components, dependent: :destroy
   has_many :sensors, through: :components
   has_one :postprocessing, dependent: :destroy
+  has_many :ingest_errors, dependent: :destroy
 
   has_and_belongs_to_many :experiments
 

app/models/ingest_error.rb

@@ -0,0 +1,3 @@
+class IngestError < ActiveRecord::Base
+  belongs_to :device
+end

app/views/user_mailer/device_ingest_errors.html.erb

@@ -0,0 +1,66 @@
+<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
+<html xmlns="http://www.w3.org/1999/xhtml">
+  <head>
+    <meta name="viewport" content="width=device-width" />
+    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8" />
+    <title>Device has errrors</title>
+    <%= render "email_css" %>
+  </head>
+
+  <body itemscope itemtype="http://schema.org/EmailMessage">
+
+    <table class="body-wrap">
+      <tr>
+        <td></td>
+        <td class="container" width="600">
+          <div class="content">
+            <table class="main" width="100%" cellpadding="0" cellspacing="0">
+              <tr>
+                <td class="alert alert-warning">
+                  <h3 class="mail-title">
+                    <img class="text-white" src="https://fablabbcn.github.io/assets/images/sck_logo_box.png" alt="SCK logo">
+                  </h3>
+                </td>
+              </tr>
+              <tr>
+                <td class="content-wrap">
+                  <table width="100%" cellpadding="0" cellspacing="0">
+                    <tr>
+                      <td class="content-block">
+                        <h2 class="corporate-text-color"><%= @user.username %>,</h2>
+                      </td>
+                    </tr>
+                    <tr>
+                      <td class="content-block">
+                        <p class="corporate-text-color">
+                        We have encountered errors on processing data from the
+                        <a href="https://smartcitizen.me/kits/<%= @device.id %>" class="">device '<%= @device %>'</a>.
+                        </p>
+                        <p>
+                        If these errors persist, you may want to <a href="https://docs.smartcitizen.me/Guides/firmware/Update%20the%20firmware/">update the device firmware</a>, or <a href="mailto:support@smartcitizen.me">get in touch with our support team</a>.
+                        </p>
+                      </td>
+                    </tr>
+                    <tr>
+                      <td class="content-block">
+                        <a href="https://smartcitizen.me/kits/<%= @device.id %>/edit" class="btn-primary">Manage your notifications</a>
+                      </td>
+                    </tr>
+                    <tr>
+                      <td class="content-block">
+                        <p class="corporate-text-color">The Smart Citizen Team</p>
+                      </td>
+                    </tr>
+                  </table>
+                </td>
+              </tr>
+            </table>
+
+          </div>
+        </td>
+        <td></td>
+      </tr>
+    </table>
+
+  </body>
+</html>

db/migrate/20241014052837_create_device_ingest_errors.rb

@@ -0,0 +1,13 @@
+class CreateDeviceIngestErrors < ActiveRecord::Migration[6.1]
+  def change
+    create_table :ingest_errors do |t|
+      t.references :device, null: false, foreign_key: true
+      t.text :topic
+      t.text :message
+      t.text :error_class
+      t.text :error_message
+      t.text :error_trace
+      t.timestamps
+    end
+  end
+end

db/schema.rb

@@ -10,7 +10,8 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2024_10_09_174732) do
+ActiveRecord::Schema.define(version: 2024_10_14_052837) do
+
   # These are extensions that must be enabled in order to support this database
   enable_extension "adminpack"
   enable_extension "hstore"
@@ -161,6 +162,18 @@
     t.index ["sluggable_type"], name: "index_friendly_id_slugs_on_sluggable_type"
   end
 
+  create_table "ingest_errors", force: :cascade do |t|
+    t.bigint "device_id", null: false
+    t.text "topic"
+    t.text "message"
+    t.text "error_class"
+    t.text "error_message"
+    t.text "error_trace"
+    t.datetime "created_at", precision: 6, null: false
+    t.datetime "updated_at", precision: 6, null: false
+    t.index ["device_id"], name: "index_ingest_errors_on_device_id"
+  end
+
   create_table "measurements", id: :serial, force: :cascade do |t|
     t.string "name"
     t.text "description"
@@ -338,6 +351,7 @@
   add_foreign_key "devices_tags", "devices"
   add_foreign_key "devices_tags", "tags"
   add_foreign_key "experiments", "users", column: "owner_id"
+  add_foreign_key "ingest_errors", "devices"
   add_foreign_key "postprocessings", "devices"
   add_foreign_key "sensors", "measurements"
   add_foreign_key "uploads", "users"

spec/lib/raw_mqtt_message_parser_spec.rb

@@ -59,10 +59,9 @@
     expect(parsed).to eq({ data: [ { recorded_at: "2024-09-25T13:19:38Z", sensors: [{id: "100", value: "-2000.12345"}] }]})
   end
 
-  it "returns nil if no valid message parsed" do
+  it "raises an error if no valid message parsed" do
     message = "ceci n'est pas un message"
-    parsed = parser.parse(message)
-    expect(parsed).to eq(nil)
+    expect {parser.parse(message)}.to raise_error(RuntimeError)
   end
 
   it "parses timestamps at any position in the packet" do