Better logging for named pipe #11144
Conversation
| throw; | ||
| } | ||
|
|
||
| Logger.LogError("Failed to read ProjectInfoAction from stream"); |
There was a problem hiding this comment.
Should this log the exception? I have no idea why this would fail, and need to be re-tried, but seems like the logs could help tell us.
There was a problem hiding this comment.
tbh it could be right now the only way it can throw is by getting an Assumes.Unreachable which isn't very actionable.
There was a problem hiding this comment.
In a perfect world, Assumed.Unreachable should never be hit. It should just be used to make the compiler happy. If it's being hit, then expectations are incorrect.
There was a problem hiding this comment.
I agree with that for sure, but trying to figure out why the expectations are incorrect is currently the hard part.
Because by reading data it moves the stream forward. That said I have a suspicion that it won't really recover but did some defensive programming around it. The updated server side is more important, but with VS Code this will flood logs pretty quickly if we don't stop. Maybe I should have just exited on fail... Or have some better retry logic to reconnect or something... but for now this is mostly to help figure out why Mac integration tests are failing in VS Code |
This change is a follow-up to #11144 (comment) that addresses two issues that may help with diagnosis: 1. `Assumed.Unreachable(...)`captures the `[CallerFilePath]` and `[CallerLineNumber]` but didn't do anything with them. Now, those values are appended to the exception message. 2. Two `Assumed.Unreachable(...)` overloads have been added to allow a custom exception message to be provided that can be used to add more detail about a failure.
Add logging on the server side for why a project isn't being serialized.
Reduce some of the logging on the client side if it gets into a bad state. Log once for first bad read and once when it gets to a point where we think we returned to a good state.