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

Fixes #12685 - Improve error handling for TSVB #12688

Merged
merged 2 commits into from
Aug 10, 2017

Conversation

simianhacker
Copy link
Member

This PR improves the error handling for Javascript thrown errors on the server. It also adds more details to permission errors which is caused by a missing aggregation key.

image

@cjcenizal
Copy link
Contributor

Woah, I've never seen this state before! @snide Have you seen this?

@simianhacker simianhacker changed the title Fixes #12685 - Improve error handling Fixes #12685 - Improve error handling for TSVB Jul 6, 2017
@snide
Copy link
Contributor

snide commented Jul 6, 2017

No but reminds me of the multiple permission display one that came in a couple days ago.

@cjcenizal
Copy link
Contributor

@simianhacker How can we trigger this error to repro your screenshot?

@simianhacker
Copy link
Member Author

It's the error message in TSVB. If you create a visualization with an error in it (a bad script in a calculation works really well) then you will see it.

image

Copy link
Member

@weltenwort weltenwort left a comment

Choose a reason for hiding this comment

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

The error message pointing the user to incorrect permissions also shows up when entering an index pattern that does not match any indices. Maybe we should include that in the message as well, e.g. "The aggregations key is missing from the response. Please check the index pattern and your permissions for reading from it."?

@thomasneirynck thomasneirynck added Feature:Visualizations Generic visualization features (in case no more specific feature label is available) and removed v5.6.0 labels Jul 10, 2017
@thomasneirynck
Copy link
Contributor

Closes #12685.

@thomasneirynck
Copy link
Contributor

@simianhacker makes sense to change to add that info to the error message as well? If so, we can probably go ahead and merge this. thx,

@simianhacker simianhacker merged commit 805c15c into elastic:master Aug 10, 2017
@simianhacker
Copy link
Member Author

Back ported to 6.0 with dd36884
Back ported to 6.x with 47abe12

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:TSVB TSVB (Time Series Visual Builder) Feature:Visualizations Generic visualization features (in case no more specific feature label is available) release_note:fix review v6.0.0-beta2 v6.0.0 v7.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants