throw minijinja error

[chottolabs]

[chottolabs]

o1-mini is actually great... it just 1-shot this without me really trying with the prompt and it had some good suggestions too (although not a huge fan of how many tokens it spent more or less repeating the context back to me lol)

prompt:

<file>
QUERY: When minijinja-cli fails, it crashes instead of erroring

o1 response

When using `plenary.job` in Neovim to execute external commands like `minijinja-cli`, handling errors gracefully is crucial to prevent unexpected crashes. In your current implementation, invoking `error` inside the `on_stderr` callback can cause the entire Neovim process to terminate abruptly if `minijinja-cli` encounters an error. 

To address this issue, it's better to handle errors after the job has completed rather than within the callback. This approach allows you to capture the error messages and decide how to handle them without crashing Neovim. Below is a revised version of your `make_prompt_from_template` function that incorporates proper error handling:

```lua
local Path = require 'plenary.path'
local Scan = require 'plenary.scandir'
local Job = require 'plenary.job'
local api = vim.api

local M = {}

---Renders a prompt template using minijinja-cli and returns the rendered lines
---
---@param prompt_template_path Path absolute path to a jinja file
---@param prompt_args table typically PROMPT_ARGS_STATE which needs to be json encoded
---@return string rendered_prompt
---@return string? error_message Optional error message if rendering fails
function M.make_prompt_from_template(prompt_template_path, prompt_args)
  -- Check if minijinja-cli is executable
  if vim.fn.executable('minijinja-cli') ~= 1 then
    error("Can't find minijinja-cli, download it from https://github.com/mitsuhiko/minijinja or add it to $PATH", 2)
  end

  -- Verify that the template file exists
  if not prompt_template_path:exists() then
    error(string.format('Could not find template at %s', prompt_template_path:absolute()), 2)
  end

  -- Encode the prompt arguments as JSON
  local json_data = vim.json.encode(prompt_args)

  -- Initialize the job to run minijinja-cli
  local active_job = Job:new {
    command = 'minijinja-cli',
    args = {
      '-f', 'json',
      '--lstrip-blocks',
      '--trim-blocks',
      prompt_template_path:absolute(),
      '-',
    },
    -- Provide the JSON data to minijinja-cli via stdin
    writer = json_data,
    -- Collect stdout and stderr outputs
    stdout_buffered = true,
    stderr_buffered = true,
  }

  -- Run the job synchronously
  active_job:sync()

  -- Check the exit code to determine if the job was successful
  if active_job.code ~= 0 then
    -- Concatenate stderr messages for debugging
    local error_msg = table.concat(active_job:stderr_result(), '\n')
    -- Optionally, you can return the error message instead of throwing an error
    -- return nil, error_msg
    error("minijinja-cli failed:\n" .. error_msg, 2)
  end

  -- Concatenate the stdout result to form the rendered prompt
  return table.concat(active_job:result(), '\n')
end

---... [rest of your module remains unchanged] ...

return M
```

### Explanation of Changes

1. **Deferred Error Handling**:
   - **Removed `on_stderr` Callback**: Instead of handling errors immediately within the `on_stderr` callback by invoking `error`, we let the job run to completion and handle errors afterward.
   
2. **Buffered Output**:
   - **Enabled Buffering**: Setting `stdout_buffered` and `stderr_buffered` to `true` ensures that all stdout and stderr outputs are collected and can be accessed after the job completes.
   
3. **Exit Code Check**:
   - **Check `active_job.code`**: After the job completes, inspect `active_job.code` to determine if `minijinja-cli` executed successfully (`0` usually indicates success).
   - **Handle Non-zero Exit Codes**: If the exit code is not `0`, concatenate the stderr output and either throw an error or handle it as needed (e.g., return `nil` and the error message).
   
4. **Enhanced Error Messages**:
   - **Detailed Error Information**: By concatenating stderr messages, you provide more context about why `minijinja-cli` failed, which can be invaluable for debugging.

### Optional: Returning Error Messages Instead of Throwing

If you prefer not to throw errors and instead handle them gracefully within your plugin or configuration, you can modify the error handling part as follows:

```lua
if active_job.code ~= 0 then
  local error_msg = table.concat(active_job:stderr_result(), '\n')
  return nil, error_msg
end
```

With this change, `make_prompt_from_template` will return `nil` and the error message, allowing the caller to handle the error without crashing Neovim. Here's how you might handle it:

```lua
local rendered_prompt, err = M.make_prompt_from_template(template_path, args)
if not rendered_prompt then
  vim.notify("Error rendering template: " .. err, vim.log.levels.ERROR)
  return
end
-- Proceed with using rendered_prompt
```

### Additional Recommendations

- **Asynchronous Handling**: Consider running the job asynchronously using `:start` instead of `:sync` to prevent blocking the editor, especially for operations that might take longer.
  
- **Logging for Debugging**: Utilize `vim.notify` or other logging mechanisms to provide informative messages to the user without interrupting the workflow.

- **Input Validation**: Ensure that `prompt_args` contains valid data before encoding it to JSON to prevent unexpected errors from `minijinja-cli`.

By implementing these changes, you can ensure that errors from `minijinja-cli` are handled gracefully, enhancing the stability and user experience of your Neovim setup.

whereas deepseek wasn't really getting the memo at all

deepseek 2.5

The issue you're describing sounds like a problem with the `minijinja-cli` tool, which is not directly related to the Neovim configuration provided. However, since you're using Neovim, you might be running `minijinja-cli` from within Neovim or as part of a plugin that integrates with it.

### Possible Causes and Solutions

1. **Crash Instead of Error Handling**:
   - **Cause**: The `minijinja-cli` tool might be designed to crash (exit with a non-zero status) when it encounters an error, rather than gracefully handling the error and returning an error code.
   - **Solution**: Ensure that the tool is properly handling errors and returning appropriate exit codes. If the tool is part of a plugin or script, you might need to modify the script to handle errors more gracefully.

2. **Neovim Integration**:
   - **Cause**: If `minijinja-cli` is being invoked from within Neovim (e.g., via a plugin or custom script), the way Neovim handles external commands might be causing the crash to appear more severe.
   - **Solution**: Check the Neovim configuration or the plugin that invokes `minijinja-cli`. Ensure that it handles the command's output and exit status correctly. You might need to add error handling logic to catch and manage the crash.

3. **Environment Issues**:
   - **Cause**: There might be environmental issues (e.g., missing dependencies, incorrect paths) that cause `minijinja-cli` to crash.
   - **Solution**: Verify that all dependencies required by `minijinja-cli` are installed and accessible in your environment. Check the tool's documentation for any specific environment requirements.

### Example of Error Handling in Neovim

If `minijinja-cli` is being invoked from within Neovim, you might want to add error handling to manage the crash more gracefully. Here's an example of how you might do this in Lua:

```lua
local function run_minijinja_cli()
  local command = "minijinja-cli"
  local args = { "--some-option", "some-value" }

  local handle = io.popen(table.concat({command, table.unpack(args)}, " "))
  if not handle then
    print("Failed to run minijinja-cli")
    return
  end

  local result = handle:read("*a")
  handle:close()

  if result:match("error") then
    print("minijinja-cli encountered an error: " .. result)
  else
    print("minijinja-cli executed successfully: " .. result)
  end
end

-- Example usage
vim.api.nvim_create_user_command('RunMiniJinjaCli', run_minijinja_cli, {})
```

### Conclusion

The issue with `minijinja-cli` crashing instead of erroring is likely related to how the tool is designed to handle errors. To address this, you should:

1. Ensure that `minijinja-cli` is properly handling errors and returning appropriate exit codes.
2. If `minijinja-cli` is invoked from within Neovim, add error handling to manage the crash more gracefully.
3. Verify that all dependencies and environment settings are correct.

If you provide more details about how `minijinja-cli` is being used within Neovim, I can offer more specific advice.

[dceluis]

Hi, @chottolabs . I think on current plenary versions its not stderr_buffered but enable_recording.

It might be worth double checking because plenary seems to raise trying to access stderr_result unless the above param is set to true:

https://github.com/nvim-lua/plenary.nvim/blob/ec289423a1693aeae6cd0d503bac2856af74edaa/lua/plenary/job.lua#L464C3-L467C4

[chottolabs]

hmm.. you're right - it's even weirder that it seems to be working like usual on my side tho

[chottolabs]

is this even intended? enable_recording =/= enabled_recording typo? 🤨

[chottolabs]

yeah so i guess enable_recording defaults to true if nil so it never triggered that assert

[dceluis]

yeah so i guess enable_recording defaults to true if nil so it never triggered that assert

Ohhhh I see. I only knew of enable_recording because of my work on the curl error handling. I didn't realize it was on by default. Nothing to fix then, sorry!

[chottolabs]

honestly good to make it explicit tho, I will add it in for consistency