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

Follow up on #938 #1080

Closed
tlienart opened this issue Jul 17, 2023 · 4 comments
Closed

Follow up on #938 #1080

tlienart opened this issue Jul 17, 2023 · 4 comments

Comments

@tlienart
Copy link
Contributor

tlienart commented Jul 17, 2023

Sorry I tried to comment on #938 but it looks like I wasn't able to re-open the issue. The problem still occurs for users of LiveServer.jl getting these confusing error messages about broken pipes which shouldn't actually concern them.

I tried silencing all logging by wrapping the main LiveServer loop (effectively happening around a HTTP.listen!) in a TestLogger but that impacted the logging of other non-HTTP stuff that happens during the loop so had to roll it back.

  1. in Allow hiding handler error #938 commits seem to suggest that the issue was fixed even though I still see these broken pipe messages; is there a step I need to take here that I might have missed?
  2. depending on (1) could some logic be added to forcibly filter out these @logmsgv handler error messages? e.g. via a keyword argument passed to listen! -> listenloop -> handle_connection ?

(happy to open a PR for (2) btw)

thanks in advance

@quinnj
Copy link
Member

quinnj commented Jul 17, 2023

Do you have a simple script that reproduces the error? I can take a look at this in the next day or two.

@tlienart
Copy link
Contributor Author

tlienart commented Jul 17, 2023

No, this error message is difficult to recreate (note that it's not an error in the sense that it doesn't affect the usability of LiveServer in a visible way, in particular it doesn't interrupt the server (and it shouldn't)).
Best I can tell you is that LiveServer uses WebSockets (possibly in a non-ideal way) to connect the server to browser tabs and update those upon changes. This code:

https://github.com/tlienart/LiveServer.jl/blob/282beb53377da859f5c260026dcc19305e250c5b/src/server.jl#L51-L71

When interrupting and re-starting the server with many tabs open and re-triggering updates, we sometimes get the message triggered (I'd say the frequency is around 5-10%).

The only other possible crumb of information is that I saw a message of yours here also in reply to someone asking about broken pipes where you were suggesting that simultaneous requests using @async may cause troubles.

I'm aware that this is very little information to actually debug this, this is why I'm asking if we could add an ignore_handler_error to listen! which would be passed to listenloop and finally to handle_connection to make the @logmsgv line like this:

ignore_handler_error || @logmsgv 1 level "handle_connection handler error" exception=(e, stacktrace(catch_backtrace()))

thanks

@tlienart
Copy link
Contributor Author

tlienart commented Jul 19, 2023

even simpler would be to pass verbose to handle_connection(...) after access_log and, as used elsewhere in the code base:

verbose >= 0 && @logmsgv ...

users who wish to hide this can then just pass verbose = -1 to the listen! call.

@tlienart
Copy link
Contributor Author

tlienart commented Jul 20, 2023

Actually I just discovered that messages like these can be filtered out externally using LoggingExtras:

using LoggingExtras
with_logger(EarlyFilteredLogger(nt -> nt._module !== HTTP.Servers, global_logger())) do 
  # http listening logic + websocket update stuff
end

this seems to work fine for my use case and given I can't really give an MWE which, understandably, doesn't help the debugging, I'll close this issue.

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

2 participants