- Notifications
You must be signed in to change notification settings - Fork 326
Log to stderr as well as the log file, on non-Darwin platforms#2155
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
base:main
Are you sure you want to change the base?
Conversation
ahoppen left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do log to stderr until we set up the global log file handlers using setUpGlobalLogFileHandler. We should also log to stderr from there. With the current implementation, we would log every message twice if setting up the global log file handlers failed (once from the log handler and then once again from your fputs call).
Wilfred commented May 16, 2025
Gotcha, presumably something like this is better? |
bnbarham commented May 16, 2025
@swift-ci please test |
Uh oh!
There was an error while loading. Please reload this page.
bnbarham commented May 19, 2025
@swift-ci please test |
bnbarham commented May 19, 2025 • edited
Loading Uh oh!
There was an error while loading. Please reload this page.
edited
Uh oh!
There was an error while loading. Please reload this page.
I'm not sure this is actually possible with our current implementation. All the calls are direct to OSLog and That also makes me wonder if we want this in general, since it means we have very different output to LSP clients between the platforms. EDIT: Ah, it seems like various editors also treat stderr output as actual errors (eg. neovim logs stderr output with the format "[ERROR][timestamp] ..."). So it may indeed be best to avoid doing this generally. I've opened #2161 which seems like a better direction to go here. |
ahoppen commented Aug 12, 2025
@swift-ci Please test macOS |
ahoppen commented Aug 12, 2025
@swift-ci Please test Windows |
This is consistent with other LSP servers, and makes debugging sourcekit-lsp easier.
ahoppen left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for missing this. I just found the PR again, looks good to me, I just fixed a trailing whitespace issue on your branch.
ahoppen commented Aug 12, 2025
@swift-ci Please test |
ahoppen commented Aug 12, 2025
@swift-ci Please test Windows |
bnbarham left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to re-iterate the edit I made in the previous comment - I'm not sure this is a path we want to go down due to editors treating stderr output as errors. Neovim is one example here (it logs stderr output with the format "[ERROR][timestamp] ..."), but I imagine there's probably others.
I'd prefer something like #2161, but I'd also be open to adding this behind a flag to make things easier to debug in the meantime.
Based on @bnbarham’s comment, I agree that this isn’t the right direction to go.
Wilfred commented Aug 13, 2025
@bnbarham I don't think that's correct. All the other LSP implementations I've looked at (e.g rust-analyzer, clangd) write logs to stderr without problems. For example, if I create a hello.cpp file and start clangd, I get the following output to stderr: I mostly use Emacs and VS Code for my LSP needs, but I don't think that any editor treats stderr output as error. Note that LSP doesn't say anything about stderr. You don't even need to use stdin/stdout -- the spec supports stdio, pipes, sockets and node-ipc. If an LSP server wants to show an error to a user, there are You can see in the linked thread that a clangd maintainer is asking for more options for stderr logging, but gets a ton of value from having these logs! I certainly find stderr logs super useful when investigating LSP issues, and stderr seems like a very common convention. |
bnbarham commented Aug 13, 2025
Yeah, it's not a pop-up, just:
Which seems like it could cause some confusion to someone looking at those logs. Given we already log to a file, my preference would be to |
This is consistent with other LSP servers, and makes debugging sourcekit-lsp easier.
This was initially discussed in #2116. Let me know if you think we should log to stderr on macOS too.