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

feat(react-native): new source maps generation script with support for hermes #1169

Merged
merged 9 commits into from
Sep 7, 2023
Merged

feat(react-native): new source maps generation script with support for hermes #1169

merged 9 commits into from
Sep 7, 2023

Conversation

andreybutov
Copy link
Collaborator

@andreybutov andreybutov commented Aug 23, 2023

Status

READY

Description

This PR replaces the original honeybadger-generate-sourcemaps.sh script with the honeybadger-upload-sourcemaps.sh script which has support for Hermes, and automatically uploads the generated sourcemaps to Honeybadger. The section on source maps in README.md has also been updated with usage instructions.

Todo

@andreybutov andreybutov changed the title feat(react-native): New source maps generation script with support for Hermes and automatic upload to Honeybadger. feat(react-native): New source maps generation script (Hermes + auto upload) Aug 23, 2023
@andreybutov andreybutov changed the title feat(react-native): New source maps generation script (Hermes + auto upload) feat(react-native): New source maps generation script with support for hermes. Aug 24, 2023
@BethanyBerkowitz BethanyBerkowitz changed the title feat(react-native): New source maps generation script with support for hermes. feat(react-native): new source maps generation script with support for hermes Aug 24, 2023
Copy link
Contributor

@BethanyBerkowitz BethanyBerkowitz left a comment

Choose a reason for hiding this comment

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

I tried running this and ran into two issues:

  1. I couldn't get npx honeybadger-upload-sourcemaps to run in my test project until I installed the package globally. I do vaguely remember having a problem with the previous npx honeybadger-generate-sourcemaps command as well.. unfortunately I did not write down what I did to fix that 😆. Hopefully the answer was not "install it globally", because that's not the right way to do it. Anyway I don't think this is an issue with this PR, but maybe either you our @subzero10 has an idea for me on this.
  2. After installing globally, the output of the script I got was
Hermes is enabled ...
Error: The Hermes compiler executable for Android was not found in the expected locations.

Happy to help debug, but this is just what I got on the first run.

packages/react-native/README.md Outdated Show resolved Hide resolved
@subzero10
Copy link
Member

I couldn't get npx honeybadger-upload-sourcemaps to run in my test project until I installed the package globally. I do vaguely remember having a problem with the previous npx honeybadger-generate-sourcemaps command as well.. unfortunately I did not write down what I did to fix that 😆. Hopefully the answer was not "install it globally", because that's not the right way to do it. Anyway I don't think this is an issue with this PR, but maybe either you our @subzero10 has an idea for me on this.

Yep, I had this issue multiple times as well. I think it works OK if the package is published on npm.

@andreybutov
Copy link
Collaborator Author

@BethanyBerkowitz @subzero10

I couldn't get npx honeybadger-upload-sourcemaps to run in my test project until I installed the package globally.

I don't know of a command to prevent npx from installing something globally if it doesn't exist locally to run it. My understanding is that if it doesn't find a file in ./node_modules/./bin/, it will download it, execute it, and clean up after itself. In my testing, when I did an npm build && npm pack, and then installed the HB library from the tgz, the honeybadger-upload-sourcemaps script was properly symlinked to in ./node_modules/.bin, and it executed from there when I ran npx.

Error: The Hermes compiler executable for Android was not found in the expected locations.

I made some updates to the script. @BethanyBerkowitz can you please try to run it again? If you get the same error, could you let me know what version of react-native in the package.json of your test app? Thanks!

@BethanyBerkowitz
Copy link
Contributor

  1. npx honeybadger-upload-sourcemaps

Yahoo, I saw the upload now! Not sure if it's because of your changes or if I screwed something up before, but definitely an improvement.

Generating the Android source map ...
Generating the iOS source map ...
Hermes is enabled ...
Generating Hermes Android source map ...
Generating Hermes iOS source map ...
Source maps generated. iOS: Uploading to Honeybadger ...
Source maps uploaded to Honeybadger.

My problem now is that I still don't see the sourcemap applied. I tried throwing a JS error using the examples/react-native-cli project on the ios simulator. I tried npm run ios and also release mode npm run ios:release.

I can see the sourcemaps uploaded ✅
Minified URL

But not applied ❌
InvokeGuardedCallbackProd(

I did check that the revision matched:
Screen Shot 2023-09-06 at 11 32 18 AM

@andreybutov
Copy link
Collaborator Author

andreybutov commented Sep 6, 2023

@BethanyBerkowitz

My problem now is that I still don't see the sourcemap applied. I tried throwing a JS error using the examples/react-native-cli project on the ios simulator.

Yes, I'm still working out with @joshuap why the backend isn't applying the uploaded iOS maps, but this PR was more about the introduction of hermes support for Android source maps (for this: #1159 ). iOS are still an outstanding issue that I will look into with a separate PR, as needed.

@BethanyBerkowitz
Copy link
Contributor

@andreybutov Glad to hear the iOS thing is a known issue!

I tested using android, and saw the sourcemaps get applied, both in the react-native-cli example project and the expo example project 🎉 🎉 🎉

In the expo app, the script at first errored with The Hermes compiler executable for iOS was not found in the expected location: .../expo-app/node_modules/.bin/../../ios/Pods/hermes-engine/destroot/bin/hermesc. This was because pod install hadn't run. In Expo, you don't actually need to run pod install yourself, it does it for you when you run npx expo run:ios. But either way, it might be helpful if this error included Did you install Pods? (or something similar). Pretty minor, I don't feel like I need to re-review if you decide to add that.

@subzero10
Copy link
Member

iOS are still an outstanding issue that I will look into with a separate PR, as needed.

I create #1182 to track this.

@subzero10
Copy link
Member

Note to me:

  • We should update the docs project to read the updated README.md from this repo.

subzero10
subzero10 previously approved these changes Sep 6, 2023
Copy link
Member

@subzero10 subzero10 left a comment

Choose a reason for hiding this comment

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

I left a couple of comments, but nothing blocking!

@andreybutov
Copy link
Collaborator Author

@BethanyBerkowitz

it might be helpful if this error included Did you install Pods? (or something similar).

You're right! I added the additional info to the error message. Thanks!

@andreybutov
Copy link
Collaborator Author

@subzero10 I think this PR is good to go.

@subzero10 subzero10 merged commit ed10921 into honeybadger-io:master Sep 7, 2023
@subzero10
Copy link
Member

Done! This should be available with v6.2.0.

@andreybutov andreybutov deleted the hermes_sourcemaps branch September 7, 2023 22:19
@andreybutov andreybutov restored the hermes_sourcemaps branch September 7, 2023 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants