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

[etk/error-reporting] Setup action hook callback to capture React Error Boundary exceptions #65503

Merged

Conversation

fullofcaffeine
Copy link
Contributor

@fullofcaffeine fullofcaffeine commented Jul 12, 2022

Follow up to: WordPress/gutenberg#42024.

Proposed Changes

Register a callback to editor.ErrorBoundary.errorLogged WP action hook that will forward the error to Sentry every time the action is called.

The action has been added to all active React Error Boundaries in Gutenberg in the following changeset: WordPress/gutenberg#42024 .

Testing Instructions

  1. Sync this ETK build to your sandbox (or use install-plugin.sh to install from this branch);
  2. Get GB from trunk (or get the 13.7+, if available)
  3. Follow the instructions in this PR to simulate an error that will be captured by one of the Error Boundaries
  4. Build and sync the GB build to your sandbox (use a8c-wp-env to easily sync)
  5. Wait for the error to be triggered
  6. It should appear in Sentry after a while (in the wpcom-gutenberg-wp-admin project)

WARNING: Even if the testing plan passes, this should only be merged when GB 13.7 is shipped to WPCOM simple.

@fullofcaffeine fullofcaffeine changed the title [etk/error-reporting] Setup action hook callback to capture React Error Boundary exceptions… [etk/error-reporting] Setup action hook callback to capture React Error Boundary exceptions Jul 12, 2022
@matticbot
Copy link
Contributor

matticbot commented Jul 12, 2022

@matticbot
Copy link
Contributor

This PR modifies the release build for editing-toolkit

To test your changes on WordPress.com, run install-plugin.sh editing-toolkit capture/gb-react-error-boundary-errors-with-sentry on your sandbox.

To deploy your changes after merging, see the documentation: PCYsg-mMA-p2

@fullofcaffeine fullofcaffeine requested a review from a team July 12, 2022 16:57
@matticbot matticbot added the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Jul 12, 2022
@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@noahtallen
Copy link
Contributor

The code changes LGTM! I'll wait to test until GB is on edge to make things easier, unless you want quicker feedback :)

@fullofcaffeine fullofcaffeine force-pushed the capture/gb-react-error-boundary-errors-with-sentry branch from 8bbabd9 to b187cad Compare August 5, 2022 02:04
@fullofcaffeine
Copy link
Contributor Author

@noahtallen Thanks! I've just rebased it, and it should be ready to test! I'll also have a last look tomorrow :)

… in Gutenberg with Sentry

Register a callback to `editor.ErrorBoundary.errorLogged` action that will forward the error to Sentry everytime the action is called.

The action has been added to all active React Error Boundaries in Gutenberg in the following changeset: WordPress/gutenberg#42024 .
@fullofcaffeine fullofcaffeine force-pushed the capture/gb-react-error-boundary-errors-with-sentry branch from b187cad to 221feb2 Compare August 5, 2022 21:36
Copy link
Contributor

@noahtallen noahtallen left a comment

Choose a reason for hiding this comment

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

I was able to sync a Gutenberg build which causes an error in the PostTitle component. I then looked in the network requests and verified that data was posted to Sentry which includes a full error report for the exception. I also checked with the trunk build of ETK and verified that it does not send a full exception to Sentry.

@@ -18,6 +19,12 @@ function activateSentry() {
release: 'wpcom-test-01',
} );

// Capture exceptions from Gutenberg React Error Boundaries
addAction( 'editor.ErrorBoundary.errorLogged', 'etk/error-reporting', ( error ) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this work on Atomic if gutenberg is disabled? (I assume it would, as long as addAction doesn't crash everything if the action doesn't exist.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Atomic doesn't activate this module yet, but if it did, I don't see why it would crash -- if AT didn't have Gutenberg (and if the block editor still doesn't have the call to doAction) then the callback will just be registered here but never triggered.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was that what you had in mind?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think so, yeah. I was wondering what addAction does if the given "action type" doesn't exist

Copy link
Contributor

Choose a reason for hiding this comment

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

Since it's not loaded (I keep forgetting that!) I agree we don't need to worry about it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think so, yeah. I was wondering what addAction does if the given "action type" doesn't exist

I don't think it implements such a check -- though I agree it'd be nice to double check, though! I'll check this out in the following days.

@fullofcaffeine fullofcaffeine merged commit 8f3ed09 into trunk Aug 5, 2022
@fullofcaffeine fullofcaffeine deleted the capture/gb-react-error-boundary-errors-with-sentry branch August 5, 2022 22:11
@github-actions github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Aug 5, 2022
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.

3 participants