daa84 / neovim-lib

Rust library for Neovim clients
GNU Lesser General Public License v3.0
192 stars 26 forks source link

Executing "echo" as a command but not pressing enter in Neovim causes neovim_lib to panic #19

Closed Olical closed 6 years ago

Olical commented 6 years ago

Hi there, I'm executing echo "foo" as a command which shows up in Neovim fine.

pub fn command(&mut self, cmd: &str) {
    if let Err(msg) = self.nvim.command(cmd) {
        error!("Command failed ({}): {}", cmd, msg);
    }
}

The problem is that if I don't clear the "press enter to continue" message in Neovim within 5 seconds (I think that's the timeout?) when I eventually press enter neovim_lib will panic. If this is intended (because Neovim doesn't respond to the RPC call while it's waiting for the user to press enter) is there a way I can send a command without waiting for a response?

In my logs I see this after 5 seconds:

13:27:11 [ERROR] Command failed (echo "[foo] 127.0.0.1:5555 for files matching 'clj$'
[bar] 127.0.0.1:5555 for files matching 'cljc?$'"): Unknown error type: Wait timeout (nvim_command)

And then I see this after pressing enter in Neovim since I pipe stderr to echoerr:

conjure: 13 stderr thread '<unnamed>' panicked at 'called `Result::unwrap()` on an `Err` value: "SendError(..)"', libcore/result.rs:1009:5
conjure: 13 stderr note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at libst
d/sys/unix/backtrace/tracing/gcc_s.rs:49
   1: std::sys_common::backtrace::print
             at libstd/sys_common/backtrace.rs:71
             at libstd/sys_common/backtrace.rs:59
   2: std::panicking::defau
lt_hook::{{closure}}
             at libstd/panicking.rs:211
   3: std::panicking::default_hook
             at libstd/panicking.rs:227
   4: std::panicking::rust_panic_with_hook
             at libstd/panic
king.rs:477
   5: std::panicking::continue_panic_fmt
             at libstd/panicking.rs:391
   6: rust_begin_unwind
             at libstd/panicking.rs:326
   7: core::panicking::panic_fmt
             at
libcore/panicking.rs:77
   8: core::result::unwrap_failed
   9: neovim_lib::rpc::client::Sender::send

The backtrace was really weirdly formatted because of Neovim but I'd tried to recover it as best I can.

Thanks a lot! The library has been extremely helpful for me so far!

Olical commented 6 years ago

Now I'm no Rust expert, this is my first project since reading The Book, but could it be because this error is being unwrapped here? https://github.com/daa84/neovim-lib/blob/f43acadb83db23f57ae3bbef4821741c1fa8019f/src/rpc/client.rs#L24

KillTheMule commented 6 years ago

"Press enter" prompts are ugly. But if you're sending a message to neovim, won't the user press Enter? You could give nvim_feedkeys or nvim_input a shot, but you'd have to be sure there actually is a prompt...

Seems like a shortcoming of nvim for me. I'd avoid things that produce a prompt, but you might want to open an issue at the neovim repo to ask for advice or maybe a fix. Better yet, do it on the gitter channel.

Olical commented 6 years ago

Hmm, maybe. I'm not really causing the prompt on purpose though, I'm just calling the echo command. Sure the user will probably press enter at some point, but if they press it after five seconds my program panics which I think is a little over the top.

I was half hoping there was another way to execute a command in a "fire and forget" way so it didn't wait for Neovim to acknowledge it. I guess Neovim locks up after an echo waiting for the user to press enter so it doesn't respond to the RPC client until that's gone.

Olical commented 6 years ago

Side note, I want to use echo and have it block, I'm displaying a list of connections for the user to read. I want it to stay on screen. I just don't want my program to panic because Neovim doesn't respond while that prompt is on screen. If I can disable the timeout for this call it'd solve it too.

KillTheMule commented 6 years ago

Yeah, as I said, I think neovim not responding during a press enter prompt seems wrong. Imho though echo is woefully inadequate to communicate with your user, I suggest finding something else.

KillTheMule commented 6 years ago

I've asked on gitter, and received the following comment from @bfredl:

we should probably just change the code to allow it. vim 8 does it, and AFAIK it hasn't caused any specific problem

So maybe it will work in the future. I'd set out to find better ways of communication anyways :)

(e) Btw, did you see https://docs.rs/neovim-lib/0.5.4/neovim_lib/session/struct.Session.html#method.set_infinity_timeout?

bfredl commented 6 years ago

I was half hoping there was another way to execute a command in a "fire and forget" way so it didn't wait for Neovim to acknowledge it.

@Olical There is, async calls, as defined by the NeovimApiAsync trait in these bindings.

Olical commented 6 years ago

Yeah, as I said, I think neovim not responding during a press enter prompt seems wrong. Imho though echo is woefully inadequate to communicate with your user, I suggest finding something else.

Of course, echo is not my main source of communication, it's on the side of a dedicated buffer for meta information about connections and listing connections briefly. Maybe I have to move all of this into the buffer, I'd just prefer not to I guess?

I didn't spot the infinity timeout, that could help in my specific case too, thanks!

I'll also give the async call a go again, although I did try that and I'm pretty sure I saw the exact same issue? I think there's still a five second timeout and something inside the library tries to unwrap an Err after those five seconds have expired?

Olical commented 6 years ago

Okay the async call with the current git version of the library works for me with echo. I think my PR still has some helpful renaming and exposing in it though. I also think warning instead of panicking when it times out makes more sense.

daa84 commented 6 years ago

@Olical

Olical commented 6 years ago

Closing now since using the async API should suit my needs for the time being. Hopefully this issue will be useful for anyone who hits the same issue in the future. Thanks for your time everyone, sorry if I approached it the wrong way!