fix(lib): a malformed saved record or send argument no longer stops a driver, and errors keep their stack

template/src/lib/bindings.lua

@@ -111,6 +111,22 @@ local PROXY_BINDING_END = 5999
 --- @field displayName string
 --- @field class string
 
+--- Calls `fn` with each saved binding that has its ID, and `skipped` with the namespace and key of any other entry.
+--- @param saved table<string, table<string, Binding>> The saved bindings.
+--- @param fn fun(binding: Binding)
+--- @param skipped? fun(namespace: any, key: any)
+local function eachBinding(saved, fn, skipped)
+  for namespace, keys in pairs(saved) do
+    for key, binding in pairs(type(keys) == "table" and keys or {}) do
+      if Select(binding, "bindingId") ~= nil then
+        fn(binding)
+      elseif skipped then
+        skipped(namespace, key)
+      end
+    end
+  end
+end
+
 --- Creates a new Bindings instance.
 --- @return Bindings bindings A new Bindings instance.
 function Bindings:new()
@@ -123,43 +139,44 @@ end
 --- If the binding does not exist, it creates a new one with a unique ID.
 --- @param namespace string The namespace of the binding.
 --- @param key string The key of the binding.
---- @param type string The type of the binding (e.g., "CONTROL" or "PROXY").
+--- @param bindingType string The type of the binding (e.g., "CONTROL" or "PROXY").
 --- @param provider boolean Whether the binding is a provider.
 --- @param displayName string The display name of the binding.
 --- @param class string The class of the binding.
 --- @return Binding|nil binding The binding object or nil if the binding could not be created.
-function Bindings:getOrAddDynamicBinding(namespace, key, type, provider, displayName, class)
+function Bindings:getOrAddDynamicBinding(namespace, key, bindingType, provider, displayName, class)
   log:trace(
     "Binding:getOrAddDynamicBinding(%s, %s, %s, %s, %s, %s)",
     namespace,
     key,
-    type,
+    bindingType,
     provider,
     displayName,
     class
   )
   local bindings = self:getBindings()
   --- @type Binding|nil
   local binding = Select(bindings, namespace, key)
-  if binding == nil then
-    local bindingId = self:_getNextBindingId(type)
+  if Select(binding, "bindingId") == nil or binding.type == nil or binding.class == nil then
+    -- A saved record missing a field the restore needs is replaced, keeping its ID if it has one.
+    local bindingId = Select(binding, "bindingId") or self:_getNextBindingId(bindingType)
     if bindingId == nil then
       return nil
     end
     binding = {
       key = key,
       bindingId = bindingId,
-      type = type,
+      type = bindingType,
       provider = provider,
       displayName = displayName,
       class = class,
     }
     --- @cast binding Binding
 
-    bindings[namespace] = bindings[namespace] or {}
+    bindings[namespace] = type(bindings[namespace]) == "table" and bindings[namespace] or {}
     bindings[namespace][key] = binding
     self:_saveBindings(bindings)
-    C4:AddDynamicBinding(bindingId, type, provider, displayName, class, false, false)
+    C4:AddDynamicBinding(bindingId, bindingType, provider, displayName, class, false, false)
   elseif binding.displayName ~= displayName or binding.provider ~= provider or binding.class ~= class then
     -- Control4 has no in-place rename, so re-add under the same id, with the record's own
     -- type (its id came from that type's range and restoreBindings re-adds from it). Wiring
@@ -292,25 +309,28 @@ end
 function Bindings:restoreBindings()
   log:trace("Binding:restoreBindings()")
   local deviceBindings = GetDeviceBindings(tointeger(C4:GetDeviceID()))
-  for _, keys in pairs(self:getBindings()) do
-    for _, binding in pairs(keys) do
-      deviceBindings[binding.bindingId] = nil
-      -- AddDynamicBinding raises on a nil name, which an older driver build could save; the
-      -- record keeps its id, and the driver's next getOrAddDynamicBinding for it adds it named.
-      if binding.displayName ~= nil then
-        log:debug("Restoring %s binding %s", binding.class, binding.displayName)
-        C4:AddDynamicBinding(
-          binding.bindingId,
-          binding.type,
-          binding.provider,
-          binding.displayName,
-          binding.class,
-          false,
-          false
-        )
-      end
+  eachBinding(self:getBindings(), function(binding)
+    deviceBindings[binding.bindingId] = nil
+    -- AddDynamicBinding raises on a nil name, which an older driver build could save, and a record without its
+    -- type or class can't be added as it was. Either keeps its id, and the driver's next getOrAddDynamicBinding
+    -- for it adds it whole.
+    if binding.type == nil or binding.class == nil then
+      log:warn("Skipping saved binding %s: it has no type or class", binding.bindingId)
+    elseif binding.displayName ~= nil then
+      log:debug("Restoring %s binding %s", binding.class, binding.displayName)
+      C4:AddDynamicBinding(
+        binding.bindingId,
+        binding.type,
+        binding.provider,
+        binding.displayName,
+        binding.class,
+        false,
+        false
+      )
     end
-  end
+  end, function(namespace, key)
+    log:warn("Skipping saved binding %s/%s: it has no binding ID", namespace, key)
+  end)
   -- Delete unknown bindings inside our managed ranges, but never the driver's own
   -- static driver.xml connections, which share the id space (see staticConnectionIds).
   local static = staticConnectionIds()
@@ -325,26 +345,24 @@ end
 --- Retrieves the next available binding ID for a given type. Ensures that the
 --- ID is unique and within the allowed range.
 --- @private
---- @param type string The type of the binding (e.g., "CONTROL" or "PROXY").
+--- @param bindingType string The type of the binding (e.g., "CONTROL" or "PROXY").
 --- @return integer|nil bindingId The next available binding ID or nil if the maximum is exceeded.
-function Bindings:_getNextBindingId(type)
-  log:trace("Binding:_getNextBindingId(%s)", type)
+function Bindings:_getNextBindingId(bindingType)
+  log:trace("Binding:_getNextBindingId(%s)", bindingType)
   --- @type table<integer, boolean>
   local currentBindings = {}
-  for _, keys in pairs(self:getBindings()) do
-    for _, binding in pairs(keys) do
-      currentBindings[binding.bindingId] = true
-    end
-  end
+  eachBinding(self:getBindings(), function(binding)
+    currentBindings[binding.bindingId] = true
+  end)
   local nextId, maxId = CONTROL_BINDING_START, CONTROL_BINDING_END
-  if type == "PROXY" then
+  if bindingType == "PROXY" then
     nextId, maxId = PROXY_BINDING_START, PROXY_BINDING_END
   end
   while currentBindings[nextId] ~= nil and nextId <= maxId do
     nextId = nextId + 1
   end
   if nextId > maxId then
-    log:error("maximum %s bindings exceeded", type)
+    log:error("maximum %s bindings exceeded", bindingType)
     return nil
   end
   return nextId
@@ -371,14 +389,12 @@ end
 --- This does not affect static bindings defined in driver.xml.
 function Bindings:reset()
   log:trace("Bindings:reset()")
-  for _, nsBindings in pairs(self:getBindings()) do
-    for _, binding in pairs(nsBindings) do
-      log:debug("Removing binding '%s' (id=%s)", binding.displayName, binding.bindingId)
-      C4:RemoveDynamicBinding(binding.bindingId)
-      RFP[binding.bindingId] = nil
-      OBC[binding.bindingId] = nil
-    end
-  end
+  eachBinding(self:getBindings(), function(binding)
+    log:debug("Removing binding '%s' (id=%s)", binding.displayName, binding.bindingId)
+    C4:RemoveDynamicBinding(binding.bindingId)
+    RFP[binding.bindingId] = nil
+    OBC[binding.bindingId] = nil
+  end)
   self:_saveBindings(nil)
 end
 

template/src/lib/conditionals.lua

@@ -28,6 +28,19 @@ local CONDITIONAL_ID_START = 10
 --- @field conditionalId number
 --- @field name string
 
+--- Calls `fn` with each saved conditional that has its ID, skipping any other saved entry.
+--- @param saved table<string, table<string, Conditional>> The saved conditionals.
+--- @param fn fun(conditional: Conditional)
+local function eachConditional(saved, fn)
+  for _, keys in pairs(saved) do
+    for _, conditional in pairs(type(keys) == "table" and keys or {}) do
+      if Select(conditional, "conditionalId") ~= nil then
+        fn(conditional)
+      end
+    end
+  end
+end
+
 --- Creates a new instance of the `Conditionals` class.
 --- @return Conditionals conditionals A new instance of the `Conditionals` class.
 function Conditionals:new()
@@ -54,7 +67,7 @@ function Conditionals:upsertConditional(namespace, key, conditional, testFunctio
   conditional.conditionalId = conditionalId
   conditional.name = "CONDITIONAL_" .. conditionalId
 
-  conditionals[namespace] = conditionals[namespace] or {}
+  conditionals[namespace] = type(conditionals[namespace]) == "table" and conditionals[namespace] or {}
   conditionals[namespace][key] = conditional
 
   TC[conditional.name] = testFunction
@@ -96,13 +109,9 @@ end
 function Conditionals:_getNextConditionalId()
   log:trace("Conditionals:_getNextConditionalId()")
   local currentConditionals = {}
-  for _, keys in pairs(self:getConditionals()) do
-    for _, conditional in pairs(keys) do
-      if Select(conditional, "conditionalId") ~= nil then
-        currentConditionals[conditional.conditionalId] = true
-      end
-    end
-  end
+  eachConditional(self:getConditionals(), function(conditional)
+    currentConditionals[conditional.conditionalId] = true
+  end)
   local nextId = CONDITIONAL_ID_START
   while currentConditionals[nextId] ~= nil do
     nextId = nextId + 1
@@ -130,12 +139,10 @@ end
 --- Resets all conditionals, removing them from the system and clearing persisted storage.
 function Conditionals:reset()
   log:trace("Conditionals:reset()")
-  for _, nsConditionals in pairs(self:getConditionals()) do
-    for _, conditional in pairs(nsConditionals) do
-      log:debug("Removing conditional '%s' (id=%s)", conditional.name, conditional.conditionalId)
-      TC[conditional.name] = nil
-    end
-  end
+  eachConditional(self:getConditionals(), function(conditional)
+    log:debug("Removing conditional '%s' (id=%s)", conditional.name, conditional.conditionalId)
+    TC[conditional.name] = nil
+  end)
   self:_saveConditionals(nil)
 end
 
@@ -146,11 +153,9 @@ local conditionals = Conditionals:new()
 function GetConditionals()
   log:trace("GetConditionals()")
   local progConditionals = {}
-  for _, keys in pairs(conditionals:getConditionals()) do
-    for _, conditional in pairs(keys) do
-      progConditionals[tostring(conditional.conditionalId)] = conditional
-    end
-  end
+  eachConditional(conditionals:getConditionals(), function(conditional)
+    progConditionals[tostring(conditional.conditionalId)] = conditional
+  end)
   return progConditionals
 end
 

template/src/lib/diagnostics.lua

@@ -4,6 +4,9 @@
 ---
 --- While enabled it owns ON_HANDLER_ERROR, ON_TIMER_ERROR and ON_UNHANDLED_REJECTION, the hooks the vendored
 --- dispatchers, timers and deferred call on an error. A driver adds its own points to each report with onReport.
+---
+--- An error's message and traceback are stored with each newline, tab and carriage return written as a literal \n,
+--- \t and \r, which keeps the point on one line, so whatever reads them back unescapes them.
 
 local log = require("lib.logging")
 local http = require("lib.http")
@@ -209,7 +212,26 @@ function Diagnostics:onReport(callback)
   self._onReport = callback
 end
 
---- Captures an error and writes it directly to the logs database.
+--- Splits an error into its message and traceback. An HTTP rejection from lib.http is a table whose `error` is
+--- already redacted.
+--- @param err any What the dispatcher, timer or rejection passed.
+--- @return string message
+--- @return string traceback
+local function splitError(err)
+  if type(err) == "table" then
+    err = type(err.error) == "string" and err.error or type(err.message) == "string" and err.message or "a table"
+  end
+  local text = tostring(err)
+  local marker = "\nstack traceback:\n"
+  local at = text:find(marker, 1, true)
+  if at then
+    return text:sub(1, at - 1), text:sub(at + #marker)
+  end
+  return text, ""
+end
+
+--- Captures an error and writes it directly to the logs database. A message given with no traceback is split, so
+--- one that is a whole debug.traceback() is stored as its message and its traceback.
 --- Rate-limited to ERROR_RATE_LIMIT writes per ERROR_RATE_WINDOW seconds.
 --- @param message string
 --- @param traceback string|nil
@@ -219,8 +241,11 @@ function Diagnostics:captureError(message, traceback, source)
     return
   end
 
-  message = tostring(message or "unknown error")
-  traceback = tostring(traceback or "")
+  if traceback == nil or traceback == "" then
+    message, traceback = splitError(message or "unknown error")
+  end
+  message = tostring(message)
+  traceback = tostring(traceback)
   source = tostring(source or "unknown")
 
   local now = os.time()
@@ -247,24 +272,6 @@ function Diagnostics:captureError(message, traceback, source)
   self:_writeErrorLog(message, traceback, source, now)
 end
 
---- Splits an error into its message and traceback. An HTTP rejection from lib.http is a table whose `error` is
---- already redacted.
---- @param err any What the dispatcher, timer or rejection passed.
---- @return string message
---- @return string traceback
-local function splitError(err)
-  if type(err) == "table" then
-    err = type(err.error) == "string" and err.error or type(err.message) == "string" and err.message or "a table"
-  end
-  local text = tostring(err)
-  local marker = "\nstack traceback:\n"
-  local at = text:find(marker, 1, true)
-  if at then
-    return text:sub(1, at - 1), text:sub(at + #marker)
-  end
-  return text, ""
-end
-
 --- Packs every value, nils included.
 --- @return table
 local function pack(...)