internal: convert unwrap to except and add a debug log

[Sarrus1]

Remove an unsafe unwrap that can cause crashes if the value is a SendError.

This is my first PR on this repo, please let me know if there is anything I can improve or details I can provide.

[Veykril]

I believe this is intentional. The receiver for the given sender dropping before the server has properly exited smells like a bug by the consumer of lsp-server. The receiver shouldn't be dropped before.

[Sarrus1]

It did seem intentional yes.

However, this has happened on multiple stable versions of VSCode as a receiver. I assume they implemented the protocol correctly.

Wouldn't it be better if this did not crash the server and logged an error instead ? Missing a notification does not seem fatal.

Is it possible for the receiver to be overwhelmed by the amount of notifications sent from the server? I imagine this would cause it to skip notifications and cause this error as well.

[Veykril]

Wouldn't it be better if this did not crash the server and logged an error instead ? Missing a notification does not seem fatal.

Well, if the unwrap panics then that means the receiver has been dropped at which point no more messages can be sent over that channel anyways (no matter whether we panic or ignore the result).

Is it possible for the receiver to be overwhelmed by the amount of notifications sent from the server? I imagine this would cause it to skip notifications and cause this error as well.

The channel is created with a capacity of 0 meaning that any send operation will block until the receiver has received it.

[Sarrus1]

The channel is created with a capacity of 0 meaning that any send operation will block until the receiver has received it.

Ok ok so overwhelming the receiver is off the table.

no more messages can be sent over that channel anyways

Ah right so nothing can be done from this point, I see.

Thank you very much for answering my questions and looking into the PR!
I will try to better repro the issue to isolate the cause and report here if I find anything 👍

[lnicola]

We could change it to an expect with an explanation of what happened, I suppose.

[Veykril]

Can you remove the merge commit from the history here? Or just squash the commits into one

[Sarrus1]

Sorry about that, should be all good now 👍

[Veykril]

Thanks
@bors r+

[bors]

📌 Commit 02d5c0a has been approved by Veykril

It is now in the queue for this repository.

[bors]

⌛ Testing commit 02d5c0a with merge ed4e28b...

[bors]

☀️ Test successful - checks-actions
Approved by: Veykril
Pushing ed4e28b to master...