Skip to content

More concise error message for schema validation errors#32

Merged
aeschli merged 1 commit intomicrosoft:masterfrom
literalplus:pn/val-err-msg
Oct 24, 2018
Merged

More concise error message for schema validation errors#32
aeschli merged 1 commit intomicrosoft:masterfrom
literalplus:pn/val-err-msg

Conversation

@literalplus
Copy link
Contributor

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.

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
Copy link
Collaborator

aeschli commented Oct 24, 2018

Change makes sense. Thanks!

@aeschli aeschli merged commit 82d4aa4 into microsoft:master Oct 24, 2018
@aeschli aeschli added this to the October 2018 milestone Oct 24, 2018
@literalplus literalplus deleted the pn/val-err-msg branch October 24, 2018 14:01
@literalplus
Copy link
Contributor Author

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants