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

[vtadmin-web] Add client-side error handling interface + Bugsnag implementation #8287

Merged
merged 1 commit into from
Jun 10, 2021

Conversation

doeg
Copy link
Contributor

@doeg doeg commented Jun 8, 2021

Signed-off-by: Sara Bee 855595+doeg@users.noreply.github.com

Description

  • Adds a generic + extensible interface for observability into client-side errors.
  • Implements the ErrorHandler interface for Bugsnag, for deployments that use it.

Implementations for other monitoring services (common ones being Sentry, Rollbar, Datadog, etc.) are left as future enhancements. 😸 (I briefly considered implementing one other, to prove the concept, but... it didn't really make sense to add an integration that no one uses!) I looked into the JavaScript libraries for the aforementioned monitoring services and all of them fit nicely into the ErrorHandler interface proposed here.

Also unimplemented for now + worth mentioning are sourcemaps, which provide proper stack-traces for errors (since our client-side code is minified). "Undocumented" is perhaps a better term than "unimplemented", as source map uploads will (unfortunately) be the responsibility of the operator. I do promise to document how we do it, once we do it. 🙏

Here's an example of what this (specifically, the metadata) looks like in Bugsnag:

Screen Shot 2021-06-09 at 8 05 53 AM

And here's what it looks like with the Bugsnag/Slack integration:

Screen Shot 2021-06-09 at 8 05 33 AM

Many thanks to David + Anders from PlanetScale for pointing me at https://vitess.io/docs/user-guides/configuration-basic/monitoring/#tools, which was super useful in terms of precedent + documentation!

Related Issue(s)

N/A

Checklist

  • Tests were added or are not required
  • Documentation was added or is not required

Deployment Notes

For any deployments that want to integrate VTAdmin with Bugsnag, you'll need to set the REACT_APP_BUGSNAG_API_KEY environment variable.

if (!json.ok) throw new HttpResponseNotOkError(endpoint, json);

return json as HttpOkResponse;
try {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately this diff looks much worse than it is, since I had to surround everything in a try/catch. Logic changes here are minimal and well-tested. 🙏

@doeg doeg added this to In progress in VTAdmin via automation Jun 8, 2021
@doeg doeg added Type: Enhancement Logical improvement (somewhere between a bug and feature) Component: VTAdmin VTadmin interface labels Jun 8, 2021
@doeg doeg force-pushed the sarabee-bugsnag branch 3 times, most recently from 3b08e3d to 51d80b2 Compare June 9, 2021 12:09
@doeg doeg marked this pull request as ready for review June 9, 2021 12:14
@doeg doeg requested a review from ajm188 as a code owner June 9, 2021 12:14
…ementation

Signed-off-by: Sara Bee <855595+doeg@users.noreply.github.com>
Copy link
Contributor

@ajm188 ajm188 left a comment

Choose a reason for hiding this comment

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

🐛 🐍

@ajm188 ajm188 merged commit ca1ac44 into vitessio:main Jun 10, 2021
VTAdmin automation moved this from In progress to Done Jun 10, 2021
@ajm188 ajm188 deleted the sarabee-bugsnag branch June 10, 2021 03:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: VTAdmin VTadmin interface Type: Enhancement Logical improvement (somewhere between a bug and feature)
Projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants