Skip to content

Conversation

@ramya-rao-a
Copy link
Contributor

Fixes #15374

When promises from language feature providers are rejected, we currently call onUnexpectedError which ends up calling error telemetry service that logs the error as a telemetry event.
There are 2 reasons why we wouldn't want this

  • It is noise when we triage the error telemetry events because these errors are external as far as the core product is concerned.
  • It may contain PII.

@jrieken
Copy link
Member

jrieken commented Nov 28, 2016

This is full list of language features/registries. Unsure if we have already covered all. I am also happy to help @ramya-rao-a. If you don't mind I can push more changes straight into your branch.

  • ReferenceProviderRegistry
  • RenameProviderRegistry
  • SuggestRegistry
  • SignatureHelpProviderRegistry
  • HoverProviderRegistry
  • DocumentSymbolProviderRegistry
  • DocumentHighlightProviderRegistry
  • DefinitionProviderRegistry
  • CodeLensProviderRegistry
  • CodeActionProviderRegistry
  • DocumentFormattingEditProviderRegistry
  • DocumentRangeFormattingEditProviderRegistry
  • OnTypeFormattingEditProviderRegistry
  • LinkProviderRegistry

@ramya-rao-a
Copy link
Contributor Author

ramya-rao-a commented Nov 28, 2016

@jrieken I got my list from the modes.ts file. The ones missing (format and rename) were not using onUnexpectedError at the moment, so didn't touch them.
Feel free to push changes to my branch

@jrieken
Copy link
Member

jrieken commented Nov 28, 2016

yeah, we might not be using that util, but their error might still end up in normal error telemetry otherwise

@ramya-rao-a
Copy link
Contributor Author

Updated the RenameProviderRegistry.

DocumentFormattingEditProviderRegistry and DocumentRangeFormattingEditProviderRegistry handle the rejected promises by ignoring them. So these won't bubble up and reach telemetry. Am not sure if they were ignored intentionally or did we just miss calling onUnexpectedError in these cases.

In case of OnTypeFormattingEditProviderRegistry, the provideOnTypeFormattingEdits doesn't seem to be returning a promise. Doing a .then on it gives syntax errors. You know anything about that?

@jrieken
Copy link
Member

jrieken commented Nov 29, 2016

I have pushed two more commits - one to fix a test failure as we apparently expect rename error to bubble up. I now log and send a generic error around. Also, updated the formatting logic to always log errors - I believe that wasn't ignored for a specific reason.

@ramya-rao-a
Copy link
Contributor Author

@jrieken Travis is failing in Mac. You know anything about the failures?

@ramya-rao-a ramya-rao-a merged commit aa099fe into master Dec 1, 2016
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Error Telemetry has errors from processes run inside the extension that may contain PII

4 participants