Skip to content

Add asserts to narrow down position issue #35742

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

Merged
merged 3 commits into from
Dec 18, 2019

Conversation

uniqueiniquity
Copy link
Contributor

In Visual Studio, we're seeing hundreds of hits where a location returned in semanticDiagnosticsSync has either a non-positive line or non-positive offset.

To narrow down the investigation, I'm adding these asserts so that we have just a little more context to help the investigation (that is, I think it would be helpful to know whether the bad value is null/undefined, 0, or negative).

Copy link
Contributor

@armanio123 armanio123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. I wonder if it's a good idea to do some work to include the server request and response on the telemetry-bot as additional information. We can discuss this offline.

Copy link
Contributor Author

@uniqueiniquity uniqueiniquity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a couple more asserts to handle the input - for example, I think that if we get undefined as input, we might output NaN.

@uniqueiniquity uniqueiniquity merged commit 3c153d8 into microsoft:master Dec 18, 2019
@uniqueiniquity uniqueiniquity deleted the addPositionAssert branch December 18, 2019 22:19
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

Successfully merging this pull request may close these issues.

4 participants