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

Improve error messages/notifications #65

Closed
Tracked by #1068
andrewazores opened this issue Apr 30, 2020 · 3 comments
Closed
Tracked by #1068

Improve error messages/notifications #65

andrewazores opened this issue Apr 30, 2020 · 3 comments
Assignees
Labels
feat New feature or request

Comments

@andrewazores
Copy link
Member

#59 (comment)

As in the pull request review above, the error messages sent from ContainerJFR could use some improvement. In some cases errors are only reflected in response status codes, with no meaningful payload text. In other cases there is payload text included, but this text may be simply raw, unformatted text (no JSON wrapping etc.), and with no particular well-established conventions. This makes it difficult for ContainerJFR Web or other clients to consume these messages and produce meaningful notifications for end users.

@hareetd
Copy link
Contributor

hareetd commented Oct 8, 2021

From what I can tell, it seems errors from the backend are routed through 1 of 5 classes on the frontend: Api.service.tsx, NotificationChannel.service.tsx, Report.service.tsx, Targets.service.tsx, and TargetSelect.tsx. There doesn't seem to be a lot of work required to improve the error logging/notifications there.

Looking through the backend, for this conversation I'm just going to focus on the HTTP API handlers since the bulk of errors are emitted there. In terms of payload text, what do you have in mind when it comes to improving the emitted errors since they typically consist of various Exceptions types (with payloads that we haven't touched) emitted automatically along the call-chain or are manually thrown by us in special cases (eg. 404 Recording not found), in which the case the payload is already taken care of. In the latter case, should I be ensuring Exception messages are JSON wrapped?

Do you have a specific example I could work off of?

@andrewazores
Copy link
Member Author

I don't have any specific examples - keep in mind that this issue was filed in April 2020, and things on both the frontend and backend side have already changed (improved, hopefully) a fair amount since then as well. I do think there are still cases where we can display better error messages, but I don't have any specifics in mind right now. I can take a look around and see if I can find things that need improvement.

@hareetd
Copy link
Contributor

hareetd commented Oct 13, 2021

I don't have any specific examples - keep in mind that this issue was filed in April 2020, and things on both the frontend and backend side have already changed (improved, hopefully) a fair amount since then as well.

Good point :P

I can take a look around and see if I can find things that need improvement.

Sounds good, I appreciate it. I'll keep looking as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants