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

Fire telemetry if Razor buffers get out of sync, and recover from same #6494

Merged
merged 2 commits into from
Oct 5, 2023

Conversation

davidwengier
Copy link
Contributor

This is the VS Code equivalent of dotnet/razor#9202

Fixes #6488

I'm not sure why this is so easily reproducable for the user reporting the error, but its extremely intermittent for me. I basically hit it twice today, and never again, but I did at least see that we had duplicate HTML content in our generated file when I hit it, so can be sure this is the issue we are running into.

Perhaps a new timing issue with our initialization, now that its not deferred until file open, but with part of Roslyn and the extension still expecting it to be. Hopefully dotnet/razor#9344 will fix those issues.

@davidwengier davidwengier requested a review from a team as a code owner October 5, 2023 05:40
@davidwengier
Copy link
Contributor Author

@allisonchou is this all I need to do for telemetry reporting? It seems too easy :)

Copy link
Contributor

@allisonchou allisonchou left a comment

Choose a reason for hiding this comment

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

LGTM!

@allisonchou is this all I need to do for telemetry reporting? It seems too easy :)

I think so? I'm not as familiar with telemetry on the Typescript side of things - @ryzngard would probably know for sure. Does it show up on the GDPR site?

src/razor/src/document/razorDocumentManager.ts Outdated Show resolved Hide resolved
src/razor/src/document/razorDocumentManager.ts Outdated Show resolved Hide resolved
@davidwengier
Copy link
Contributor Author

Does it show up on the GDPR site?

I haven't actually been able to repro the issue to check, since putting the fix in, but I'll try some more.

I was mainly wondering if it's okay to just be hooking up the TelemetryReporter like this, and calling methods on it at will, or if there needs to be checks of some kind so we only do it in DevKit

Can you tell I copied and pasted this bit? :)

Co-authored-by: Allison Chou <allichou@microsoft.com>
Copy link
Contributor

@ryzngard ryzngard left a comment

Choose a reason for hiding this comment

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

I believe this is correct for telemetry in ts.

:shipit:

@davidwengier davidwengier merged commit 1564600 into dotnet:main Oct 5, 2023
8 checks passed
@davidwengier davidwengier deleted the BuffersOutOfSync branch October 5, 2023 23: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.

Formatting razor file leads "Request textDocument/formatting failed" with different line numbers
3 participants