[Gutenberg] Add error boundary components and exception logging #20359
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
There was a problem hiding this comment. @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:
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. There was a problem hiding this comment. Approach looks good to me; thanks for documenting it all with comments 👍 There was a problem hiding this comment. Thanks @AliSoftware for taking a look and validating the approach, I appreciate it 🙇 . |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,9 +34,12 @@ | |
| # Create the file names | ||
| app = get_app_name_option!(options) | ||
| version_name = current_version_name | ||
| 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 | ||
|
|
@@ -102,9 +105,12 @@ | |
| # Create the file names | ||
| app = get_app_name_option!(options) | ||
| version_name = current_version_name | ||
| 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 | ||
|
|
@@ -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 | ||
|
|
||
| ##################################################################################### | ||
|
|
@@ -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 | ||
|
|
||
| ##################################################################################### | ||
|
|
@@ -359,4 +367,41 @@ 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.downcase, '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 and source map files are copied to a specific folder as part of the build process. | ||
fluiddot marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| bundle_source_map_path = File.join(PROJECT_ROOT_FOLDER, 'WordPress', 'build', 'react-native-bundle-source-map') | ||
|
There was a problem hiding this comment. This folder is created in: WordPress-Android/WordPress/build.gradle Lines 727 to 731 in eadc097 |
||
|
|
||
| sentry_upload_sourcemap( | ||
| auth_token: sentry_token, | ||
| org_slug: org_slug, | ||
| project_slug: project_slug, | ||
| version: release_version, | ||
| dist: current_build_code, | ||
| # When the React native bundle is generated, the source map file references include the local machine path; | ||
| # With the `rewrite` and `strip_common_prefix` options, Sentry automatically strips this part. | ||
| rewrite: true, | ||
| strip_common_prefix: true, | ||
| sourcemap: bundle_source_map_path | ||
| ) | ||
| end | ||
| end | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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 theupload_gutenberg_sourcemapsfunction 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 🙇 !
There was a problem hiding this comment.
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.gradlebut your explanations above makes sense to me so I think the approach sounds good 👍There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Great, thanks for taking a look and validating the approach 🙇 !