run server installation in async execution context (#525)

12 files changed, 329 insertions, 98 deletions

diff --git a/lua/nvim-lsp-installer/core/async/init.lua b/lua/nvim-lsp-installer/core/async/init.lua
index 45bf2398..85f4c83c 100644
--- a/lua/nvim-lsp-installer/core/async/init.lua
+++ b/lua/nvim-lsp-installer/core/async/init.lua
@@ -79,6 +79,10 @@ local function new_execution_context(suspend_fn, callback, ...)
     end
 end
 
+exports.run = function(suspend_fn, callback)
+    return new_execution_context(suspend_fn, callback)
+end
+
 exports.scope = function(suspend_fn)
     return function(...)
         return new_execution_context(suspend_fn, function() end, ...)
diff --git a/lua/nvim-lsp-installer/core/async/spawn.lua b/lua/nvim-lsp-installer/core/async/spawn.lua
index e643f098..5fc7eee7 100644
--- a/lua/nvim-lsp-installer/core/async/spawn.lua
+++ b/lua/nvim-lsp-installer/core/async/spawn.lua
@@ -1,34 +1,61 @@
 local a = require "nvim-lsp-installer.core.async"
+local Result = require "nvim-lsp-installer.core.result"
 local process = require "nvim-lsp-installer.process"
 local platform = require "nvim-lsp-installer.platform"
 
 local async_spawn = a.promisify(process.spawn)
 
+---@type Record<string, fun(opts: JobSpawnOpts): Result>
 local spawn = {
     aliases = {
         npm = platform.is_win and "npm.cmd" or "npm",
     },
 }
 
+local function Failure(err, cmd)
+    return Result.failure(setmetatable(err, {
+        __tostring = function()
+            return ("spawn: %s failed with exit code %d"):format(cmd, err.exit_code)
+        end,
+    }))
+end
+
 setmetatable(spawn, {
     __index = function(self, k)
         return function(args)
-            local stdio = process.in_memory_sink()
             local cmd_args = {}
             for i, arg in ipairs(args) do
                 cmd_args[i] = arg
             end
             ---@type JobSpawnOpts
             local spawn_args = {
-                stdio_sink = stdio.sink,
+                stdio_sink = args.stdio_sink,
                 cwd = args.cwd,
                 env = args.env,
                 args = cmd_args,
             }
+
+            local stdio
+            if not spawn_args.stdio_sink then
+                stdio = process.in_memory_sink()
+                spawn_args.stdio_sink = stdio.sink
+            end
+
             local cmd = self.aliases[k] or k
             local _, exit_code = async_spawn(cmd, spawn_args)
-            assert(exit_code == 0, ("%q exited with an error code: %d."):format(cmd, exit_code))
-            return table.concat(stdio.buffers.stdout, ""), table.concat(stdio.buffers.stderr, "")
+
+            if exit_code == 0 then
+                return Result.success {
+                    stdout = stdio and table.concat(stdio.buffers.stdout, "") or nil,
+                    stderr = stdio and table.concat(stdio.buffers.stderr, "") or nil,
+                }
+            else
+                return Failure({
+                    exit_code = exit_code,
+                    stdout = stdio and table.concat(stdio.buffers.stdout, "") or nil,
+                    stderr = stdio and table.concat(stdio.buffers.stderr, "") or nil,
+                }, cmd)
+            end
         end
     end,
 })
diff --git a/lua/nvim-lsp-installer/core/result.lua b/lua/nvim-lsp-installer/core/result.lua
index 33623c66..a94ffe52 100644
--- a/lua/nvim-lsp-installer/core/result.lua
+++ b/lua/nvim-lsp-installer/core/result.lua
@@ -32,6 +32,14 @@ function Result:get_or_nil()
     end
 end
 
+function Result:get_or_throw()
+    if self:is_success() then
+        return self.value
+    else
+        error(self.value.error, 2)
+    end
+end
+
 function Result:err_or_nil()
     if self:is_failure() then
         return self.value.error
@@ -46,4 +54,27 @@ function Result:is_success()
     return getmetatable(self.value) ~= Failure
 end
 
+---@param mapper_fn fun(value: any): any
+function Result:map(mapper_fn)
+    if self:is_success() then
+        return Result.success(mapper_fn(self.value))
+    else
+        return self
+    end
+end
+
+---@param mapper_fn fun(value: any): any
+function Result:map_catching(mapper_fn)
+    if self:is_success() then
+        local ok, result = pcall(mapper_fn, self.value)
+        if ok then
+            return Result.success(result)
+        else
+            return Result.failure(result)
+        end
+    else
+        return self
+    end
+end
+
 return Result
diff --git a/lua/nvim-lsp-installer/jobs/version-check/init.lua b/lua/nvim-lsp-installer/jobs/version-check/init.lua
index 11df3721..da33ea9d 100644
--- a/lua/nvim-lsp-installer/jobs/version-check/init.lua
+++ b/lua/nvim-lsp-installer/jobs/version-check/init.lua
@@ -5,6 +5,7 @@ local pip3 = require "nvim-lsp-installer.installers.pip3"
 local gem = require "nvim-lsp-installer.installers.gem"
 local cargo_check = require "nvim-lsp-installer.jobs.outdated-servers.cargo"
 local gem_check = require "nvim-lsp-installer.jobs.outdated-servers.gem"
+local pip3_check = require "nvim-lsp-installer.jobs.outdated-servers.pip3"
 local spawn = require "nvim-lsp-installer.core.async.spawn"
 
 local M = {}
@@ -21,15 +22,6 @@ local function version_in_receipt(field_name)
     end
 end
 
----@param package string
----@return string
----@TODO DRY
-local function normalize_pip3_package(package)
-    -- https://stackoverflow.com/a/60307740
-    local s = package:gsub("%[.*%]", "")
-    return s
-end
-
 local function noop()
     return Result.failure "Unable to detect version."
 end
@@ -37,16 +29,17 @@ end
 ---@type Record<InstallReceiptSourceType, fun(server: Server, receipt: InstallReceipt): Result>
 local version_checker = {
     ["npm"] = function(server, receipt)
-        local stdout = spawn.npm {
+        return spawn.npm({
             "ls",
             "--json",
             cwd = server.root_dir,
-        }
-        local npm_packages = vim.json.decode(stdout)
-        return Result.success(npm_packages.dependencies[receipt.primary_source.package].version)
+        }):map_catching(function(result)
+            local npm_packages = vim.json.decode(result.stdout)
+            return npm_packages.dependencies[receipt.primary_source.package].version
+        end)
     end,
     ["pip3"] = function(server, receipt)
-        local stdout = spawn.python3 {
+        return spawn.python3({
             "-m",
             "pip",
             "list",
@@ -54,54 +47,58 @@ local version_checker = {
             "json",
             cwd = server.root_dir,
             env = process.graft_env(pip3.env(server.root_dir)),
-        }
-        local pip_packages = vim.json.decode(stdout)
-        local normalized_pip_package = normalize_pip3_package(receipt.primary_source.package)
-        for _, pip_package in ipairs(pip_packages) do
-            if pip_package.name == normalized_pip_package then
-                return Result.success(pip_package.version)
+        }):map_catching(function(result)
+            local pip_packages = vim.json.decode(result.stdout)
+            local normalized_pip_package = pip3_check.normalize_package(receipt.primary_source.package)
+            for _, pip_package in ipairs(pip_packages) do
+                if pip_package.name == normalized_pip_package then
+                    return pip_package.version
+                end
             end
-        end
-        return Result.failure "Failed to find pip package version."
+            error "Unable to find pip package."
+        end)
     end,
     ["gem"] = function(server, receipt)
-        local stdout = spawn.gem {
+        return spawn.gem({
             "list",
             cwd = server.root_dir,
             env = process.graft_env(gem.env(server.root_dir)),
-        }
-        local gems = gem_check.parse_gem_list_output(stdout)
-        if gems[receipt.primary_source.package] then
-            return Result.success(gems[receipt.primary_source.package])
-        else
-            return Result.failure "Failed to find gem package version."
-        end
+        }):map_catching(function(result)
+            local gems = gem_check.parse_gem_list_output(result.stdout)
+            if gems[receipt.primary_source.package] then
+                return gems[receipt.primary_source.package]
+            else
+                error "Failed to find gem package version."
+            end
+        end)
     end,
     ["cargo"] = function(server, receipt)
-        local stdout = spawn.cargo {
+        return spawn.cargo({
             "install",
             "--list",
             "--root",
             server.root_dir,
             cwd = server.root_dir,
-        }
-        local crates = cargo_check.parse_installed_crates(stdout)
-        a.scheduler() -- needed because vim.fn.* call
-        local package = vim.fn.fnamemodify(receipt.primary_source.package, ":t")
-        if crates[package] then
-            return Result.success(crates[package])
-        else
-            return Result.failure "Failed to find cargo package version."
-        end
+        }):map_catching(function(result)
+            local crates = cargo_check.parse_installed_crates(result.stdout)
+            a.scheduler() -- needed because vim.fn.* call
+            local package = vim.fn.fnamemodify(receipt.primary_source.package, ":t")
+            if crates[package] then
+                return crates[package]
+            else
+                error "Failed to find cargo package version."
+            end
+        end)
     end,
     ["git"] = function(server)
-        local stdout = spawn.git {
+        return spawn.git({
             "rev-parse",
             "--short",
             "HEAD",
             cwd = server.root_dir,
-        }
-        return Result.success(vim.trim(stdout))
+        }):map_catching(function(result)
+            return vim.trim(result.stdout)
+        end)
     end,
     ["opam"] = noop,
     ["dotnet"] = noop,
diff --git a/lua/nvim-lsp-installer/server.lua b/lua/nvim-lsp-installer/server.lua
index 948d11db..5597bba7 100644
--- a/lua/nvim-lsp-installer/server.lua
+++ b/lua/nvim-lsp-installer/server.lua
@@ -1,4 +1,5 @@
 local dispatcher = require "nvim-lsp-installer.dispatcher"
+local a = require "nvim-lsp-installer.core.async"
 local fs = require "nvim-lsp-installer.fs"
 local log = require "nvim-lsp-installer.log"
 local platform = require "nvim-lsp-installer.platform"
@@ -207,57 +208,47 @@ end
 ---@param context ServerInstallContext
 ---@param callback ServerInstallCallback
 function M.Server:install_attached(context, callback)
-    context.receipt = receipt.InstallReceiptBuilder.new()
-    context.receipt:with_start_time(vim.loop.gettimeofday())
-    local context_ok, context_err = pcall(self._setup_install_context, self, context)
-    if not context_ok then
-        log.error("Failed to setup installation context.", context_err)
-        callback(false)
-        return
-    end
-    local install_ok, install_err = pcall(
-        self._installer,
-        self,
-        vim.schedule_wrap(function(success)
-            if success then
-                if not self:promote_install_dir(context.install_dir) then
-                    context.stdio_sink.stderr(
-                        ("Failed to promote the temporary installation directory %q.\n"):format(context.install_dir)
-                    )
-                    pcall(fs.rmrf, self:get_tmp_install_dir())
-                    pcall(fs.rmrf, context.install_dir)
-                    callback(false)
-                    return
-                end
+    a.run(
+        function()
+            context.receipt = receipt.InstallReceiptBuilder.new()
+            context.receipt:with_start_time(vim.loop.gettimeofday())
 
-                -- The tmp dir should in most cases have been "promoted" and already renamed to its final destination,
-                -- but we make sure to delete it should the installer modify the installation working directory during
-                -- installation.
-                pcall(fs.rmrf, self:get_tmp_install_dir())
+            a.scheduler()
+            self:_setup_install_context(context)
+            local async_installer = a.promisify(function(server, context, callback)
+                -- args are shifted
+                return self._installer(server, callback, context)
+            end)
+            assert(async_installer(self, context), "Installation failed.")
 
-                self:_write_receipt(context.receipt)
+            a.scheduler()
+            if not self:promote_install_dir(context.install_dir) then
+                error(("Failed to promote the temporary installation directory %q."):format(context.install_dir))
+            end
 
-                -- Dispatch the server is ready
-                vim.schedule(function()
-                    dispatcher.dispatch_server_ready(self)
-                    for _, on_ready_handler in ipairs(self._on_ready_handlers) do
-                        on_ready_handler(self)
-                    end
-                end)
-                callback(true)
-            else
-                pcall(fs.rmrf, self:get_tmp_install_dir())
+            self:_write_receipt(context.receipt)
+
+            -- Dispatch the server is ready
+            vim.schedule(function()
+                dispatcher.dispatch_server_ready(self)
+                for _, on_ready_handler in ipairs(self._on_ready_handlers) do
+                    on_ready_handler(self)
+                end
+            end)
+        end,
+        vim.schedule_wrap(function(ok, result)
+            if not ok then
                 pcall(fs.rmrf, context.install_dir)
-                callback(false)
+                log.fmt_error("Server installation failed, server_name=%s, error=%s", self.name, result)
+                context.stdio_sink.stderr(tostring(result) .. "\n")
             end
-        end),
-        context
+            -- The tmp dir should in most cases have been "promoted" and already renamed to its final destination,
+            -- but we make sure to delete it should the installer modify the installation working directory during
+            -- installation.
+            pcall(fs.rmrf, self:get_tmp_install_dir())
+            callback(ok)
+        end)
     )
-    if not install_ok then
-        log.error("Installer raised an unexpected error.", install_err)
-        context.stdio_sink.stderr(tostring(install_err) .. "\n")
-        callback(false)
-    end
 end
 
 function M.Server:uninstall()
diff --git a/lua/nvim-lsp-installer/ui/display.lua b/lua/nvim-lsp-installer/ui/display.lua
index 889c3ac3..30c6ca75 100644
--- a/lua/nvim-lsp-installer/ui/display.lua
+++ b/lua/nvim-lsp-installer/ui/display.lua
@@ -201,11 +201,11 @@ function M.delete_win_buf(win_id, bufnr)
     -- It should probably be unnecessary once https://github.com/neovim/neovim/issues/15548 is resolved
     vim.schedule(function()
         if win_id and vim.api.nvim_win_is_valid(win_id) then
-            log.debug "Deleting window"
+            log.trace "Deleting window"
             vim.api.nvim_win_close(win_id, true)
         end
         if bufnr and vim.api.nvim_buf_is_valid(bufnr) then
-            log.debug "Deleting buffer"
+            log.trace "Deleting buffer"
             vim.api.nvim_buf_delete(bufnr, { force = true })
         end
         if redraw_by_win_id[win_id] then
@@ -305,7 +305,7 @@ function M.new_view_only_win(name)
         if not win_valid or not buf_valid then
             -- the window has been closed or the buffer is somehow no longer valid
             unsubscribe(true)
-            log.debug("Buffer or window is no longer valid", win_id, bufnr)
+            log.trace("Buffer or window is no longer valid", win_id, bufnr)
             return
         end
 
@@ -398,7 +398,7 @@ function M.new_view_only_win(name)
         ---@alias DisplayOpenOpts {effects: table<string, fun()>, highlight_groups: string[]|nil}
         ---@type fun(opts: DisplayOpenOpts)
         open = vim.schedule_wrap(function(opts)
-            log.debug "Opening window"
+            log.trace "Opening window"
             assert(has_initiated, "Display has not been initiated, cannot open.")
             if win_id and vim.api.nvim_win_is_valid(win_id) then
                 -- window is already open
diff --git a/lua/nvim-lsp-installer/ui/status-win/init.lua b/lua/nvim-lsp-installer/ui/status-win/init.lua
index beb788ed..810c45cc 100644
--- a/lua/nvim-lsp-installer/ui/status-win/init.lua
+++ b/lua/nvim-lsp-installer/ui/status-win/init.lua
@@ -688,7 +688,7 @@ local function init(all_servers)
             mutate_state(function(state)
                 if success then
                     -- release stdout/err output table.. hopefully ¯\_(ツ)_/¯
-                    state.servers[server.name].installer.tailed_output = {}
+                    state.servers[server.name].installer.tailed_output = { "" }
                 end
                 state.servers[server.name].is_installed = success
                 state.servers[server.name].installer.is_running = false
diff --git a/stylua.toml b/stylua.toml
index f307a39f..eb741557 100644
--- a/stylua.toml
+++ b/stylua.toml
@@ -1,2 +1,2 @@
 indent_type = "Spaces"
-no_call_parentheses = true
+call_parentheses = "None"
diff --git a/tests/core/async_spec.lua b/tests/core/async/async_spec.lua
index 88af14af..88af14af 100644
--- a/tests/core/async_spec.lua
+++ b/tests/core/async/async_spec.lua
diff --git a/tests/core/async/spawn_spec.lua b/tests/core/async/spawn_spec.lua
new file mode 100644
index 00000000..721e4d08
--- /dev/null
+++ b/tests/core/async/spawn_spec.lua
@@ -0,0 +1,47 @@
+local spawn = require "nvim-lsp-installer.core.async.spawn"
+local process = require "nvim-lsp-installer.process"
+
+describe("async spawn", function()
+    it(
+        "should spawn commands and return stdout & stderr",
+        async_test(function()
+            local result = spawn.env {
+                env = { "FOO=bar" },
+            }
+            assert.is_true(result:is_success())
+            assert.equals("FOO=bar\n", result:get_or_nil().stdout)
+            assert.equals("", result:get_or_nil().stderr)
+        end)
+    )
+
+    it(
+        "should use provided stdio_sink",
+        async_test(function()
+            local stdio = process.in_memory_sink()
+            local result = spawn.env {
+                env = { "FOO=bar" },
+                stdio_sink = stdio.sink,
+            }
+            assert.is_true(result:is_success())
+            assert.equals(nil, result:get_or_nil().stdout)
+            assert.equals(nil, result:get_or_nil().stderr)
+            assert.equals("FOO=bar\n", table.concat(stdio.buffers.stdout, ""))
+            assert.equals("", table.concat(stdio.buffers.stderr, ""))
+        end)
+    )
+
+    it(
+        "should pass command arguments",
+        async_test(function()
+            local result = spawn.bash {
+                "-c",
+                'echo "Hello $VAR"',
+                env = { "VAR=world" },
+            }
+
+            assert.is_true(result:is_success())
+            assert.equals("Hello world\n", result:get_or_nil().stdout)
+            assert.equals("", result:get_or_nil().stderr)
+        end)
+    )
+end)
diff --git a/tests/core/result_spec.lua b/tests/core/result_spec.lua
new file mode 100644
index 00000000..725041de
--- /dev/null
+++ b/tests/core/result_spec.lua
@@ -0,0 +1,88 @@
+local Result = require "nvim-lsp-installer.core.result"
+local match = require "luassert.match"
+
+describe("result", function()
+    it("should create success", function()
+        local result = Result.success "Hello!"
+        assert.is_true(result:is_success())
+        assert.is_false(result:is_failure())
+        assert.equals("Hello!", result:get_or_nil())
+    end)
+
+    it("should create failure", function()
+        local result = Result.failure "Hello!"
+        assert.is_true(result:is_failure())
+        assert.is_false(result:is_success())
+        assert.equals("Hello!", result:err_or_nil())
+    end)
+
+    it("should return value on get_or_throw()", function()
+        local result = Result.success "Hello!"
+        local val = result:get_or_throw()
+        assert.equals("Hello!", val)
+    end)
+
+    it("should throw failure on get_or_throw()", function()
+        local result = Result.failure "Hello!"
+        local err = assert.has_error(function()
+            result:get_or_throw()
+        end)
+        assert.equals("Hello!", err)
+    end)
+
+    it("should map() success", function()
+        local result = Result.success "Hello"
+        local mapped = result:map(function(x)
+            return x .. " World!"
+        end)
+        assert.equals("Hello World!", mapped:get_or_nil())
+        assert.is_nil(mapped:err_or_nil())
+    end)
+
+    it("should not map() failure", function()
+        local result = Result.failure "Hello"
+        local mapped = result:map(function(x)
+            return x .. " World!"
+        end)
+        assert.equals("Hello", mapped:err_or_nil())
+        assert.is_nil(mapped:get_or_nil())
+    end)
+
+    it("should raise exceptions in map()", function()
+        local result = Result.success "failure"
+        local err = assert.has_error(function()
+            result:map(function()
+                error "error"
+            end)
+        end)
+        assert.equals("error", err)
+    end)
+
+    it("should map_catching() success", function()
+        local result = Result.success "Hello"
+        local mapped = result:map_catching(function(x)
+            return x .. " World!"
+        end)
+        assert.equals("Hello World!", mapped:get_or_nil())
+        assert.is_nil(mapped:err_or_nil())
+    end)
+
+    it("should not map_catching() failure", function()
+        local result = Result.failure "Hello"
+        local mapped = result:map_catching(function(x)
+            return x .. " World!"
+        end)
+        assert.equals("Hello", mapped:err_or_nil())
+        assert.is_nil(mapped:get_or_nil())
+    end)
+
+    it("should catch errors in map_catching()", function()
+        local result = Result.success "value"
+        local mapped = result:map_catching(function()
+            error "This is an error"
+        end)
+        assert.is_false(mapped:is_success())
+        assert.is_true(mapped:is_failure())
+        assert.is_true(match.has_match "This is an error$"(mapped:err_or_nil()))
+    end)
+end)
diff --git a/tests/server_spec.lua b/tests/server_spec.lua
index 744b4cf8..81ae83b0 100644
--- a/tests/server_spec.lua
+++ b/tests/server_spec.lua
@@ -2,6 +2,13 @@ local spy = require "luassert.spy"
 local lsp_installer = require "nvim-lsp-installer"
 local server = require "nvim-lsp-installer.server"
 local a = require "nvim-lsp-installer.core.async"
+local context = require "nvim-lsp-installer.installers.context"
+local fs = require "nvim-lsp-installer.fs"
+
+local function timestamp()
+    local seconds, microseconds = vim.loop.gettimeofday()
+    return (seconds * 1000) + math.floor(microseconds / 1000)
+end
 
 describe("server", function()
     it(
@@ -47,4 +54,43 @@ describe("server", function()
             assert.is_false(srv:is_installed())
         end)
     )
+
+    it(
+        "should remove directories upon installation failure",
+        async_test(function()
+            local srv = FailingServerGenerator {
+                name = "remove_dirs_failure",
+                root_dir = server.get_server_root_path "remove_dirs_failure",
+                installer = {
+                    -- 1. sleep 500ms
+                    function(_, callback)
+                        vim.defer_fn(function()
+                            callback(true)
+                        end, 500)
+                    end,
+                    -- 2. promote install dir
+                    context.promote_install_dir(),
+                    -- 3. fail
+                    function(_, callback)
+                        callback(false)
+                    end,
+                },
+            }
+            srv:install()
+
+            -- 1. installation started
+            a.sleep(50)
+            assert.is_true(fs.dir_exists(srv:get_tmp_install_dir()))
+
+            -- 2. install dir promoted
+            a.sleep(500)
+            assert.is_false(fs.dir_exists(srv:get_tmp_install_dir()))
+
+            -- 3. installation failed
+            a.sleep(200)
+
+            assert.is_false(srv:is_installed())
+            assert.is_false(fs.dir_exists(srv:get_tmp_install_dir()))
+        end)
+    )
 end)