fix(eventloop): isolate handler failures

env.sample

@@ -1,5 +1,5 @@
 # Host-side watchdog for the normal `just test` CraftOS-PC process.
-TRAPOS_TEST_TIMEOUT_SECONDS=3
+TRAPOS_TEST_TIMEOUT_SECONDS=4
 
 # Dedicated `just test-timeout` fixture timings.
 TRAPOS_TEST_TIMEOUT_WATCHDOG_SECONDS=1

manifest.json

@@ -1,6 +1,6 @@
 {
   "name": "TrapOS",
-  "version": "0.12.3",
+  "version": "0.12.4",
   "branch": "master",
   "packages": [
     "trapos"

packages/index.json

@@ -1,13 +1,13 @@
 {
   "packages": {
-    "trapos-core": "0.9.3",
+    "trapos-core": "0.9.4",
     "trapos-test": "0.2.3",
-    "trapos-boot": "0.4.2",
+    "trapos-boot": "0.4.3",
     "trapos-net": "0.3.2",
     "trapos-ui": "0.2.4",
     "trapos-ai": "0.8.0",
-    "trapos-cloud": "0.4.3",
+    "trapos-cloud": "0.4.4",
     "trapos-sandbox-legacy": "0.3.2",
-    "trapos": "0.12.3"
+    "trapos": "0.12.4"
   }
 }

packages/trapos-boot/ccpm.json

@@ -1,6 +1,6 @@
 {
   "name": "trapos-boot",
-  "version": "0.4.2",
+  "version": "0.4.3",
   "description": "TrapOS boot: startup MOTD and autostart server launcher",
   "dependencies": ["trapos-core"],
   "files": ["programs/motd.lua", "startup/boot.lua"],

packages/trapos-boot/startup/boot.lua

@@ -45,7 +45,17 @@ if periphemu then
 	periphemu.create("top", "modem")
 end
 
-_G.bootEventLoop = createEventLoop()
+local function reportBootEventLoopError(eventName, err)
+	local message = "boot eventloop handler error (" .. tostring(eventName) .. "): " .. tostring(err)
+	local printErrorFn = _G.printError
+	if type(printErrorFn) == "function" then
+		printErrorFn(message)
+	else
+		print(message)
+	end
+end
+
+_G.bootEventLoop = createEventLoop({ onError = reportBootEventLoopError })
 
 local shellExited = false
 local action = "shutdown"

packages/trapos-cloud/apis/libcloud.lua

@@ -228,6 +228,7 @@ local function createCloud()
     -- Inbound frames of an advertised type are re-emitted as trapos_cloud_request
     -- events; unadvertised (non-ping) types get an immediate unknown_type reply.
     local servedTypes = {};
+    local scheduleReconnect = nil;
 
     local function sendFrame(frame)
       if not activeWs then
@@ -253,10 +254,16 @@ local function createCloud()
       if not reconnecting then
         log('info', 'connecting', { url = url });
       end
-      httpLike.websocketAsync(url);
+      local ok, err = pcall(function() httpLike.websocketAsync(url); end);
+      if not ok then
+        if not reconnecting then
+          log('warn', 'websocket open failed', { url = url, error = tostring(err) });
+        end
+        scheduleReconnect();
+      end
     end
 
-    local function scheduleReconnect()
+    function scheduleReconnect()
       activeWs = nil;
       if state == 'unauthorized' or state == 'displaced' then
         return;

packages/trapos-cloud/ccpm.json

@@ -1,6 +1,6 @@
 {
   "name": "trapos-cloud",
-  "version": "0.4.3",
+  "version": "0.4.4",
   "description": "TrapOS cloud gateway client: WS daemon (servers/cloud) + cloud command + MCP exec/write server",
   "dependencies": ["trapos-core"],
   "files": [

packages/trapos-cloud/tests/cloud.lua

@@ -209,6 +209,27 @@ testlib.test('startSession connects to the /gateway path', function()
   testlib.assertEquals(ctx.url, 'ws://host:4444/gateway');
 end);
 
+testlib.test('startSession retries when websocketAsync throws', function()
+  local http = { connects = {} };
+  function http.websocketAsync(url)
+    http.connects[#http.connects + 1] = url;
+    error('open boom', 0);
+  end
+  function http.websocket()
+    return nil;
+  end
+
+  local ctx = startSession({ http = http });
+
+  testlib.assertEquals(ctx.session.isReady(), false);
+  testlib.assertEquals(#http.connects, 1);
+  testlib.assertEquals(#ctx.el.timers, 1);
+
+  ctx.el.fireTimer(#ctx.el.timers);
+  testlib.assertEquals(#http.connects, 2);
+  testlib.assertEquals(#ctx.el.timers, 2);
+end);
+
 testlib.test('startSession sends hello on open and goes ready on ack', function()
   local ctx = startSession({ secret = 'hunter2' });
   local ws = fakes.fakeWs();

packages/trapos-core/apis/eventloop.lua

@@ -16,19 +16,26 @@
 -- end)
 --
 -- events.runLoop();
-local next_eventloop_id = 1;
+local EVENTLOOP_COUNTER_KEY = '__trapos_next_eventloop_id';
+
+local function nextEventLoopId()
+  local id = tonumber(_G[EVENTLOOP_COUNTER_KEY]) or 1;
+  _G[EVENTLOOP_COUNTER_KEY] = id + 1;
+  return id;
+end
 
 local function noop()
 end
 
 local STOP = '@libeventloop/STOP_HANDLER_SUBSCRIPTION';
 
-local function createEventLoop()
+local function createEventLoop(opts)
+  opts = opts or {};
   local api = {}
 
   local runningLoop = false;
-  local eventloop_id = next_eventloop_id;
-  next_eventloop_id = next_eventloop_id + 1;
+  local eventloop_id = nextEventLoopId();
+  local stopRequested = false;
 
   local shouldCheckHandlers = true;
 
@@ -58,6 +65,25 @@ local function createEventLoop()
   local nextOnStartHandlerId = 1;
   local onStartHandlers = {};
 
+  local function reportError(eventName, err)
+    if type(opts.onError) == 'function' then
+      pcall(opts.onError, eventName, err);
+    end
+  end
+
+  local function dispatchHandler(eventName, handler, ...)
+    if type(opts.onError) ~= 'function' then
+      return true, handler(...);
+    end
+
+    local ok, result = pcall(handler, ...);
+    if not ok then
+      reportError(eventName, result);
+      return false, nil;
+    end
+    return true, result;
+  end
+
   local function resetOnStartHandlers()
     nextOnStartHandlerId = 1;
     onStartHandlers = {};
@@ -139,6 +165,7 @@ local function createEventLoop()
 
   local function resetAll()
     runningLoop = false;
+    stopRequested = false;
     handlersCounter = 0;
     allHandlers = {};
     runningHandlers = nil;
@@ -183,7 +210,11 @@ local function createEventLoop()
   -- stopLoop
   function api.stopLoop()
     if api.isRunningLoop() then
-      os.queueEvent(END_OF_LOOP)
+      if runningHandlers or runningTimeoutHandler then
+        stopRequested = true;
+      else
+        os.queueEvent(END_OF_LOOP)
+      end
     else
       error("libeventloop error: loop is already stopped")
     end
@@ -274,7 +305,7 @@ local function createEventLoop()
         for k, h in pairs(timeoutHandlers) do
           if k == timerId then
             removeTimeoutHandler(k)
-            h();
+            dispatchHandler('timer', h);
           end
         end
         runningTimeoutHandler = false
@@ -291,8 +322,8 @@ local function createEventLoop()
         if handlers then
           runningHandlers = eventName
           for _, handler in pairs(handlers) do
-            local result_handler = handler(table.unpack(packed))
-            if result_handler == api.STOP then
+            local ok, result_handler = dispatchHandler(eventName, handler, table.unpack(packed))
+            if ok and result_handler == api.STOP then
               api.unregister(eventName, handler)
             end
           end
@@ -300,11 +331,12 @@ local function createEventLoop()
           flushUnregisterQueue()
         end
 
-        if eventName == END_OF_LOOP or (shouldCheckHandlers and eventName == 'terminate') then
-          execOnStopHandlers()
-          resetAll()
-          break
-        end
+      end
+
+      if stopRequested or eventName == END_OF_LOOP or (shouldCheckHandlers and eventName == 'terminate') then
+        execOnStopHandlers()
+        resetAll()
+        break
       end
     end
   end

packages/trapos-core/ccpm.json

@@ -1,6 +1,6 @@
 {
   "name": "trapos-core",
-  "version": "0.9.3",
+  "version": "0.9.4",
   "description": "TrapOS base: package manager, event loop, upgrade and event tools",
   "dependencies": [],
   "files": [

packages/trapos-core/tests/eventloop.lua

@@ -138,6 +138,127 @@ testlib.test('onStart and onStop run around loop', function()
   testlib.assertEquals(order, 'start>timeout>stop');
 end);
 
+testlib.test('same-loop stop does not leak to a sibling loop with the same id', function()
+  local createEventLoopA = assert(loadfile('/apis/eventloop.lua'))();
+  local createEventLoopB = assert(loadfile('/apis/eventloop.lua'))();
+  local bootEvents = createEventLoopA();
+  local verifierEvents = createEventLoopB();
+  local bootProbeHandled = false;
+
+  bootEvents.register('eventloop_test_same_loop_probe', function()
+    bootProbeHandled = true;
+    bootEvents.stopLoop();
+  end);
+
+  verifierEvents.register('eventloop_test_same_loop_stop', function()
+    verifierEvents.stopLoop();
+    os.queueEvent('eventloop_test_same_loop_probe');
+  end);
+
+  os.queueEvent('eventloop_test_same_loop_stop');
+  parallel.waitForAll(function() bootEvents.runLoop(true); end, function() verifierEvents.runLoop(); end);
+
+  testlib.assertTrue(bootProbeHandled);
+end);
+
+testlib.test('external stop events use ids unique across module instances', function()
+  local createEventLoopA = assert(loadfile('/apis/eventloop.lua'))();
+  local createEventLoopB = assert(loadfile('/apis/eventloop.lua'))();
+  local bootEvents = createEventLoopA();
+  local verifierEvents = createEventLoopB();
+  local bootProbeHandled = false;
+
+  bootEvents.register('eventloop_test_external_stop', function()
+    verifierEvents.stopLoop();
+    os.queueEvent('eventloop_test_external_probe');
+  end);
+
+  bootEvents.register('eventloop_test_external_probe', function()
+    bootProbeHandled = true;
+    bootEvents.stopLoop();
+  end);
+
+  os.queueEvent('eventloop_test_external_stop');
+  parallel.waitForAll(function() bootEvents.runLoop(true); end, function() verifierEvents.runLoop(true); end);
+
+  testlib.assertTrue(bootProbeHandled);
+end);
+
+testlib.test('protected dispatch reports errors and keeps the loop alive', function()
+  local errors = {};
+  local events = createEventLoop({
+    onError = function(eventName, err)
+      errors[#errors + 1] = { eventName = eventName, err = err };
+    end,
+  });
+  local siblingCalled = false;
+  local laterCalled = false;
+
+  events.register('eventloop_test_protected', function()
+    error('handler boom', 0);
+  end);
+
+  events.register('eventloop_test_protected', function()
+    siblingCalled = true;
+    os.queueEvent('eventloop_test_protected_later');
+  end);
+
+  events.register('eventloop_test_protected_later', function()
+    laterCalled = true;
+    events.stopLoop();
+  end);
+
+  os.queueEvent('eventloop_test_protected');
+  events.runLoop();
+
+  testlib.assertEquals(#errors, 1);
+  testlib.assertEquals(errors[1].eventName, 'eventloop_test_protected');
+  testlib.assertTrue(string.find(tostring(errors[1].err), 'handler boom', 1, true) ~= nil);
+  testlib.assertTrue(siblingCalled);
+  testlib.assertTrue(laterCalled);
+end);
+
+testlib.test('protected timeout dispatch reports errors and keeps the loop alive', function()
+  local errors = {};
+  local events = createEventLoop({
+    onError = function(eventName, err)
+      errors[#errors + 1] = { eventName = eventName, err = err };
+    end,
+  });
+  local laterCalled = false;
+
+  events.setTimeout(function()
+    error('timeout boom', 0);
+  end, 0);
+
+  events.setTimeout(function()
+    laterCalled = true;
+    events.stopLoop();
+  end, 0);
+
+  events.runLoop();
+
+  testlib.assertEquals(#errors, 1);
+  testlib.assertEquals(errors[1].eventName, 'timer');
+  testlib.assertTrue(string.find(tostring(errors[1].err), 'timeout boom', 1, true) ~= nil);
+  testlib.assertTrue(laterCalled);
+end);
+
+testlib.test('timeout handler can stop a loop with persistent handlers', function()
+  local events = createEventLoop();
+
+  events.register('eventloop_test_never_fired', function()
+  end);
+
+  events.setTimeout(function()
+    events.stopLoop();
+  end, 0);
+
+  events.runLoop();
+
+  testlib.assertTrue(not events.isRunningLoop());
+end);
+
 testlib.test('empty loop returns and runs onStop', function()
   local events = createEventLoop();
   local stopped = false;

packages/trapos/ccpm.json

@@ -1,6 +1,6 @@
 {
   "name": "trapos",
-  "version": "0.12.3",
+  "version": "0.12.4",
   "description": "TrapOS full install meta-package",
   "dependencies": [
     "trapos-boot",