neovim icon indicating copy to clipboard operation
neovim copied to clipboard

Lua: structured concurrency, Promises, task pipelines

Open justinmk opened this issue 2 years ago • 75 comments

related: https://github.com/neovim/neovim/issues/11312

Problem

There's no builtin abstraction for:

  • Representing a task.
    • Example: vim.system() returns its own ad-hoc "task" that can be "awaited" via :wait().
  • Orchestrating "pipelines" (quasi monads?) of work ("tasks") and handling errors.
    • Example: shell-like task chains: vim.fs.files():filter(..):map(function(f) vim.fs.delete(f) end)
    • See also vim.iter.

Request for comments

What are the current patterns for queuing promises in Lua, that are being used in the wild for Nvim plugins (and Nvim core itself)? Are these good enough? Do we need any sugar in the Nvim lua stdlib to encourage these patterns?

Expected behavior

Task or Promise abstraction:

  • "Promise" abstraction (or something better), maximally leveraging Lua coroutines + libuv.
  • "Task" abstraction (could this be avoided by overloading the "Promise" concept (or vice versa)?).
  • Function that creates an awaitable task from "normal" functions.
    • Don't want to have to call await everywhere, that is a horrible result of JavaScript's async/await model.
    • Instead, could something like vim.system() be "promisified" without its knowledge? Or could it handle differently when it detects that it's in a coroutine? So vim.system() would be synchronous normally, but vim.async(vim.system, ...):wait() would be asynchronous.

Structured concurrency:

  • await_all, await_any (pseudo-names). See javascript's Promise.all().
  • Programmer can handle errors and cancellations.
  • Programmer can cancel specific coroutines/promises.
  • Aggregate results
    • This is something jobwait() currently doesn't help with.

Implementations

  • https://github.com/lewis6991/async.nvim
  • https://github.com/kevinhwang91/promise-async
  • https://github.com/ms-jpq/lua-async-await

Related

  • https://github.com/sourcegraph/conc
  • https://github.com/bitfield/script (example of "pipeline" interface)

justinmk avatar Aug 02 '22 11:08 justinmk

I believe the gold standard is https://github.com/nvim-lua/plenary.nvim#plenaryasync, but @lewis6991 rolled his own for GitSigns. Either could conceivably be upstreamed if deemed useful enough.

clason avatar Aug 02 '22 11:08 clason

For context here's the complete implementation Gitsigns uses:

local co = coroutine

local async_thread = {
   threads = {},
}

local function threadtostring(x)
   if jit then
      return string.format('%p', x)
   else
      return tostring(x):match('thread: (.*)')
   end
end

function async_thread.running()
   local thread = co.running()
   local id = threadtostring(thread)
   return async_thread.threads[id]
end

function async_thread.create(fn)
   local thread = co.create(fn)
   local id = threadtostring(thread)
   async_thread.threads[id] = true
   return thread
end

function async_thread.finished(x)
   if co.status(x) == 'dead' then
      local id = threadtostring(x)
      async_thread.threads[id] = nil
      return true
   end
   return false
end

local function execute(async_fn, ...)
   local thread = async_thread.create(async_fn)

   local function step(...)
      local ret = { co.resume(thread, ...) }
      local stat, err_or_fn, nargs = unpack(ret)

      if not stat then
         error(string.format("The coroutine failed with this message: %s\n%s",
         err_or_fn, debug.traceback(thread)))
      end

      if async_thread.finished(thread) then
         return
      end

      assert(type(err_or_fn) == "function", "type error :: expected func")

      local ret_fn = err_or_fn
      local args = { select(4, unpack(ret)) }
      args[nargs] = step
      ret_fn(unpack(args, 1, nargs))
   end

   step(...)
end

local M = {}

function M.wrap(func, argc)
   return function(...)
      if not async_thread.running() then
         return func(...)
      end
      return co.yield(func, argc, ...)
   end
end

function M.void(func)
   return function(...)
      if async_thread.running() then
         return func(...)
      end
      execute(func, ...)
   end
end

M.scheduler = M.wrap(vim.schedule, 1)

return M

I basically just copy and paste this for all my plugins now.

lewis6991 avatar Aug 02 '22 12:08 lewis6991

I basically just copy and paste this for all my plugins now.

That's a good sign. Does it support cancellation? How does error handling look? How does it look to aggregate results?

justinmk avatar Aug 02 '22 12:08 justinmk

That's a good sign. Does it support cancellation? How does error handling look? How does it look to aggregate results?

It doesn't support issuing out several coroutines with any kind of join, so there is nothing to cancel or aggregate, the 95% usecase is for use with sequential system commands (via vim.loop.spawn). We could implement that kind of stuff, I just haven't found a situation yet where I've really needed it.

How does error handling look?

Here's an example of injecting error("I'm an error") mid-way through setup() in Gitsigns:

Error executing vim.schedule lua callback: ...e/pack/packer/start/gitsigns.nvim/lua/gitsigns/async.lua:67: The coroutine failed with this message: ...im/site/p
ack/packer/start/gitsigns.nvim/lua/gitsigns.lua:429: I'm an error
stack traceback:
        [C]: in function 'error'
        ...im/site/pack/packer/start/gitsigns.nvim/lua/gitsigns.lua:429: in function <...im/site/pack/packer/start/gitsigns.nvim/lua/gitsigns.lua:393>
stack traceback:
        [C]: in function 'error'
        ...e/pack/packer/start/gitsigns.nvim/lua/gitsigns/async.lua:67: in function <...e/pack/packer/start/gitsigns.nvim/lua/gitsigns/async.lua:62>

It just issues out both stacktraces: one from the application code, and one from the async lib. I've found it useful enough that I don't feel the need to improve it.

lewis6991 avatar Aug 02 '22 12:08 lewis6991

I think this would be a great addition to the stdlib. I've been playing around with async implementations as well, and I'd like to bring forward some topics that I think are worth consideration:

  • How to handle situations where the callback that resumes execution is being invoked more than once.

  • Should callback signatures be standardized to allow easy translation between async <-> callback-style invocations? (e.g. fun(..., callback: fun (err: any?, result: any?))) For example:

    -- callback style
    vim.fs.find(".git", function (err, path)
      -- ...
    end)
    
    -- async style
    local a = vim.async
    local async_find = a.syncify(vim.fs.find) -- this could perhaps even be done automatically by vim.fs.find if it sees that no callback is passed as last arg
    local ok, path_or_err = pcall(async_find, ".git")
    
  • How to handle nested coroutines? Should the async coroutine forward yields to parent coroutines, on what criterion?

  • Should you be able to retrieve the result of an async function from a non-async context, e.g. through a callback?

  • How to handle multiple return values across async/sync boundaries (return val1, val2)?

  • Perhaps too early to even think about but I wanted to surface the idea anyway: Should user scripts perhaps be executed in a "global" async coroutine context? This would allow direct access to async primitives without users having to explicitly create async constructs.

williamboman avatar Aug 03 '22 13:08 williamboman

How to handle situations where the callback that resumes execution is being invoked more than once.

A closure could be created that can be called only once.

fun(..., callback: fun (err: any?, result: any?)) How to handle multiple return values across async/sync boundaries (return val1, val2)?

Maybe instead it could be ok: boolean, results: any....

How to handle nested coroutines? Should the async coroutine forward yields to parent coroutines, on what criterion?

Maybe I'm missing something, but it should always resume the caller?

edit: Okay, I get what you mean now, you're asking about suspending a nested coroutine. I think both behaviors should be supported, the standard way of using coroutine.yield directly, but also a way of suspending the entire call stack. I think this could be done by yielding a special constant, based on which the parent coroutine would decide whether to suspend or not, until it hits the main thread.

ii14 avatar Aug 03 '22 22:08 ii14

What are the current patterns for queuing promises in Lua, that are being used in the wild for Nvim plugins (and Nvim core itself)? Are these good enough? Do we need any sugar in the Nvim lua stdlib to encourage these patterns?

In some of my plugins I started using the pattern norcalli mentioned in https://github.com/neovim/neovim/issues/11312#issuecomment-548042444:

local function send_request_sync(method, params)
	local co = coroutine.running()
	send_request(method, params, function(...) coroutine.resume(co, ...) end)
	return coroutine.yield()
end

and found that it's both simple and effective in avoiding callback nesting.

I think if we were to add some async/await or promise abstraction it would help to formulate the problem first, to help ensure we're solving the right thing. E.g. I can see how functionality to race two or more operations, cancellation or waiting for multiple operations can be useful and the LSP stuff in neovim could probably benefit for some of it. But if all it does is add some sugar over the coroutine pattern above then I'd say it's not worth it.

mfussenegger avatar Aug 04 '22 07:08 mfussenegger

simple implementation of aysnc await is easy and can works well.

function async(f, ...)
    local co = coroutine.create(f)
    local function exec(...)
        local ok, data = coroutine.resume(co, ...)
        if not ok then
            error(debug.traceback(co, data))
        end
        if coroutine.status(co) ~= "dead" then
            data(exec)
        end
    end
    exec(...)
end

function await(f)
    local co = coroutine.running()
    local ret
    f(function(...)
        if coroutine.status(co) == "running" then
            ret = table.pack(...)
        else
            return coroutine.resume(co, ...)
        end
    end)
    if ret then
        return table.unpack(ret, 1, ret.n)
    else
        return coroutine.yield()
    end
end

if want to support async/await in builtin . I think it should be more complete . support range , promise and status will be better

-- make ranged for loop 
range(start, stop, [step])
--promise
promis.new(function(resolve,reject))

--chain call
promise:chain_call(func)

-- cache
promis:catch(function)

-- await
promise:await()

-- resolve
promise:resolve(...)

-- error
promise:error(err)

-- status: Pending | Resolved | Errored
promise:status()
promise:isPending, promis:isResolved,promis:IsErroed

all/any waitForAll/waitForAny

glepnir avatar Aug 04 '22 07:08 glepnir

Perhaps ideas and implementation from https://github.com/kevinhwang91/promise-async can be useful.

muniter avatar Aug 04 '22 12:08 muniter

I've read something similar about Javascript: A Study on Solving Callbacks with JavaScript Generators, hope it might help.

Chromosore avatar Aug 07 '22 09:08 Chromosore

I am also implementing async-await for my plugin development. (I plan to use it primarily for sequencing code related to key mapping.)

Currently, I have not written the Promise.all equivalent, but will be able to add it easily.

https://github.com/hrsh7th/nvim-kit/blob/main/lua/plugin_name/Async/init.spec.lua#L27

hrsh7th avatar Aug 22 '22 06:08 hrsh7th

I think if we were to add some async/await or promise abstraction it would help to formulate the problem first, to help ensure we're solving the right thing. E.g. I can see how functionality to race two or more operations, cancellation or waiting for multiple operations can be useful and the LSP stuff in neovim could probably benefit for some of it. But if all it does is add some sugar over the coroutine pattern above then I'd say it's not worth it.

@mfussenegger Absolutely. If there are patterns we can document without adding sugar, that is definitely preferred. So to close this, we can document those patterns. Or both: maybe we only need sugar for "race two or more operations, cancellation or waiting for multiple operations".

justinmk avatar Aug 23 '22 15:08 justinmk

Using @mfussenegger suggestion in https://github.com/neovim/neovim/issues/19624#issuecomment-1204867814 I've come up with a very simple and lean implementation with some necessary safety check boilerplate:

local M = {}

function M.wrap(func, argc)
  return function(...)
    local co = coroutine.running()
    if not co then
      error('wrapped functions must be called in a coroutine')
    end

    local args = {...}
    args[argc] = function(...)
      if co == coroutine.running() then
        error('callback called in same coroutine as calling function')
      end
      coroutine.resume(co, ...)
    end
    func(unpack(args, 1, argc))
    return coroutine.yield()
  end
end

function M.void(func)
  return function(...)
    if coroutine.running() then
      return func(...)
    else
      return coroutine.wrap(func)(...)
    end
  end
end

M.scheduler = M.wrap(vim.schedule, 1)

return M

Example usage:

local function foo(x, cb)
  vim.schedule(function()
    cb(x + 1)
  end)
end

local afoo = wrap(foo, 2)

local bar = void(function(x)
  local a = afoo(x)
  print('Hello', a)
end)

bar(3)

Outputs Hello 4

lewis6991 avatar Aug 24 '22 14:08 lewis6991

Just chiming in: packer has another async/await implementation based on https://github.com/ms-jpq/lua-async-await, but I consider it to be messy and would love to rip it out in favor of a stdlib alternative. It has limited support for interrupting a sequence of jobs and aggregating results, but could be improved in its API, robustness to errors, etc.

wbthomason avatar Sep 01 '22 20:09 wbthomason

I have a lot of experience with this because I implemented the async library in plenary. The first iteration was basically a direct copy of https://github.com/ms-jpq/lua-async-await. It was not the best design because there is no point for async await with thunks. It felt like the extra thunks were just there to mirror the syntax from javascript or rust. await was literally implemented like this:

function await(f)
  f(nil)
end

It was also much harder to pass async functions to higher order functions.

The new version did not need async or await, and you can call functions just like normal synchronous functions. Furthermore, higher order functions that expect synchronous functions just work like normal. There is also no extra syntax for for loops

-- now iterators just work like normal
for entry in readdir() do
end

-- instead of
local iter = await(readdir())
while true do
  local entry = await(iter())
  if entry == nil then break end
end

Back then I didn't realize that this was basically delimited continuations. Now, I think we should just implement algebraic effects using one-shot delimited continuations which is described in this paper and implemented in eff.lua. The issue with just using coroutines for converting callback based functions is that you don't get to use coroutines for anything else. With algebraic effects, you can have other effects in addition to async effects. Algebraic effects can implement Reader (local configuration), Exceptions, State (local state), Generators, and more. Also, algebraic effects separate the effects from interpretation. For example, imagine some program using the file system effect, log effect, and read effect:

local handle = handler {
  val = function(v) return v end,
  [Log] = function(k, arg)
    print(arg)
    return k()
  end,
  [ReadFile] = function(k, path)
    return k(fs.read_file(path))
  end,
  [Read] = function(k)
    return k(config)
  end
}
handle(my_program)

If you wanted to test your functions, you can change the interpretation to not use IO.

local vfs = {my_file = "asdfasdf", another_file = "adfadsfdf"}
local logs = {}

-- handle without IO!
local handle = handler {
  val = function(v) return v end,
  [Log] = function(k, arg)
    table.insert(logs, arg)
    return k()
  end,
  [ReadFile] = function(k, path)
    return k(vfs[path])
  end,
  [Read] = function(k)
    return k(config)
  end
}

Async could be implemented like this:

local handle = handler {
  val = callback,
  [Async] = function(k, fn, ...)
    fn(k, ...)
  end
}

Exceptions:

local handle = handler {
  val = function(v) return v end,
  [Exception] = function(k, e)
    return e
  end,
}

oberblastmeister avatar Oct 16 '22 22:10 oberblastmeister

I implemented AsyncTask (which is almost like the JavaScript Promises interface, but always synchronous if possible). (The always synchronous if possible is needed to support feedkeys handling.)

If neovim core supports async-await, I'm wondering if neovim should introduce promsie like async primitives or not.

  • async-await
    • https://github.com/hrsh7th/nvim-kit/blob/main/lua/kit/kit/Async/init.lua
  • Keymap module
    • https://github.com/hrsh7th/nvim-kit/blob/main/lua/kit/kit/Vim/Keymap.spec.lua#L28
  • IO module
    • https://github.com/hrsh7th/nvim-kit/blob/main/lua/kit/kit/IO/init.lua#L114
  • Convert callback style to Promise style
    • https://github.com/hrsh7th/nvim-kit/blob/main/lua/kit/kit/IO/init.lua#L54

hrsh7th avatar Dec 02 '22 04:12 hrsh7th

I found the interesting PR. https://github.com/luvit/luv/pull/618

hrsh7th avatar Dec 03 '22 02:12 hrsh7th

@oberblastmeister I've taken a look at eff.lua, and it appears to be a completely generalized version of the current async library we're using. If you take eff.lua and the async implementation you mentioned and refactor enough, you get to the same async lib I implemented in gitsigns.

It's quite interesting, but I'm not sure if we need something as generalized as that. The main need is for an async lib to better utilize libuv and all of it's callback based functions.

The issue with just using coroutines for converting callback based functions is that you don't get to use coroutines for anything else.

eff.lua uses coroutines in the exact same way the current async lib does, so not sure this is true.

I implemented AsyncTask (which is almost like the JavaScript Promises interface, but always synchronous if possible). (The always synchronous if possible is needed to support feedkeys handling.)

If neovim core supports async-await, I'm wondering if neovim should introduce promsie like async primitives or not.

* async-await
  
  * [hrsh7th/nvim-kit@`main`/lua/___kit___/kit/Async/init.lua](https://github.com/hrsh7th/nvim-kit/blob/main/lua/___kit___/kit/Async/init.lua?rgh-link-date=2022-12-02T04%3A13%3A30Z)

* Keymap module
  
  * [hrsh7th/nvim-kit@`main`/lua/___kit___/kit/Vim/Keymap.spec.lua#L28](https://github.com/hrsh7th/nvim-kit/blob/main/lua/___kit___/kit/Vim/Keymap.spec.lua?rgh-link-date=2022-12-02T04%3A13%3A30Z#L28)

* IO module
  
  * [hrsh7th/nvim-kit@`main`/lua/___kit___/kit/IO/init.lua#L114](https://github.com/hrsh7th/nvim-kit/blob/main/lua/___kit___/kit/IO/init.lua?rgh-link-date=2022-12-02T04%3A13%3A30Z#L114)

* Convert callback style to Promise style
  
  * [hrsh7th/nvim-kit@`main`/lua/___kit___/kit/IO/init.lua#L54](https://github.com/hrsh7th/nvim-kit/blob/main/lua/___kit___/kit/IO/init.lua?rgh-link-date=2022-12-02T04%3A13%3A30Z#L54)

It looks like this implementation creates a new coroutine for every async function. This is a big drawback compared to the plenary/gitsigns implementations that uses a single coroutine for each context. It also requires await which isn't needed in the other implementations.

I've created https://github.com/lewis6991/async.nvim which I'm using as a reference for all the plugins I use async in. The library is small enough that (for now) it's ok to just vendor directly in plugins without needing it as a depedency.

lewis6991 avatar Dec 03 '22 10:12 lewis6991

Yes. My implementation creates a coroutine for each asynchronous operation. I did not think this would affect performance. I will investigate.

I also wonder that "it is the drawback that requires the call to await".

Because I believe the difference is where and when to yield asynchronous primitives.

For example, look at the following example

-- Return just AsyncTask
local function timeout_async(ms)
  return AsyncTask.new(function(resolve))
    vim.defer_fn(resolve, ms)
  end)
end

-- Return AsyncTask if it is running in the main coroutine, otherwise yield.
local function timeout(ms)
  local task = timeout_async(ms)
  if vim.async.in_context() then
    return task:await()
  end
  return task
end

vim.async.run(function())
  local s = uv.now()
  timeout(200) -- does not need to `await` call.
   assert.is_true(math.abs(uv.now() - s - 200) < 10)
end)

In this example, timeout_async just returns an AsyncTask, but timeout delegates resolution to any async coroutine context.

I think it is a difficult question as to whether we should choose Promise, thunk, or another means as an asynchronous primitive. However, it seems natural to me to introduce some kind of asynchronous primitive.

To me, the Promise style seem like a good choice, since there are many JavaScript users and they are easy to understand. Also, it is easy to convert parallel waits instead of serial ones. e.g. await Promise.all(tasks)

Translated with www.DeepL.com/Translator (free version)

hrsh7th avatar Dec 03 '22 15:12 hrsh7th

the Promise style seem like a good choice, since there are many JavaScript users and they are easy to understand.

Comforting javascript users is not a goal, and should not be factored into any of the designs here. As a typescript/javascript user myself, I'm very skeptical that async/await is a good interface. We should focus on what feels right for a Lua + Nvim.

The main need is for an async lib to better utilize libuv and all of it's callback based functions.

Since the introduce of jobstart(), long before libuv was exposed. Network tasks like LSP are another common use-case.

Sounds like https://github.com/lewis6991/async.nvim is informed by a lot of previous work, which is a good sign! Does it provide a way to cancel tasks, handle errors/cancellations, and aggregate results?

justinmk avatar Dec 04 '22 01:12 justinmk

As a typescript/javascript user myself, I'm very skeptical that async/await is a good interface. We should focus on what feels right for a Lua + Nvim.

I've been using plenary's async module with neotest for a while now and have found the lack of an async/await syntax to be much more fitting with Lua's simplistic style of programming. IMO adding async/await would be clunky.

Sounds like https://github.com/lewis6991/async.nvim is informed by a lot of previous work, which is a good sign! Does it provide a way to cancel tasks, handle errors/cancellations, and aggregate results?

I wanted to introduce the async usage to nvim-dap-ui which is similar to LSP, where it's currently using a lot of callbacks. I worked off of @lewis6991's implementation and added the ability to cancel tasks and handle errors rather than them just being raised to the user. His implementation already covers aggregating results. It's very similar to Python's asyncio Task class just because I didn't want to have to design the API much. Also added some flow control primitives.

Implementation here https://github.com/rcarriga/nvim-dap-ui/blob/feat/new-api/lua/dapui/async/base.lua and some usage in tests https://github.com/rcarriga/nvim-dap-ui/blob/feat/new-api/tests/unit/async/base_spec.lua. Excuse the long namespaced names, it's just for generating documentation.

rcarriga avatar Dec 30 '22 18:12 rcarriga

Thought I'd update as I've been having a bit of fun with this. I've done a fair bit of refactoring and adding of features in the last few days.

The main features are currently:

  1. Handling results and errors of tasks along with cancelling.
  2. Helper functions for coordinating tasks, including gathering results from a list of tasks and getting the result of the first task to complete (and cancelling the others).
  3. Async versions of many libuv functions, pretty much the same way plenary does it with the added bonus of all of the functions being typed with emmylua annotations so users will have full type inference.
  4. Creating a fully documented and typed LSP client interface which is generated by a script based off of the LSP spec meta model and then implemented very simply with some metatables here.
  5. Async primitives (queues, events and semaphores) which are often useful in async code, again based off of Python's asyncio modules.

Not sure if it's the direction core would want to go with an async library, (e.g. the LSP client but I was doing something similar for the DAP specification and LSP was mentioned as a key reason for the async usage so thought I'd slap something together for it). I think it does demonstrate the advantages of a single library by implementing broadly useful features and providing a stable, tested solution for async that can be used by anybody.

Here is some sample usage of the LSP client. Open the script in the nvim-dap-ui branch to see the type checking/documentation with Lua LSP

local async = require("dapui").async

async
  .run(function()
    local client = async.lsp.client(2)

    -- Notifications usage
    client.notify.textDocument_willSave({
      reason = "3",
      textDocument = {
        uri = vim.uri_from_bufnr(0),
      },
    })

    -- Request usage
    local symbols = client.request.textDocument_documentSymbol(0, {
      textDocument = { uri = vim.uri_from_bufnr(0) },
    }, { timeout = 1000 })

    return symbols
  end)
  .add_callback(function(task)
    if task.error() then
      vim.schedule_wrap(error)(task.error())
    else
      print(vim.inspect(task.result()))
    end
  end)

rcarriga avatar Jan 03 '23 22:01 rcarriga

What is the purpose of add_callback. Why can't that code be appended to the end of the async function?

lewis6991 avatar Jan 03 '23 22:01 lewis6991

The add_callback usage is very arbitrary here. It's just to show result/error handling. I'd imagine the most common use case would be just for error handling. add_callback would be preferred over just wrapping everything in pcall because pcall won't catch errors outside of the coroutine such as cancellation.

rcarriga avatar Jan 04 '23 08:01 rcarriga

is add_callback more commonly called then ?

justinmk avatar Jan 04 '23 08:01 justinmk

Ah yes that's true. Just named it based off of Python's add_done_callback, but I'd definitely prefer then :sweat_smile:

Edit: One key difference though is that this doesn't return a new task, so you can't chain them together like JS Promises

rcarriga avatar Jan 04 '23 09:01 rcarriga

Edit: One key difference though is that this doesn't return a new task, so you can't chain them together like JS Promises

any reason we wouldn't want that?

justinmk avatar Jan 04 '23 09:01 justinmk

I don't know about @justinmk thoughts, but I would like any implementation included in core to be fairly lean and minimal.

For the features I have focused on, this includes:

  • functions for creating async contexts
  • functions for creating async functions from callback style functions (libuv)
  • functions for interleaving execution of multiple async functions (and aggregate the results).
  • Allow async functions to fully interact with non-async functions via passing arguments and returning results.
  • Allow async functions to run in a protected mode (like pcall) [WIP]
  • Overall, async functions appearing like normal functions as much as possible.

And excludes:

  • treating async functions like first class objects (tasks with methods)
  • job cancellation: this can be done by providing custom callbacks to libuv functions instead of wrapping them directly
  • chaining: just create a parent async function instead

Maybe we can revisit adding more advanced things in the future, but for now I think we should consider a minimum viable product that covers 95% of usecases with the least complexity.

lewis6991 avatar Jan 04 '23 10:01 lewis6991

any reason we wouldn't want that?

Not particularly but I suppose the question would also be would we want it? Because we're only creating task objects for the top level async contexts instead of all async functions, I'm not sure there is a use case for chaining tasks. Happy to be wrong though!

I don't know about @justinmk thoughts, but I would like any implementation included in core to be fairly lean and minimal.

Totally makes sense, for core I think just getting a single implementation that can be built upon by plugins would be the minimal goal so that there is no issue mixing async plugins. After that it's a lot more subjective and could be done in a plugin. There is some work going on to get common dependencies like plenary added to rockspec so that users won't even have to specify the dependencies (For example https://github.com/nvim-neotest/neotest/pull/185) which would make keeping most of this outside core even more logical.

  • treating async functions like first class objects (tasks with methods)
  • job cancellation: this can be done by providing custom callbacks to libuv functions instead of wrapping them directly

I would question these exclusions. The task objects don't represent an async function but an entire async context (i.e. a running coroutine). Implementing error handling and cancelling outside of the core implementation reliably sounds like it would be difficult without manually wrapping async functions everywhere or users coupling their code to the core implementation (by using the coroutines directly) but I could definitely be missing something obvious.

The task objects are only some fairly minor changes over your base implementation. To me at least, they don't add much complexity in comparison to the payoff of being able to cancel, handle errors and get the traceback (useful for hung tasks).

rcarriga avatar Jan 04 '23 10:01 rcarriga

I would question these exclusions. The task objects don't represent an async function but an entire async context (i.e. a running coroutine). Implementing error handling and cancelling outside of the core implementation reliably sounds like it would be difficult without manually wrapping async functions everywhere or users coupling their code to the core implementation (by using the coroutines directly) but I could definitely be missing something obvious.

My opinion is completely based on the fact I've never needed these features. If I did, my instinct would be to construct things so I can get access to the underlying uv handle so I can explicitly call close to kill any jobs which may or may not call an exit callback. Or just not use the async lib at all. At the end of the day, the main motivation here is just to eliminate "callback-hell", but sometimes it isn't the worst solution depending on what you're trying to do.

The task objects are only some fairly minor changes over your base implementation. To me at least, they don't add much complexity in comparison to the payoff of being able to cancel, handle errors and get the traceback (useful for hung tasks).

If that's the case, then we can definitely consider them. (EDIT: ref https://github.com/rcarriga/nvim-dap-ui/blob/feat/new-api/lua/dapui/async/tasks.lua)

lewis6991 avatar Jan 04 '23 11:01 lewis6991

Implementing error handling and cancelling outside of the core implementation reliably sounds like it would be difficult without manually wrapping async functions everywhere or users coupling their code to the core implementation

yeah, I see error handling and cancel as requirements. Doing that manually is unpleasant, it's one of the key features of a good concurrency interface.

justinmk avatar Jan 04 '23 11:01 justinmk