fix(reader): run KOReader sync jobs and HTTP requests asynchronously

Wrap KOReader plugin sync jobs and API requests to execute asynchronously using the Trapper coroutine wrapper and subprocess runner. This keeps the main UI thread responsive and prevents Kindle and other devices from becoming unresponsive for 10-30 seconds during automatic sync operations.

Closes #608

koreader-plugin/bookorbit.koplugin/bookorbit_api.lua

@@ -2,7 +2,8 @@
 HTTP client for the BookOrbit server.
 
 Speaks the kosync-compatible progress endpoints plus the BookOrbit plugin
-endpoints. All requests are blocking; callers keep them short and chunked.
+endpoints. Sync clients run blocking requests in a subprocess when called from
+a Trapper coroutine, keeping KOReader's UI loop responsive.
 ]]
 
 local http = require("socket.http")
@@ -70,6 +71,7 @@ function BookOrbitApi.new(opts)
         device_id = opts.device_id,
         device_model = opts.device_model,
         plugin_version = opts.plugin_version,
+        background_requests = opts.background_requests == true,
     }, BookOrbitApi)
 end
 
@@ -79,7 +81,7 @@ end
 
 -- Returns decoded_body on success, or nil, err_code, decoded_error_body.
 -- err_code is a number for HTTP errors and a string for transport errors.
-function BookOrbitApi:request(method, path, body)
+function BookOrbitApi:requestBlocking(method, path, body)
     local sink = {}
     local request = {
         url = self.server_url .. path,
@@ -127,6 +129,31 @@ function BookOrbitApi:request(method, path, body)
     return decoded or {}
 end
 
+function BookOrbitApi:request(method, path, body)
+    if not self.background_requests then
+        return self:requestBlocking(method, path, body)
+    end
+
+    local loaded, Trapper = pcall(require, "ui/trapper")
+    if not loaded or not Trapper:isWrapped() then
+        return self:requestBlocking(method, path, body)
+    end
+
+    -- An unmounted widget lets Trapper poll without intercepting reader input.
+    local trap_widget = {}
+    local completed, result = Trapper:dismissableRunInSubprocess(function()
+        local response, err, errbody = self:requestBlocking(method, path, body)
+        return { response = response, err = err, errbody = errbody }
+    end, trap_widget)
+    trap_widget.dismiss_callback = nil
+    if not completed then
+        return nil, "background_request_interrupted"
+    end
+
+    result = result or {}
+    return result.response, result.err, result.errbody
+end
+
 function BookOrbitApi:query(path, params)
     if not params then return path end
 

koreader-plugin/bookorbit.koplugin/bookorbit_book_sync.lua

@@ -12,6 +12,7 @@ under mutual exclusion, so sweep and snapshot never double-upload.
 
 local InfoMessage = require("ui/widget/infomessage")
 local Notification = require("ui/widget/notification")
+local Trapper = require("ui/trapper")
 local UIManager = require("ui/uimanager")
 local logger = require("logger")
 local T = require("ffi/util").template
@@ -214,12 +215,14 @@ local function step(ctx, fn)
         return fn(ctx)
     end
     UIManager:scheduleIn(STEP_DELAY, function()
-        local ok, err = pcall(fn, ctx)
-        if not ok then
-            logger.err("BookOrbit: book sync step failed:", err)
-            ctx.had_errors = true
-            finish(ctx)
-        end
+        Trapper:wrap(function()
+            local ok, err = pcall(fn, ctx)
+            if not ok then
+                logger.err("BookOrbit: book sync step failed:", err)
+                ctx.had_errors = true
+                finish(ctx)
+            end
+        end)
     end)
 end
 

koreader-plugin/bookorbit.koplugin/bookorbit_progress_sync.lua

@@ -203,7 +203,11 @@ function ProgressSync:updateProgress(ensure_networking, interactive, on_suspend)
         return
     end
 
-    if ensure_networking and NetworkMgr:willRerunWhenOnline(function() self:updateProgress(ensure_networking, interactive, on_suspend) end) then
+    if ensure_networking and NetworkMgr:willRerunWhenOnline(function()
+            self:runInSyncCoroutine(function()
+                self:updateProgress(ensure_networking, interactive, on_suspend)
+            end)
+        end) then
         return
     end
 
@@ -272,7 +276,11 @@ function ProgressSync:getProgress(ensure_networking, interactive)
         return
     end
 
-    if ensure_networking and NetworkMgr:willRerunWhenOnline(function() self:getProgress(ensure_networking, interactive) end) then
+    if ensure_networking and NetworkMgr:willRerunWhenOnline(function()
+            self:runInSyncCoroutine(function()
+                self:getProgress(ensure_networking, interactive)
+            end)
+        end) then
         return
     end
 

koreader-plugin/bookorbit.koplugin/bookorbit_sweep.lua

@@ -15,6 +15,7 @@ local InfoMessage = require("ui/widget/infomessage")
 local Notification = require("ui/widget/notification")
 local ReadHistory = require("readhistory")
 local UIManager = require("ui/uimanager")
+local Trapper = require("ui/trapper")
 local logger = require("logger")
 local lfs = require("libs/libkoreader-lfs")
 local util = require("util")
@@ -173,12 +174,14 @@ end
 
 local function step(ctx, fn)
     UIManager:scheduleIn(STEP_DELAY, function()
-        local ok, err = pcall(fn, ctx)
-        if not ok then
-            logger.err("BookOrbit: sweep step failed:", err)
-            ctx.had_errors = true
-            finish(ctx)
-        end
+        Trapper:wrap(function()
+            local ok, err = pcall(fn, ctx)
+            if not ok then
+                logger.err("BookOrbit: sweep step failed:", err)
+                ctx.had_errors = true
+                finish(ctx)
+            end
+        end)
     end)
 end
 

koreader-plugin/bookorbit.koplugin/bookorbit_sync_job_runner.lua

@@ -0,0 +1,32 @@
+local Trapper = require("ui/trapper")
+local logger = require("logger")
+
+local SyncJobRunner = {}
+
+function SyncJobRunner.prepare(job)
+    local original_run = job.run
+    local original_async = job.async == true
+
+    job.async = true
+    job.run = function(done, submitted_job)
+        Trapper:wrap(function()
+            local ok, err = xpcall(function()
+                if original_async then
+                    original_run(done, submitted_job)
+                else
+                    original_run(done, submitted_job)
+                    done()
+                end
+            end, debug.traceback)
+            if not ok then
+                logger.err("BookOrbit: sync job failed:", err)
+                done()
+            end
+        end)
+        return true
+    end
+
+    return job
+end
+
+return SyncJobRunner

koreader-plugin/bookorbit.koplugin/bookorbit_updater.lua

@@ -154,7 +154,9 @@ function UpdateCheck:checkForUpdate()
         return
     end
     NetworkMgr:runWhenConnected(function()
-        self:doCheckForUpdate()
+        self:runInSyncCoroutine(function()
+            self:doCheckForUpdate()
+        end)
     end)
 end
 

koreader-plugin/bookorbit.koplugin/main.lua

@@ -20,6 +20,7 @@ local InfoMessage = require("ui/widget/infomessage")
 local NetworkMgr = require("ui/network/manager")
 local Notification = require("ui/widget/notification")
 local PluginShare = require("pluginshare")
+local Trapper = require("ui/trapper")
 local UIManager = require("ui/uimanager")
 local WidgetContainer = require("ui/widget/container/widgetcontainer")
 local logger = require("logger")
@@ -35,13 +36,14 @@ local BookOrbitHighlightSummary = require("bookorbit_highlight_summary")
 local BookOrbitOpenAnnotationScheduler = require("bookorbit_open_annotation_scheduler")
 local BookOrbitState = require("bookorbit_state")
 local BookOrbitSyncCoordinator = require("bookorbit_sync_coordinator")
+local BookOrbitSyncJobRunner = require("bookorbit_sync_job_runner")
 local BookOrbitMainMenu = require("bookorbit_main_menu")
 local BookOrbitMenuPin = require("bookorbit_menu_pin")
 local BookOrbitProgressSync = require("bookorbit_progress_sync")
 local BookOrbitSweep = require("bookorbit_sweep")
 local BookOrbitUpdater = require("bookorbit_updater")
 
-local PLUGIN_VERSION = "1.2.0"
+local PLUGIN_VERSION = "1.2.1"
 
 local SYNC_STRATEGY = {
     PROMPT = 1,
@@ -55,6 +57,7 @@ local LAST_ERROR_LABELS = {
     invalid_json = _("invalid server response"),
     unsupported_server = _("server update required"),
     body_too_large = _("request too large"),
+    background_request_interrupted = _("background request interrupted"),
     partial_failure = _("partial sync failure"),
 }
 
@@ -228,7 +231,7 @@ function BookOrbit:applyProvision()
     os.remove(provision_path)
 end
 
-function BookOrbit:apiOpts()
+function BookOrbit:apiOpts(background_requests)
     return {
         server_url = self.settings.server_url,
         username = self.settings.username,
@@ -236,11 +239,19 @@ function BookOrbit:apiOpts()
         device_id = self.device_id,
         device_model = Device.model,
         plugin_version = PLUGIN_VERSION,
+        background_requests = background_requests == true,
     }
 end
 
 function BookOrbit:newClient()
-    return BookOrbitApi.new(self:apiOpts())
+    return BookOrbitApi.new(self:apiOpts(true))
+end
+
+function BookOrbit:runInSyncCoroutine(fn)
+    if Trapper:isWrapped() then
+        return fn()
+    end
+    return Trapper:wrap(fn)
 end
 
 function BookOrbit:isLoggedIn()
@@ -384,7 +395,8 @@ function BookOrbit:retryOpenBookMatch()
 end
 
 function BookOrbit:submitSyncJob(job)
-    local result = self:getSyncCoordinator():submit(job)
+    local prepared_job = BookOrbitSyncJobRunner.prepare(job)
+    local result = self:getSyncCoordinator():submit(prepared_job)
     if (result == "queued" or result == "kept") and job.interactive then
         UIManager:show(InfoMessage:new{ text = _("BookOrbit sync queued."), timeout = 2 })
     end
@@ -460,7 +472,9 @@ function BookOrbit:requestAnnotationExchange(source)
                 self:exchangeAnnotationsForOpenBook(source or "auto")
                 done()
             end
-            if NetworkMgr:willRerunWhenConnected(execute) then
+            if NetworkMgr:willRerunWhenConnected(function()
+                    self:runInSyncCoroutine(execute)
+                end) then
                 return
             end
             execute()
@@ -495,7 +509,9 @@ function BookOrbit:scheduleOpenHighlightRetry(reason, retry_count)
                     self:exchangeAnnotationsForOpenBook(reason or "annotation_retry", retry_count + 1)
                     done()
                 end
-                if NetworkMgr:willRerunWhenConnected(execute) then
+                if NetworkMgr:willRerunWhenConnected(function()
+                        self:runInSyncCoroutine(execute)
+                    end) then
                     return
                 end
                 execute()
@@ -537,7 +553,7 @@ function BookOrbit:requestSweep(interactive, source)
             async = true,
             run = function(done)
                 local started = BookOrbitSweep.run{
-                    api = self:apiOpts(),
+                    api = self:apiOpts(true),
                     interactive = interactive == true,
                     annotation_sync = self.settings.annotation_sync,
                     plugin = self,
@@ -563,7 +579,7 @@ end
 
 function BookOrbit:requestManualBookSync(snap)
     if not snap then return end
-    local api_opts = self:apiOpts()
+    local api_opts = self:apiOpts(true)
 
     self:submitSyncJob{
         family = "book_snapshot",
@@ -606,7 +622,9 @@ function BookOrbit:requestManualBookSync(snap)
                     done()
                 end
             end
-            if NetworkMgr:willRerunWhenOnline(run) then
+            if NetworkMgr:willRerunWhenOnline(function()
+                    self:runInSyncCoroutine(run)
+                end) then
                 return
             end
             run()
@@ -617,7 +635,7 @@ end
 function BookOrbit:requestBookSnapshotSync(opts)
     opts = opts or {}
     if not opts.snap then return end
-    local api_opts = self:apiOpts()
+    local api_opts = self:apiOpts(opts.synchronous ~= true)
 
     self:submitSyncJob{
         family = "book_snapshot",
@@ -649,10 +667,14 @@ function BookOrbit:requestBookSnapshotSync(opts)
             end
 
             if opts.go_online then
-                NetworkMgr:goOnlineToRun(run)
+                NetworkMgr:goOnlineToRun(function()
+                    self:runInSyncCoroutine(run)
+                end)
                 return
             end
-            if opts.ensure_networking and NetworkMgr:willRerunWhenOnline(run) then
+            if opts.ensure_networking and NetworkMgr:willRerunWhenOnline(function()
+                    self:runInSyncCoroutine(run)
+                end) then
                 return
             end
             run()
@@ -786,10 +808,12 @@ function BookOrbit:matchOpenBookForAutoSync(on_done)
         if on_done then on_done(false) end
         return
     end
-    if NetworkMgr:willRerunWhenConnected(run) then
+    if NetworkMgr:willRerunWhenConnected(function()
+            self:runInSyncCoroutine(run)
+        end) then
         return
     end
-    run()
+    self:runInSyncCoroutine(run)
 end
 
 -- Two-way annotation pull/push for the open book. Runs once per book open;

koreader-plugin/spec/bookorbit_api_test.lua

@@ -1,5 +1,7 @@
 local mock_http_body = "{}"
 local mock_http_code = 200
+local request_ran_in_subprocess = false
+local in_subprocess = false
 
 package.loaded["logger"] = {
     dbg = function() end,
@@ -64,6 +66,7 @@ package.loaded["ltn12"] = {
 
 package.loaded["socket.http"] = {
     request = function(request)
+        request_ran_in_subprocess = in_subprocess
         if mock_http_body then
             request.sink(mock_http_body)
         end
@@ -135,4 +138,42 @@ assertEqual(body, nil, "HTTP error has no decoded body")
 assertEqual(err, 503, "HTTP error preserves status code")
 assertEqual(errbody, nil, "invalid HTTP error body is ignored")
 
+local wrapped = true
+local subprocess_calls = 0
+package.loaded["ui/trapper"] = {
+    isWrapped = function()
+        return wrapped
+    end,
+    dismissableRunInSubprocess = function(_, task, trap_widget)
+        assertEqual(type(trap_widget), "table", "background request uses a detached trap widget")
+        subprocess_calls = subprocess_calls + 1
+        in_subprocess = true
+        local result = task()
+        in_subprocess = false
+        return true, result
+    end,
+}
+
+local background_client = BookOrbitApi.new{
+    server_url = "https://bookorbit.example.com/api/v1",
+    username = "reader",
+    userkey = "secret",
+    background_requests = true,
+}
+
+mock_http_body = "{\"ok\":true}"
+mock_http_code = 200
+body, err = background_client:auth()
+assertEqual(body.ok, true, "background request returns decoded body")
+assertEqual(err, nil, "background request preserves success result")
+assertEqual(subprocess_calls, 1, "wrapped background request uses subprocess")
+assertEqual(request_ran_in_subprocess, true, "HTTP request runs inside subprocess task")
+
+wrapped = false
+request_ran_in_subprocess = false
+body, err = background_client:auth()
+assertEqual(body.ok, true, "unwrapped request falls back safely")
+assertEqual(subprocess_calls, 1, "unwrapped request does not start subprocess")
+assertEqual(request_ran_in_subprocess, false, "unwrapped fallback runs in current process")
+
 print("bookorbit_api_test.lua: ok")

koreader-plugin/spec/bookorbit_sync_job_runner_test.lua

@@ -0,0 +1,67 @@
+local wrapped_calls = 0
+local logged_errors = 0
+
+package.loaded["ui/trapper"] = {
+    wrap = function(_, fn)
+        wrapped_calls = wrapped_calls + 1
+        fn()
+    end,
+}
+
+package.loaded["logger"] = {
+    err = function()
+        logged_errors = logged_errors + 1
+    end,
+}
+
+package.path = "koreader-plugin/bookorbit.koplugin/?.lua;" .. package.path
+
+local SyncJobRunner = require("bookorbit_sync_job_runner")
+
+local function assertEqual(actual, expected, label)
+    if actual ~= expected then
+        error(string.format("%s: expected %s, got %s", label, tostring(expected), tostring(actual)))
+    end
+end
+
+local completed = 0
+local sync_calls = 0
+local sync_job = SyncJobRunner.prepare{
+    run = function()
+        sync_calls = sync_calls + 1
+    end,
+}
+sync_job.run(function()
+    completed = completed + 1
+end, sync_job)
+assertEqual(sync_job.async, true, "prepared synchronous job uses async coordinator lifecycle")
+assertEqual(sync_calls, 1, "synchronous job runs once")
+assertEqual(completed, 1, "synchronous job completes after wrapped work")
+assertEqual(wrapped_calls, 1, "synchronous job runs in Trapper coroutine")
+
+local async_done
+local async_job = SyncJobRunner.prepare{
+    async = true,
+    run = function(done)
+        async_done = done
+    end,
+}
+async_job.run(function()
+    completed = completed + 1
+end, async_job)
+assertEqual(completed, 1, "asynchronous job controls its own completion")
+async_done()
+assertEqual(completed, 2, "asynchronous completion reaches coordinator callback")
+
+local failed_job = SyncJobRunner.prepare{
+    run = function()
+        error("expected failure")
+    end,
+}
+failed_job.run(function()
+    completed = completed + 1
+end, failed_job)
+assertEqual(completed, 3, "failed job releases coordinator")
+assertEqual(logged_errors, 1, "failed job is logged")
+
+print("bookorbit_sync_job_runner_test.lua: ok")