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

More concise error message for schema validation errors #32

Merged
merged 1 commit into from Oct 24, 2018

Conversation

Projects
None yet
2 participants
@xxyy
Contributor

xxyy commented Oct 24, 2018

Ellipsis

The error message shown if a schema could not be retrieved is currently not very concise, it repeats the same sentence three times. This PR shortens it a little.

ref: Microsoft/vscode#60219 (comment) by @aeschli

Old message:
bad

New message:
image

How this works is that it strips everything before Error: in the message returned by the backend, if Error: appears in the message. In the messages I observed, the removed part was something like Unable to connect to <url>: Error: , which is redundant because it's prepended by the only caller anyways.

I wasn't sure about the exact desired format of the error message, so if another format is better, I can change it. One issue with the current one is that it doesn't explicitly state that a schema is the problem. However I didn't want to change an already-translated message since translations would need to update.

More concise error message for schema validation errors
Don't repeat the same cause 'Poblem loading schema from <url>'
three times in a single error message.
We now only show the actual network
error, one description, and the URL.
@aeschli

This comment has been minimized.

Contributor

aeschli commented Oct 24, 2018

Change makes sense. Thanks!

@aeschli aeschli merged commit 82d4aa4 into Microsoft:master Oct 24, 2018

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
license/cla All CLA requirements met.

@aeschli aeschli added this to the October 2018 milestone Oct 24, 2018

@xxyy xxyy deleted the xxyy:pn/val-err-msg branch Oct 24, 2018

@xxyy

This comment has been minimized.

Contributor

xxyy commented Oct 24, 2018

Apparently, there was a test for the exact message, which fails now: https://travis-ci.org/Microsoft/vscode-json-languageservice/builds/445672376?utm_source=github_status&utm_medium=notification

I didn't see that there were tests I needed to run, the correct build command (I used yarn run compile) would be a good addition to the project readme.

I'll create another one to fix that real quick.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment