plug.kak icon indicating copy to clipboard operation
plug.kak copied to clipboard

No indication when the `do` block fails

Open Superty opened this issue 3 years ago • 3 comments

Consider installation of kak-lsp:

plug "ul/kak-lsp" do %{
    cargo build --release --locked
    cargo install --force --path .
}

In my case no toolchain was installed for cargo, thus it failed with return code 1. However, no indication of this was shown by plug.kak and I had to debug this by executing the commands manually.

Superty avatar May 13 '22 20:05 Superty

I was wondering the same thing! In my case the cargo build was failing because I had an old rust version, which meant I was using a previously installed version of a plugin and thus caused some weird errors. It took my quite some time to figure out what the issue was, and having some notification that the "do" block of that plugin failed might have spared me quite some debugging time!

0phoff avatar May 27 '22 16:05 0phoff

I'm not sure how to properly fix this, if anyone has ideas, I'll gladly accept a pull request.

andreyorst avatar May 28 '22 07:05 andreyorst

@andreyorst, I went through your code a bit in order to understand how it all works, and noticed you copy the contents of the do blocks into their own seperate shell script files, which you then run. I also noticed you already look at the return code of those code-blocks, and if we thus use exit <non-zero>, the update screen will already notify us that an error happened.

A relatively simple trick would be to prepend the hook_file with the following command (more info):

# Whenever a subcommand fails, exit the script with same errorcode
set -e

# Alternatively, also raise error if pipeline failed
set -eo pipefail

DISCLAIMER: The -o pipefail option seems to be bash specific and does not work in basic sh. It would be nice to find a way to get this working in sh as well, as it is more robust at catching errors.


One thing to note here is that this does not allow commands to silently fail anymore. As an example, I used to install kak-lsp and also install some language servers in the same do block.

plug "kak-lsp/kak-lsp" do %{
    cargo install --force --locked --path .

    # Install language servers
    npm install -g typescript-language-server typescript
    pip install -U 'python-lsp-server[rope,flake8,yapf,pydocstyle]'
}

The nice thing was that if eg. node/npm was not installed, installation of the typescript language server would simply fail and the code would go to the next line. However, with set -e, this script will crash at the npm install and thus not install python-lsp-server. However, this is quite easily fixable by either checking if the npm command exists or by adding || true at the end of the command. Both have slightly different effects as the npm command could still fail even if it exists (eg. no internet connection). For this specific case, I would thus probably prefer || true.

plug "kak-lsp/kak-lsp" do %{
    set -e

    cargo install --force --locked --path .

    # Option 1 : check if `npm` exists
    command -v npm >/dev/null 2>&1 && npm install -g typescript-language-server typescript

    # Option 2 : Simply ignore the return code
    npm install -g typescript-language-server typescript || true

    # This will always be reached even if npm failed
    pip install -U 'python-lsp-server[rope,flake8,yapf,pydocstyle]'
}

Finally, these set commands can always be reverted as well:

 # fail fast if command has non-zero error code
set -e 
cargo install --force --locked --path .

# Optional installs, allow to fail
set +e
npm install -g typescript-language-server typescript
pip install -U 'python-lsp-server[rope,flake8,yapf,pydocstyle]'

0phoff avatar May 30 '22 13:05 0phoff

I've made set -e added automatically to all do blocks. @0phoff thanks for the suggestion!

andreyorst avatar Sep 08 '22 18:09 andreyorst