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

feat(graphql): improve error handling #1295

Merged
merged 16 commits into from
Aug 7, 2024

Conversation

Josh-Matsuoka
Copy link
Contributor

@Josh-Matsuoka Josh-Matsuoka commented Jul 9, 2024

Fixes cryostatio/cryostat#394

Currently when graphQL queries fail due to missing SSL configuration or authentication failure the Web UI can't handle them correctly because they aren't throw in the traditional way. Rather than generating a 427 or 502 and accompanying HttpException they're instead masked by GraphQL and return a 200 but with a special response.

This response takes the form of an error object with various fields determined by the error handling configuration on the server. The accompanying cryostat3 PR changes this configuration to give us a form we can work with to determine the cause of the error and deal with it accordingly. With the server correctly configured this gives us the underlying exception and message, and the graphQL error classification (i.e. DataFetchingError).

This PR updates the graphQL queries and surrounding error handling to account for errors correctly and set the auth/ssl failure flags on the target accordingly.

Copy link
Member

@andrewazores andrewazores left a comment

Choose a reason for hiding this comment

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

Quite a few eslint warnings emitted:

https://github.com/cryostatio/cryostat-web/pull/1295/files

@andrewazores andrewazores changed the title Fix GraphQL Exception handling feat(graphql): improve error handling Jul 10, 2024
@andrewazores andrewazores added feat New feature or request safe-to-test labels Jul 10, 2024
@Josh-Matsuoka
Copy link
Contributor Author

There are a few test failures I'm working on resolving and I need to fix the snapshot tests as well. eslint and formatting issues are fixed

@andrewazores andrewazores merged commit 9dafc5b into cryostatio:main Aug 7, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request safe-to-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] GraphQL requests do not properly support custom auth/TLS error status codes
2 participants