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.

Sign up for GitHub

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

[Gutenberg] Add error boundary components and exception logging #20359

Merged
merged 25 commits into from
Mar 12, 2024
Merged

[Gutenberg] Add error boundary components and exception logging #20359

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
43526c5
Add log exception handlers
jhnstn Mar 1, 2024
32db175
Prevent including React Native source in build
fluiddot Feb 29, 2024
fdaa982
Add gem `fastlane-plugin-sentry`
fluiddot Feb 29, 2024
9c4f788
Update build process to upload React Native source map to Sentry
fluiddot Feb 29, 2024
f132e70
Update Gutenberg Mobile reference
fluiddot Feb 29, 2024
61dec5a
Address issues identified by Dangermattic
fluiddot Mar 1, 2024
1a63c72
Update inline comment of `sentry_upload_sourcemap` invocation
fluiddot Mar 4, 2024
e87d6d8
remove past tense 'did'
jhnstn Mar 4, 2024
b31495c
Pass the exception event data through the editor lib
jhnstn Mar 6, 2024
ce6908c
Send the exception to the crash logger
jhnstn Mar 6, 2024
732fe40
Update GB and Tracks sources
jhnstn Mar 6, 2024
bdf0b0f
Merge branch 'trunk' into rnmobile/add/log-exception-to-crash-logging
jhnstn Mar 6, 2024
6291d17
Add missing import in `GutenbergEditorFragment`
fluiddot Mar 7, 2024
278fc9a
Update Gutenberg Mobile reference
fluiddot Mar 7, 2024
e63da63
Update Tracks reference
fluiddot Mar 7, 2024
6b0ad11
Update Tracks reference
fluiddot Mar 7, 2024
9408f29
Include Tracks Maven group in editor library
fluiddot Mar 7, 2024
b2ff729
Fix `sentry.properties` path in `upload_gutenberg_sourcemaps`
fluiddot Mar 7, 2024
08b5355
Add `dist` parameter to `sentry_upload_sourcemap`
fluiddot Mar 7, 2024
31beaac
Update tracks to latest version: 3.5.0
jhnstn Mar 8, 2024
82fe4b6
Update Gutenberg Mobile reference
fluiddot Mar 11, 2024
57cc01c
Copy bundle and source map files to a specific build folder
fluiddot Mar 12, 2024
fe45679
Exclude React Native source map when building the app
fluiddot Mar 12, 2024
17ba9b9
Remove unnecessary `build_asset_folder_name` variable in `upload_gute…
fluiddot Mar 12, 2024
eadc097
Remove unnecessary params from `upload_gutenberg_sourcemaps`
fluiddot Mar 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ gem 'nokogiri'

### Fastlane Plugins

gem 'fastlane-plugin-sentry'
gem 'fastlane-plugin-wpmreleasetoolkit', '~> 9.2'
# gem 'fastlane-plugin-wpmreleasetoolkit', path: '../../release-toolkit'
# gem 'fastlane-plugin-wpmreleasetoolkit', git: 'https://github.com/wordpress-mobile/release-toolkit', branch: ''
Expand Down
3 changes: 3 additions & 0 deletions Gemfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,8 @@ GEM
xcodeproj (>= 1.13.0, < 2.0.0)
xcpretty (~> 0.3.0)
xcpretty-travis-formatter (>= 0.0.3)
fastlane-plugin-sentry (1.19.0)
os (~> 1.1, >= 1.1.4)
fastlane-plugin-wpmreleasetoolkit (9.4.0)
activesupport (>= 6.1.7.1)
buildkit (~> 1.5)
Expand Down Expand Up @@ -355,6 +357,7 @@ PLATFORMS
DEPENDENCIES
danger-dangermattic (~> 1.0)
fastlane (~> 2)
fastlane-plugin-sentry
fastlane-plugin-wpmreleasetoolkit (~> 9.2)
nokogiri
rmagick (~> 4.1)
Expand Down
3 changes: 3 additions & 0 deletions WordPress/build.gradle
View file
Open in desktop
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Following #20359 (comment) , we had to make an adjustment in order to avoid including the source map file (index.android.bundle.map) in the app. This file is only needed in the build process to upload it to Sentry and shouldn't be in the resulting APK.

Originally, we tried using exclude '**/*.bundle.map' in this file (32db175) but for some reason, it was still adding the source map file. As a second approach, we are now copying the bundle and source map files to a separate folder and deleting the source map to avoid inclusion. The new folder will be used in the upload_gutenberg_sourcemaps function when uploading the files to Sentry.

@AliSoftware since you already took a look at the original approach (https://github.com/wordpress-mobile/WordPress-Android/pull/20359/files#r1510915719), I wonder if you could double-check that this approach is valid. Thanks for your help 🙇 !

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I was focused on doing a CI deployment today, and only getting to this PR now, but I see it's already merged 🙂

I only skimmed the new code in build.gradle but your explanations above makes sense to me so I think the approach sounds good 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I was focused on doing a CI deployment today, and only getting to this PR now, but I see it's already merged 🙂

No worries @AliSoftware. Yes, we finally decided to merge the PR (internal ref: p1710254914686709/1710180482.367029-slack-C06G8EGHE1H) to unblock other potential PRs, as the counterpart Gutenberg changes were already merged. In any case, if we identify improvements to be made to this approach, we'll be happy to apply them in a separate PR.

I only skimmed the new code in build.gradle but your explanations above makes sense to me so I think the approach sounds good 👍

Great, thanks for taking a look and validating the approach 🙇 !

Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,9 @@ android {
exclude '**/libjscexecutor.so'
exclude '**/libfabricjni.so'

// Exclude React Native's JavaScript source map
exclude '**/*.bundle.map'

// Avoid React Native's JNI duplicated classes
pickFirst '**/libc++_shared.so'
pickFirst '**/libfbjni.so'
Expand Down
2 changes: 1 addition & 1 deletion build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ ext {
automatticRestVersion = '1.0.8'
automatticStoriesVersion = '2.4.0'
automatticTracksVersion = '3.4.0'
gutenbergMobileVersion = 'v1.114.0'
gutenbergMobileVersion = '6655-a88a07389f55bcf5a233a57ff01802dbe3968a04'
wordPressAztecVersion = 'v2.0'
wordPressFluxCVersion = 'trunk-8b930418a49b0d0846ed56ebf8fd8adad5011901'
wordPressLoginVersion = '1.14.0'
Expand Down
54 changes: 52 additions & 2 deletions fastlane/lanes/build.rb
View file
Open in desktop
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wordpress-mobile/apps-infrastructure I'd appreciate it if someone could take a look at this approach for uploading the React Native's JavaScript source map file to Sentry. I'm mostly seeking validation of the approach, thoughts/feedback or better alternatives. Thank you very much for the help 🙇 !

To give some context on the approach: The source map is generated as part of the Gutenerg Mobile build process, the files are being passed as assets via the library (reference). Sentry uses the JavaScript bundle and the source map to symbolicate the stack trace and reference the exact code line that caused the exception (reference).

The approach followed here consists in:

  1. Add Gem fastlane-plugin-sentry to use the sentry_upload_sourcemap action.
  2. Add upload_gutenberg_sourcemaps helper function to build lanes.
  3. To invoke sentry_upload_sourcemap we need a set of parameters that are fetched with the following steps:
    1. Load Sentry properties to obtain the token, organization and project slugs.
    2. Locate the bundle and source map files passed as assets of the Gutenberg Mobile dependency, based on app, build flavor, and build type. Note that the path points to intermediates folder within the project's build path.
  4. Invoke the upload_gutenberg_sourcemaps in the different build lanes.

As a first approach, I tried to copy the bundle and source map files to a different folder during the asset merging step in the build process. However, I finally discarded this option because seemed unnecessary. Here you can see the changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Approach looks good to me; thanks for documenting it all with comments 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @AliSoftware for taking a look and validating the approach, I appreciate it 🙇 .

Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,12 @@
# Create the file names
app = get_app_name_option!(options)
version_name = current_version_name
build_bundle(app: app, version_name: version_name, build_code: current_build_code, flavor: 'Vanilla', buildType: 'Release')
flavor = 'Vanilla'
build_type = 'Release'
build_bundle(app: app, version_name: version_name, build_code: current_build_code, flavor: flavor, buildType: build_type)

upload_build_to_play_store(app: app, version_name: version_name, track: 'production')
upload_gutenberg_sourcemaps(app: app, flavor: flavor, build_type: build_type, release_version: version_name)

create_gh_release(app: app, version_name: version_name) if options[:create_release]
end
Expand Down Expand Up @@ -102,9 +105,12 @@
# Create the file names
app = get_app_name_option!(options)
version_name = current_version_name
build_bundle(app: app, version_name: version_name, build_code: current_build_code, flavor: 'Vanilla', buildType: 'Release')
flavor = 'Vanilla'
build_type = 'Release'
build_bundle(app: app, version_name: version_name, build_code: current_build_code, flavor: flavor, buildType: build_type)

upload_build_to_play_store(app: app, version_name: version_name, track: 'beta') if options[:upload_to_play_store]
upload_gutenberg_sourcemaps(app: app, flavor: flavor, build_type: build_type, release_version: version_name)

create_gh_release(app: app, version_name: version_name, prerelease: true) if options[:create_release]
end
Expand Down Expand Up @@ -217,6 +223,7 @@
)

upload_prototype_build(product: 'WordPress', version_name: version_name)
upload_gutenberg_sourcemaps(app: 'Wordpress', flavor: PROTOTYPE_BUILD_FLAVOR, build_type: PROTOTYPE_BUILD_TYPE, release_version: version_name)
end

#####################################################################################
Expand All @@ -240,6 +247,7 @@
)

upload_prototype_build(product: 'Jetpack', version_name: version_name)
upload_gutenberg_sourcemaps(app: 'Jetpack', flavor: PROTOTYPE_BUILD_FLAVOR, build_type: PROTOTYPE_BUILD_TYPE, release_version: version_name)
end

#####################################################################################
Expand Down Expand Up @@ -359,4 +367,46 @@ def generate_prototype_build_number
"#{branch}-#{commit}"
end
end

# Uploads the React Native JavaScript bundle and source map files.
# These files are provided by the Gutenberg Mobile library.
#
# @param [String] app App name, e.g. 'WordPress' or 'Jetpack'.
# @param [String] flavor Build flavor, e.g. 'Jalapeno' or 'Vanilla'.
# @param [String] build_type Build type, e.g. 'Debug' or 'Release'.
# @param [String] release_version Release version name to attach the files to in Sentry.
#
def upload_gutenberg_sourcemaps(app:, flavor:, build_type:, release_version:)
fluiddot marked this conversation as resolved.
Show resolved Hide resolved
fluiddot marked this conversation as resolved.
Show resolved Hide resolved
# Load Sentry properties
sentry_path = File.join(PROJECT_ROOT_FOLDER, 'WordPress', 'src', app, 'sentry.properties')
sentry_properties = JavaProperties.load(sentry_path)
sentry_token = sentry_properties[:'auth.token']
project_slug = sentry_properties[:'defaults.project']
org_slug = sentry_properties[:'defaults.org']

# The bundle and source map files are extracted from merged assets location created after building the app.
# The format is: <app><flavor><build_type>
# E.g.: jetpackJalapenoDebug
build_asset_folder_name = "#{app.downcase}#{flavor}#{build_type}"
fluiddot marked this conversation as resolved.
Show resolved Hide resolved
bundle_source_map_path = File.join(PROJECT_ROOT_FOLDER, 'WordPress', 'build', 'intermediates', 'assets', build_asset_folder_name)

Dir.mktmpdir do |sourcemaps_folder|
# It's important that the bundle and source map files have specific names, otherwise, Sentry won't symbolicate the stack traces.
FileUtils.cp(File.join(bundle_source_map_path, 'index.android.bundle'), File.join(sourcemaps_folder, 'index.android.bundle'))
FileUtils.cp(File.join(bundle_source_map_path, 'index.android.bundle.map'), File.join(sourcemaps_folder, 'index.android.bundle.map'))
fluiddot marked this conversation as resolved.
Show resolved Hide resolved

sentry_upload_sourcemap(
auth_token: sentry_token,
org_slug: org_slug,
project_slug: project_slug,
version: release_version,
# When the React native bundle is generated, the source map file references include the local machine path, with the `rewrite` and
# `strip_common_prefix` include the local machine path, with the `rewrite` and `strip_common_prefix` options Sentry automatically
# strips this part.
fluiddot marked this conversation as resolved.
Show resolved Hide resolved
rewrite: true,
strip_common_prefix: true,
sourcemap: sourcemaps_folder
)
end
end
end
3 changes: 3 additions & 0 deletions ...itor/src/main/java/org/wordpress/android/editor/gutenberg/GutenbergContainerFragment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.wordpress.mobile.WPAndroidGlue.WPAndroidGlueCode.OnEditorAutosaveListener;
import org.wordpress.mobile.WPAndroidGlue.WPAndroidGlueCode.OnEditorMountListener;
import org.wordpress.mobile.WPAndroidGlue.WPAndroidGlueCode.OnGetContentInterrupted;
import org.wordpress.mobile.WPAndroidGlue.WPAndroidGlueCode.OnDidLogExceptionListener;
import org.wordpress.mobile.WPAndroidGlue.WPAndroidGlueCode.OnGutenbergDidRequestEmbedFullscreenPreviewListener;
import org.wordpress.mobile.WPAndroidGlue.WPAndroidGlueCode.OnGutenbergDidRequestUnsupportedBlockFallbackListener;
import org.wordpress.mobile.WPAndroidGlue.WPAndroidGlueCode.OnGutenbergDidSendButtonPressedActionListener;
Expand Down Expand Up @@ -100,6 +101,7 @@ public void attachToContainer(ViewGroup viewGroup, OnMediaLibraryButtonListener
OnToggleRedoButtonListener onToggleRedoButtonListener,
OnConnectionStatusEventListener onConnectionStatusEventListener,
OnBackHandlerEventListener onBackHandlerEventListener,
OnDidLogExceptionListener onDidLogExceptionListener,
boolean isDarkMode) {
mWPAndroidGlueCode.attachToContainer(
viewGroup,
Expand Down Expand Up @@ -127,6 +129,7 @@ public void attachToContainer(ViewGroup viewGroup, OnMediaLibraryButtonListener
onToggleRedoButtonListener,
onConnectionStatusEventListener,
onBackHandlerEventListener,
onDidLogExceptionListener,
isDarkMode);
}

Expand Down
10 changes: 10 additions & 0 deletions .../editor/src/main/java/org/wordpress/android/editor/gutenberg/GutenbergEditorFragment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,9 @@
import org.wordpress.android.util.helpers.MediaFile;
import org.wordpress.android.util.helpers.MediaGallery;
import org.wordpress.aztec.IHistoryListener;
import org.wordpress.mobile.ReactNativeGutenbergBridge.GutenbergBridgeJS2Parent.LogExceptionCallback;
import org.wordpress.mobile.ReactNativeGutenbergBridge.GutenbergEmbedWebViewActivity;
import org.wordpress.mobile.ReactNativeGutenbergBridge.GutenbergJsException;
import org.wordpress.mobile.WPAndroidGlue.Media;
import org.wordpress.mobile.WPAndroidGlue.MediaOption;
import org.wordpress.mobile.WPAndroidGlue.RequestExecutor;
Expand All @@ -76,6 +78,7 @@
import org.wordpress.mobile.WPAndroidGlue.WPAndroidGlueCode.OnEditorMountListener;
import org.wordpress.mobile.WPAndroidGlue.WPAndroidGlueCode.OnFocalPointPickerTooltipShownEventListener;
import org.wordpress.mobile.WPAndroidGlue.WPAndroidGlueCode.OnGetContentInterrupted;
import org.wordpress.mobile.WPAndroidGlue.WPAndroidGlueCode.OnDidLogExceptionListener;
import org.wordpress.mobile.WPAndroidGlue.WPAndroidGlueCode.OnGutenbergDidRequestEmbedFullscreenPreviewListener;
import org.wordpress.mobile.WPAndroidGlue.WPAndroidGlueCode.OnGutenbergDidRequestPreviewListener;
import org.wordpress.mobile.WPAndroidGlue.WPAndroidGlueCode.OnGutenbergDidRequestUnsupportedBlockFallbackListener;
Expand Down Expand Up @@ -603,6 +606,13 @@ public void onGotoCustomerSupportOptions() {
}
},

new OnDidLogExceptionListener() {
@Override public void didLogException(GutenbergJsException exception,
LogExceptionCallback onLogExceptionCallback) {
AppLog.d(T.EDITOR, "Gutenberg JS Exception in GB editor fragment");
onLogExceptionCallback.onLogException(true);
}
},
GutenbergUtils.isDarkMode(getActivity()));

// request dependency injection. Do this after setting min/max dimensions
Expand Down
Loading