Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bug: tasks dispose inconsistently #315

Closed
Zeioth opened this issue Jun 10, 2024 · 11 comments
Closed

bug: tasks dispose inconsistently #315

Zeioth opened this issue Jun 10, 2024 · 11 comments
Labels
bug Something isn't working

Comments

@Zeioth
Copy link

Zeioth commented Jun 10, 2024

Neovim version (nvim -v)

0.10.0

Operating system/version

arch linux

Describe the bug

Tasks do not dispose consistently:

  • Some times tasks are not disposed.
  • Orchestrators are not disposed anymore at all.

screenshot_2024-06-10_12-45-41_027391651

What is the severity of this bug?

minor (annoyance)

Steps To Reproduce

  1. on unix, and having compiler.nvim installed run :luafile ~/.local/share/nvim/lazy/compiler.nvim/tests/tests.lua. This will spawn ~100 tasks over 1 minute (don't worry if some fail due to not having the required dependency installed).
  2. wait 300s until tasks start to dispose.

You will see they don't dispose correctly like they used to.

Expected Behavior

All tasks and orchestrators dispose after 300s.

Minimal example file

No response

Minimal init.lua

No response

Additional context

Hopefully I don't have to bother you much more after this one. But it seems to be a bug.

@Zeioth Zeioth added the bug Something isn't working label Jun 10, 2024
@stevearc
Copy link
Owner

This is likely due to this change 9162631, and is mostly intentional.

The default behavior of the on_complete_dispose component did not change, but I did change the default configuration. In brief, it won't dispose until the output buffer has been viewed. This is something that can be configured by end users by customizing the components in the default section (removing the require_view section). If you want to enforce it more directly in your plugin you can specify the on_complete_dispose component and put it before the default group so that its settings take priority. I'll open a PR as example

@Zeioth
Copy link
Author

Zeioth commented Jun 10, 2024

@stevearc The tasks dispose now indeed, but the orchestrators are not disposing anymore like they used to.

Before disposing trigger

screenshot_2024-06-11_01-23-40_988485089

After disposing trigger

screenshot_2024-06-11_01-18-27_565361010

@Zeioth
Copy link
Author

Zeioth commented Jun 10, 2024

Do I also have to add the component to the orchestrator? If so that's gonna add a big chunk of boilerplate, I'm gonna try tomorrow.

@Zeioth
Copy link
Author

Zeioth commented Jun 11, 2024

Yeah indeed... that was the issue. if this is the case, compiler.nvim is gonna have to tell its users to add this to their config

component_aliases = {
  -- Components included in default will apply to all tasks
  default = {
    { "display_duration", detail_level = 2 },
    "on_output_summarize",
    "on_exit_set_status",
    "on_complete_notify",  -- comment this to disable notifications
    { "on_complete_dispose", timeout=300 }
  },
},
  • Because otherwise users wouldn't have control over the timeout.

@Zeioth
Copy link
Author

Zeioth commented Jun 11, 2024

This can be closed now but there is something I've noticed if you are interested:

  • With your current default overseer config: After the timeout trigger, if a task hasn't been seen by the user, it will never be disposed. Maybe the timeout should refresh after the user focus the task.
  • Also, because users have no reason to click the orchestrator, it normally won't be disposed.

I would advice against this default, and going back to the old on_complete_dispose by default. But this is just my opinion.

@stevearc
Copy link
Owner

After the timeout trigger, if a task hasn't been seen by the user, it will never be disposed.

This shouldn't be the case. We start a repeating timer for exactly this reason, and I tested this a few times with a short timeout. Once the user views the task, it gets disposed when the timer next runs

-- Start a repeating timer because the dispose could fail with a
-- temporary reason (e.g. the task buffer is open, or the action menu is
-- displayed for the task)
self.timer:start(
1000 * opts.timeout,
1000 * opts.timeout,
vim.schedule_wrap(function()
task:dispose()
end)
)

If you have a reliable repro, could you file it as a bug?

Also, because users have no reason to click the orchestrator, it normally won't be disposed.

I think this is fine. The pitfall I wanted to avoid is when a user starts a task that takes a while (one or more minutes), gets busy doing something else, then when they think to check back up on it, it has already been disposed and it's impossible to check the output or even see the status. That's a pretty bad experience that I've personally encountered several times. On the other side, if you have a task or set of tasks that you don't care about preserving the output of, there are a number of ways to adjust the components to get that behavior.

@Zeioth
Copy link
Author

Zeioth commented Jun 13, 2024

@stevearc I've recorded a video demo: https://www.youtube.com/watch?v=BCgMkNAN4Gs

@stevearc
Copy link
Owner

I've added some additional debug logging to the on_complete_dispose component that should help us visualize what's going on under the hood. You can set the debug log level using a configuration like this:
https://github.com/stevearc/dotfiles/blob/e575933206ccdcb0037c66aad50fab9cfb53d49a/.config/nvim/lua/plugins/overseer.lua#L37-L47

Another useful tool in debugging is pulling up the "task edit" window by pressing <C-e> when your cursor is on top of a task. That will show you what components are on the task and what their parameters are. I used this to confirm the parameters of the on_complete_dispose component when looking at a task.

I am not sure exactly what is happening in your video. I tried to replicate it (using a shorter timeout, because I do not have the patience you do), and the tasks seem to dispose correctly. One thing that I did have to change was the orchestrator tasks were being created with the default components; I had to also set them to use default_extended in order to pick up the new timeout value for on_complete_dispose. Using this minimal change on stevearc/compiler.nvim@bb6ef38, they seem to dispose properly.

If you're still seeing dispose issues, try to get me a minimal init.lua config that will reproduce the issue. It's possible there's some difference in our configs that accounts for the difference.

@stevearc stevearc reopened this Jun 14, 2024
@stevearc stevearc added the question Further information is requested label Jun 14, 2024
@Zeioth
Copy link
Author

Zeioth commented Jun 25, 2024

Hi @stevearc sorry it took me a bit to find time to test in a meaningful way. I've been testing with this minimal_file and it's weird:

What it works

The component

overseer.register_alias("default_extended", {
  { "on_complete_dispose", require_view={}, timeout=10}, -- dispose after 10 secs if the task has been seen.
  "default",
  "open_output", -- focus last executed task
})
Correctly dispose all tasks (they are guaranteed to have been seen because of `open_output`)

What doesn't work

The same component but without { "on_complete_dispose", require_view={}, timeout=10}.

After the default 300 seconds, tasks are disposed inconsistently, or not at all.

What doesn't work (neither)

The same component but without open_output

overseer.register_alias("default_extended", {
  { "on_complete_dispose", require_view={}, timeout=10},
  "default",
})

And then manually executing :OverseerToggle should ensure tasks haven't been seen. But all tasks are disposed after 10 seconds anyway.

More info

This is very weird because according to the source code of the on_complete_dispose component, we are doing the same thing as the default, the only thing changing is the timeout (from 300s to 10s)

@github-actions github-actions bot removed the question Further information is requested label Jun 25, 2024
@stevearc
Copy link
Owner

Okay let's take this case by case

Case 1

{
  { "on_complete_dispose", require_view={}, timeout=10},
  "default",
  "open_output",
}

This disposes all the tasks after 10 seconds. Great, this is the expected behavior.

Case 2

{
  "default",
  "open_output",
}

This doesn't dispose all the tasks. This is also expected. The reason is that the default group adds a component to the task that requires the user to view the output buffer for successful or failed tasks.

{ "on_complete_dispose", require_view = { "SUCCESS", "FAILURE" } },

Each task has open_output, sure, but this component enforces that the output has been viewed after the task has completed. These buffers have not been viewed after completing.

Case 3

{
  { "on_complete_dispose", require_view={}, timeout=10},
  "default",
}

This disposes all the tasks after 10 seconds. This is still expected. By passing in require_view={} you are saying "don't require that the task output be viewed before you can dispose this task". As a result, the 10 second timer starts immediately after the task completes, and then they are disposed.

Solution

I think what you are probably looking for is

{
  -- Because of the default values this is equivalent to
  -- { "on_complete_dispose", require_view={} }
  "on_complete_dispose",
  "default",
  "open_output",
}

This uses the default 300 second timer, and doesn't require that the output buffers are viewed before they can be disposed. If you are still experiencing unexpected behavior, try increasing the log level and inspecting the logs. That should help you figure out what's going on.

You can set the log level using:

require("overseer").setup({
  log = {
    {
      type = "echo",
      level = vim.log.levels.WARN,
    },
    {
      type = "file",
      filename = "overseer.log",
      level = vim.log.levels.DEBUG,
    },
  },
})

@stevearc stevearc added the question Further information is requested label Jun 26, 2024
@Zeioth
Copy link
Author

Zeioth commented Jul 25, 2024

Closing this for now because I'm not gonna have time to test it for a month or so and I don't want to pollute the bug tracker. I'll re-open in the future if I'm able to provide more detail.

@Zeioth Zeioth closed this as completed Jul 25, 2024
@github-actions github-actions bot removed the question Further information is requested label Jul 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants