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

Send real errors to telemetry #981

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

remcohaszing
Copy link
Contributor

What does this PR do?

Every call to telemetry.sendError contained the same boilerplate code to create an object with the property error, which contains a string representation of the real error. This change moves this conversion into the Telemetry implementation. This reduces the need for boilerplate code in other places.

What issues does this PR fix or reference?

This change also affects monaco-yaml. Because the telemetry now resolves the real error, the browser can log the error to the console with a better stack trace.

Is it tested? How?

npm test

Every call to `telemetry.sendError` contained the same boilerplate code
to create an object with the property `error`, which contains a string
representation of the real error. This change moves this conversion into
the `Telemetry` implementation. This reduces the need for boilerplate
code in other places.

This change also affects `monaco-yaml`. Because the telemetry now
resolves the real error, the browser can log the error to the console
with a better stack trace.
@remcohaszing
Copy link
Contributor Author

Bonus tip: if you replace convertErrorToTelemetryMsg with Node.js’ builtin function util.inspect(), you get better error representations, including error causes.

@coveralls
Copy link

Coverage Status

coverage: 84.159% (-0.02%) from 84.174%
when pulling 10dc8e6 on remcohaszing:telemetry-send-error
into f039273 on redhat-developer:main.

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.

2 participants