Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inifinite recursion in logger #447

Open
kokobd opened this issue Aug 12, 2022 · 6 comments
Open

Inifinite recursion in logger #447

kokobd opened this issue Aug 12, 2022 · 6 comments

Comments

@kokobd
Copy link
Collaborator

kokobd commented Aug 12, 2022

This was originally found in haskell/haskell-language-server#3072

In lsp, there is a debug log, printing response body:

-- We need to make sure we only send over the content of the message,
-- and no other tags/wrapper stuff
let str = J.encode msg
let out = BSL.concat
[ TL.encodeUtf8 $ TL.pack $ "Content-Length: " ++ show (BSL.length str)
, BSL.fromStrict _TWO_CRLF
, str ]
clientOut out
logger <& SendMsg (TL.decodeUtf8 str) `WithSeverity` Debug

And in HLS, our logger sends a copy of log to LSP client:
https://github.com/haskell/haskell-language-server/blob/347a7187f20242540926b9927c59a45b18798c67/exe/Main.hs#L72-L82

Then it goes into infinite recursion. A lot of logs like below are produced, and finally OOM on a machine with 64GB RAM.

graph

@michaelpj
Copy link
Collaborator

The problems are:

  1. This log is useful, it's handy to be able to see the actual response body.
  2. We would like to see this log in the HLS server log file.
  3. It would be somewhat nice to see this in the client log file.
  4. We sometimes want to see debug logs, and we don't have a severity below debug.

I think my best idea is to specifically filter out the window/showMessage and window/logMessage messages and not log them. That makes this log marginally less useful, but also they're super noisy and probably not that interesting.

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this issue Oct 31, 2023
2.2.0.0
* Many changes relating to client configuration
  - lsp now sends workspace/configuration requests in response to
    intialized and workspace/didChangeConfiguration requests. It still
    attempts to parse configuration from intializationOptions and
    workspace/didChangeConfiguration as a fallback.
  - Servers must provide a configuration section for use in
    workspace/configuration.
  - parseConfig will now be called on the object corresponding to the
    configuration section, not the whole object.
  - New callback for when configuration changes, to allow servers to react.
* The logging of messages sent by the protocol has been disabled, as this
  can prove troublesome for servers that log these to the client:
  haskell/lsp#447

2.1.0.0
* Fix handling of optional methods.
* staticHandlers now takes the client capabilities as an argument. These
  are static across the lifecycle of the server, so this allows a server to
  decide at construction e.g. whether to provide handlers for resolve
  methods depending on whether the client supports it.

2.0.0.0
* Support lsp-types-2.0.0.0.
@soulomoon
Copy link
Contributor

soulomoon commented Apr 9, 2024

What is the state of it. 🤔
It is safe to enable the log now?

@michaelpj
Copy link
Collaborator

No, I never really figured out a good solution to this. We need to do one of two things to break the recursion:

  1. Not log the problematic window/logMessage messages
  2. Not send window/logMessage messages for the problematic logs

@michaelpj
Copy link
Collaborator

I think I'm leaning towards the latter now, since that means that that lsp can continue to log things in full, and it's just the bit that sends logs to the client that needs to behave differently.

@soulomoon
Copy link
Contributor

does it mean we need to do it in hls?

@michaelpj
Copy link
Collaborator

We also export a client logger from lsp. I'm not sure if HLS has its own, I'd need to look.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants