Log stderr output from shell commands (#229)

* Log stderr output when opening note in app

* Log errors from shell commands

* CHANGELOG

CHANGELOG.md

@@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Re-implemented the native Lua YAML parser (`obsidian.yaml.native`). This should be faster and more robust now. 🤠
 - Re-implemented search/find functionality to utilize concurrency via `obsidian.async` and `plenary.async` for big performance gains. 🏎️
+- Made how run shell commands more robust, and we also log stderr lines now.
 - Submodules imported lazily.
 - Changes to internal module organization.
 

lua/obsidian/async.lua

@@ -1,5 +1,7 @@
+local Job = require "plenary.job"
 local async = require "plenary.async"
 local channel = require("plenary.async.control").channel
+local echo = require "obsidian.echo"
 local uv = vim.loop
 
 local M = {}
@@ -92,15 +94,17 @@ end
 ---@param timeout integer|?
 ---@param pause_fn function(integer)
 Executor._join = function(self, timeout, pause_fn)
+  ---@diagnostic disable-next-line: undefined-field
   local start_time = uv.uptime()
   local pause_for = 100
   if timeout ~= nil then
     pause_for = math.min(timeout / 2, pause_for)
   end
   while self.tasks_running > 0 do
     pause_fn(pause_for)
+    ---@diagnostic disable-next-line: undefined-field
     if timeout ~= nil and uv.uptime() - start_time > timeout then
-      error "Timeout error from AsyncExecutor.join()"
+      return echo.fail "Timeout error from AsyncExecutor.join()"
     end
   end
 end
@@ -177,6 +181,7 @@ end
 ---@diagnostic disable-next-line: unused-local
 ThreadPoolExecutor.submit = function(self, fn, callback, ...)
   self.tasks_running = self.tasks_running + 1
+  ---@diagnostic disable-next-line: undefined-field
   local ctx = uv.new_work(fn, function(...)
     self.tasks_running = self.tasks_running - 1
     if callback ~= nil then
@@ -249,4 +254,79 @@ File.lines = function(self, include_new_line_char)
   return lines
 end
 
+---@param cmd string
+---@param args string[]
+---@param on_stdout function|? (string) -> nil
+---@param on_exit function|? (integer) -> nil
+---@return Job
+local init_job = function(cmd, args, on_stdout, on_exit)
+  local stderr_lines = {}
+
+  return Job:new {
+    command = cmd,
+    args = args,
+    on_stdout = function(err, line)
+      if err ~= nil then
+        return echo.err("Error running command '" .. cmd "' with arguments " .. vim.inspect(args) .. "\n:" .. err)
+      end
+      if on_stdout ~= nil then
+        on_stdout(line)
+      end
+    end,
+    on_stderr = function(err, line)
+      if err then
+        return echo.err("Error running command '" .. cmd "' with arguments " .. vim.inspect(args) .. "\n:" .. err)
+      elseif line ~= nil then
+        stderr_lines[#stderr_lines + 1] = line
+      end
+    end,
+    on_exit = function(_, code, _)
+      --- NOTE: commands like `rg` return a non-zero exit code when there are no matches, which is okay.
+      --- So we only log no-zero exit codes as errors when there's also stderr lines.
+      if code > 0 and #stderr_lines > 0 then
+        echo.err(
+          "Command '"
+            .. cmd
+            .. "' with arguments "
+            .. vim.inspect(args)
+            .. " exited with non-zero exit code "
+            .. code
+            .. "\n\n[stderr]\n\n"
+            .. table.concat(stderr_lines, "\n")
+        )
+      elseif #stderr_lines > 0 then
+        echo.warn(
+          "Captured stderr output while running command '"
+            .. cmd
+            .. " with arguments "
+            .. vim.inspect(args)
+            .. ":\n"
+            .. table.concat(stderr_lines)
+        )
+      end
+      if on_exit ~= nil then
+        on_exit(code)
+      end
+    end,
+  }
+end
+
+---@param cmd string
+---@param args string[]
+---@param on_stdout function|? (string) -> nil
+---@param on_exit function|? (integer) -> nil
+M.run_job = function(cmd, args, on_stdout, on_exit)
+  local job = init_job(cmd, args, on_stdout, on_exit)
+  job:sync()
+end
+
+---@param cmd string
+---@param args string[]
+---@param on_stdout function|? (string) -> nil
+---@param on_exit function|? (integer) -> nil
+M.run_job_async = function(cmd, args, on_stdout, on_exit)
+  local job = init_job(cmd, args, on_stdout, on_exit)
+  job:start()
+end
+
 return M

lua/obsidian/client.lua

@@ -134,7 +134,7 @@ end
 ---
 ---@param search string
 ---@param search_opts string[]|?
----@param callback function
+---@param callback function (obsidian.Note[]) -> nil
 Client.search_async = function(self, search, search_opts, callback)
   local async = require "plenary.async"
   local next_path = self:_search_iter_async(search, search_opts)

lua/obsidian/command.lua

@@ -1,9 +1,9 @@
 local Path = require "plenary.path"
-local Job = require "plenary.job"
 local Note = require "obsidian.note"
 local echo = require "obsidian.echo"
 local util = require "obsidian.util"
 local search = require "obsidian.search"
+local run_job = require("obsidian.async").run_job
 
 local command = {}
 
@@ -109,6 +109,7 @@ command.open = function(client, data)
     -- bufname is an absolute path to the buffer.
     local bufname = vim.api.nvim_buf_get_name(0)
     local vault_name_escaped = vault_name:gsub("%W", "%%%0") .. "%/"
+    ---@diagnostic disable-next-line: undefined-field
     if vim.loop.os_uname().sysname == "Windows_NT" then
       bufname = bufname:gsub("/", "\\")
       vault_name_escaped = vault_name_escaped:gsub("/", [[\%\]])
@@ -172,15 +173,7 @@ command.open = function(client, data)
     return
   end
 
-  Job:new({
-    command = cmd,
-    args = args,
-    on_exit = vim.schedule_wrap(function(_, return_code)
-      if return_code > 0 then
-        echo.err("failed opening Obsidian app to note", client.opts.log_level)
-      end
-    end),
-  }):start()
+  run_job(cmd, args)
 end
 
 ---Get backlinks to a note.

lua/obsidian/echo.lua

@@ -62,6 +62,14 @@ echo.err = function(msg, log_level)
   end
 end
 
+---@param msg any
+---@param log_level integer|?
+echo.err_once = function(msg, log_level)
+  if log_level == nil or log_level <= vim.log.levels.ERROR then
+    echo.echo_once(msg, vim.log.levels.ERROR)
+  end
+end
+
 ---@param msg any
 echo.fail = function(msg)
   error("[Obsidian] " .. msg)

lua/obsidian/search.lua

@@ -1,8 +1,8 @@
 local Path = require "plenary.path"
-local Job = require "plenary.job"
 local Deque = require("plenary.async.structs").Deque
 local scan = require "plenary.scandir"
 local util = require "obsidian.util"
+local run_job_async = require("obsidian.async").run_job_async
 
 local M = {}
 
@@ -84,27 +84,21 @@ end
 ---@param dir string|Path
 ---@param term string
 ---@param opts string[]|?
----@param on_match function(MatchData)
----@param on_exit function(integer) |?
+---@param on_match function (match: MatchData) -> nil
+---@param on_exit function|? (exit_code: integer) -> nil
 M.search_async = function(dir, term, opts, on_match, on_exit)
   local cmd = M.build_search_cmd(dir, term, opts, false)
-  Job:new({
-    command = cmd[1],
-    args = { unpack(cmd, 2) },
-    on_stdout = function(err, line)
-      assert(not err, err)
-      local data = vim.json.decode(line)
-      if data["type"] == "match" then
-        local match_data = data.data
-        on_match(match_data)
-      end
-    end,
-    on_exit = function(_, code, _)
-      if on_exit ~= nil then
-        on_exit(code)
-      end
-    end,
-  }):start()
+  run_job_async(cmd[1], { unpack(cmd, 2) }, function(line)
+    local data = vim.json.decode(line)
+    if data["type"] == "match" then
+      local match_data = data.data
+      on_match(match_data)
+    end
+  end, function(code)
+    if on_exit ~= nil then
+      on_exit(code)
+    end
+  end)
 end
 
 M.FIND_CMD = { "rg", "--no-config", "--files", "--type=md" }
@@ -183,24 +177,18 @@ end
 ---@param sort_by string|?
 ---@param sort_reversed boolean|?
 ---@param opts string[]|?
----@param on_match function(string)
----@param on_exit function(integer) |?
+---@param on_match function (string) -> nil
+---@param on_exit function|? (integer) -> nil
 M.find_async = function(dir, term, sort_by, sort_reversed, opts, on_match, on_exit)
   local norm_dir = vim.fs.normalize(tostring(dir))
   local cmd = M.build_find_cmd(norm_dir, sort_by, sort_reversed, term, opts, false)
-  Job:new({
-    command = cmd[1],
-    args = { unpack(cmd, 2) },
-    on_stdout = function(err, line)
-      assert(not err, err)
-      on_match(line)
-    end,
-    on_exit = function(_, code, _)
-      if on_exit ~= nil then
-        on_exit(code)
-      end
-    end,
-  }):start()
+  run_job_async(cmd[1], { unpack(cmd, 2) }, function(line)
+    on_match(line)
+  end, function(code)
+    if on_exit ~= nil then
+      on_exit(code)
+    end
+  end)
 end
 
 ---Find all notes with the given file_name recursively in a directory.