Notifications from one LSP break another

[vasigorc]

I am using protols LSP together with metals within the same project: working on *.proto and *.scala files next to each other. protols relies on async-lsp's Router (link). When I have two file type buffers opened in nvim protols crashes with this message:

[ERROR][2025-04-08 16:36:17] .../vim/lsp/rpc.lua:770    "rpc"    "protols"    "stderr"    "\nthread 'main' panicked at <REDACTED>.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/protols-0.11.5/src/main.rs:94:46:\ncalled `Result::unwrap()` on an `Err` value: Routing(\"Unhandled notification: metals/didFocusTextDocument\")\nnote: run with `RUST_BACKTRACE=1` environment variable to display a backtrace\n"

This lands into this handler in async-lsp. Copying it below for eventual line changes on GitHub:

            unhandled_notif: Box::new(|_, notif| {
                if notif.method.starts_with("$/") {
                    ControlFlow::Continue(())
                } else {
                    ControlFlow::Break(Err(crate::Error::Routing(format!(
                        "Unhandled notification: {}",
                        notif.method,
                    ))))
                }
            }),

metals/didFocusTextDocument is valid albeit not a generic LSP notification, which is why it is not found in lsp-types (link) which is what async-lsp relies on to identify notifications as being handled or no:

    pub fn notification<N: Notification>(
        &mut self,
        handler: impl Fn(&mut St, N::Params) -> ControlFlow<Result<()>> + Send + 'static,
    ) -> &mut Self {
        self.notif_handlers.insert(
            N::METHOD,
            Box::new(
                move |state, notif| match serde_json::from_value::<N::Params>(notif.params) {
                    Ok(params) => handler(state, params),
                    Err(err) => ControlFlow::Break(Err(err.into())),
                },
            ),
        );
        self
    }

I am raising this issue with async-lsp and not with protols because there's not much thing that they can do in the case of an error, attempts to catch errors and restart server would be kind of hackish, in my opinion. Instead I think that async-lsp could make the handling more permissive. The current design forces tight coupling between different LSP implementations and limits interoperability

I realize that you don't want to be too permissive neither and that you would like to be backward compatible. Would it be possible to add a reserved setting (or reserved settings) to:

• ignore unknown notifications (call ControlFlow::Continue)
• if that is too wild, maybe add also a allow_list setting to specifically mention which notifications are ok to be ignored?

[oxalica]

I am raising this issue with async-lsp and not with protols because there's not much thing that they can do in the case of an error,

Like you already mentioned, there is also not much thing that we can do for this. Notifications, unlike requests, are incorrect to be ignored freely unless they has a $/ prefix (which are already ignored by us). It's also mentioned in the LSP spec:

If a server or client receives notifications starting with ‘$/’ it is free to ignore the notification. If a server or client receives a request starting with ‘$/’ it must error the request with error code MethodNotFound (e.g. -32601).

The main issue of ignoring unknown notification is that, notifications can change how future requests and/or notifications are interpreted, eg. textDocument/didOpen changes the meaning of positions in all future requests and notifications). Ignoring any of them may cause a permanent state de-synchronization which is impossible to recover (consider a delta change).

metals/didFocusTextDocument is valid albeit not a generic LSP notification

For non-standard/extension methods, the standard way is to negotiate them ahead in initialize, and any notification unrecognized by the peer should be suppressed. There is even a dedicated experimental field in capabilities for this purpose. See how rust-analyzer does this in: https://github.com/rust-lang/rust-analyzer/blob/bee999863b5e54a05ee19fa81df8ceca5bef200e/docs/book/src/contributing/lsp-extensions.md

• if that is too wild, maybe add also a allow_list setting to specifically mention which notifications are ok to be ignored?

You can already do this by manually add no-op handlers for notifications you want to ignore deliberately. See this in example. For foreign methods not in lsp-types, you can define new types and impl lsp_types::notification::Notification to be able do use them.

[vasigorc]

Thank you for the quick follow-up @oxalica !

You can already do this by manually add no-op handlers for notifications you want to ignore deliberately.

Just confirming that you are referring to a programmatic way to ignore notifications? In this case protols could add this extension, but it would non sustainable because metals is just one case for Scala, but then any new non-standard methods could break it: Go, Rust, Python, etc. I was referring to a configuration/setting (a dynamic) way to control this. A way which would not break any flow unless purposefully mis-configured.

It's also mentioned in the LSP spec:

If a server or client receives notifications starting with ‘$/’ it is free to ignore the notification.

Yes, I've read the spec before filing the issue, as I wasn't sure what are the recommendations. I think that the API is formal about $/ prefix, but it is silent on other cases. Requests are not the same thing, since notifications are a fire and forget approach.

you can define new types and impl lsp_types::notification::Notification

Filing an issue on the lsp_types was another option that I considered, but looking at their code, they're dealing with generic messages only. Putting all custom notifications there would soon get unwieldy, and it would be impossible to enforce for different LSP providers.