Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

feat(expo): Add RNSentrySDK APIs support to @sentry/react-native/expo plugin#4633

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
antonis wants to merge35 commits intocapture-app-start-errors-v7
base:capture-app-start-errors-v7
Choose a base branch
Loading
fromantonis/4625-expo-useNativeInit

Conversation

@antonis
Copy link
Contributor

@antonisantonis commentedMar 7, 2025•
edited
Loading

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

Based onfeat: Capture app start errors before JS
Based on#5470

📜 Description

Adds RNSentrySDK APIs support to @sentry/react-native/expo plugin by importing sentry and adding injectingRNSentrySDK.init/start in the Android MainApplication (Kotlin or Java) or AppDelegate (Objective-C or Swift).

This feature is opt-out to enable it setuseNativeInit totrue in your@sentry/react-native/expo plugin configuration.

"plugins":[["@sentry/react-native/expo",{"useNativeInit":true}],

💡 Motivation and Context

Fixes#4625

💚 How did you test it?

CI, Manual

📝 Checklist

  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII ifsendDefaultPII is enabled
  • I updated the docs if needed.
  • I updated the wizard if needed.
  • All tests passing
  • No breaking changes

🔮 Next steps

#skip-changelog

Lucienzera reacted with heart emoji
@github-actions
Copy link
Contributor

github-actionsbot commentedMar 7, 2025•
edited
Loading

Messages
📖Do not forget to updateSentry-docs with your feature once the pull request gets approved.

Generated by 🚫dangerJS against1c2a7b7

@github-actions
Copy link
Contributor

github-actionsbot commentedMar 7, 2025•
edited
Loading

iOS (legacy) Performance metrics 🚀

 PlainWith SentryDiff
Startup time1205.92 ms1206.60 ms0.69 ms
Size3.44 MiB4.67 MiB1.23 MiB

Baseline results on branch: capture-app-start-errors-v7

Startup times

RevisionPlainWith SentryDiff
60d1e83+dirty1207.79 ms1207.35 ms-0.44 ms
f26d7a8+dirty1209.49 ms1207.54 ms-1.95 ms

App size

RevisionPlainWith SentryDiff
60d1e83+dirty3.41 MiB4.67 MiB1.26 MiB
f26d7a8+dirty3.44 MiB4.67 MiB1.23 MiB

Previous results on branch: antonis/4625-expo-useNativeInit

Startup times

RevisionPlainWith SentryDiff
2c56c66+dirty1210.14 ms1211.94 ms1.80 ms

App size

RevisionPlainWith SentryDiff
2c56c66+dirty3.41 MiB4.67 MiB1.26 MiB

@github-actions
Copy link
Contributor

github-actionsbot commentedMar 7, 2025•
edited
Loading

iOS (new) Performance metrics 🚀

 PlainWith SentryDiff
Startup time1217.39 ms1216.65 ms-0.73 ms
Size3.44 MiB4.67 MiB1.23 MiB

Baseline results on branch: capture-app-start-errors-v7

Startup times

RevisionPlainWith SentryDiff
60d1e83+dirty1201.87 ms1204.61 ms2.74 ms
f26d7a8+dirty1227.33 ms1220.67 ms-6.66 ms

App size

RevisionPlainWith SentryDiff
60d1e83+dirty3.41 MiB4.67 MiB1.26 MiB
f26d7a8+dirty3.44 MiB4.67 MiB1.23 MiB

Previous results on branch: antonis/4625-expo-useNativeInit

Startup times

RevisionPlainWith SentryDiff
2c56c66+dirty1226.29 ms1223.27 ms-3.02 ms

App size

RevisionPlainWith SentryDiff
2c56c66+dirty3.41 MiB4.67 MiB1.26 MiB

@github-actions
Copy link
Contributor

github-actionsbot commentedMar 7, 2025•
edited
Loading

Android (legacy) Performance metrics 🚀

 PlainWith SentryDiff
Startup time405.06 ms416.68 ms11.62 ms
Size43.75 MiB48.08 MiB4.32 MiB

Baseline results on branch: capture-app-start-errors-v7

Startup times

RevisionPlainWith SentryDiff
f26d7a8+dirty571.36 ms637.92 ms66.56 ms
60d1e83+dirty396.72 ms390.04 ms-6.68 ms

App size

RevisionPlainWith SentryDiff
f26d7a8+dirty43.75 MiB48.08 MiB4.32 MiB
60d1e83+dirty43.75 MiB48.08 MiB4.32 MiB

Previous results on branch: antonis/4625-expo-useNativeInit

Startup times

RevisionPlainWith SentryDiff
2c56c66+dirty507.32 ms544.50 ms37.18 ms

App size

RevisionPlainWith SentryDiff
2c56c66+dirty43.75 MiB48.08 MiB4.32 MiB

@github-actions
Copy link
Contributor

github-actionsbot commentedMar 7, 2025•
edited
Loading

Android (new) Performance metrics 🚀

 PlainWith SentryDiff
Startup time402.61 ms441.25 ms38.64 ms
Size43.94 MiB48.90 MiB4.96 MiB

Baseline results on branch: capture-app-start-errors-v7

Startup times

RevisionPlainWith SentryDiff
f26d7a8+dirty380.15 ms407.84 ms27.68 ms
60d1e83+dirty406.16 ms422.83 ms16.67 ms

App size

RevisionPlainWith SentryDiff
f26d7a8+dirty43.94 MiB48.90 MiB4.96 MiB
60d1e83+dirty43.94 MiB48.90 MiB4.96 MiB

Previous results on branch: antonis/4625-expo-useNativeInit

Startup times

RevisionPlainWith SentryDiff
2c56c66+dirty375.54 ms401.17 ms25.63 ms

App size

RevisionPlainWith SentryDiff
2c56c66+dirty43.94 MiB48.90 MiB4.96 MiB

@antonisantonis linked an issueMar 7, 2025 that may beclosed by this pull request
@antonisantonis marked this pull request as ready for reviewMarch 7, 2025 16:22
Copy link
Collaborator

@lucas-zimermanlucas-zimerman left a comment

Choose a reason for hiding this comment

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

Overall the PR looks good and thank you for the tests!
I still think we could fine tune a bit more the android part and after it we could get ready for merge

antonis reacted with thumbs up emoji
antonisand others added2 commitsApril 4, 2025 11:01
Co-authored-by: LucasZF <lucas-zimerman1@hotmail.com>
@antonis
Copy link
ContributorAuthor

Note: TheBuild & Test / Type Check Typescript 3.8 (pull_request) failure should be fixed when we merge#4673 frommain

Co-authored-by: Krystof Woldrich <31292499+krystofwoldrich@users.noreply.github.com>
@krystofwoldrich
Copy link
Contributor

I'm currently updating the base branch with the latest main since there was quite a lot of development that was not synced to the feature branch.

antonis reacted with thumbs up emoji

@antonis
Copy link
ContributorAuthor

I'm currently updating the base branch with the latest main since there was quite a lot of development that was not synced to the feature branch.

Thank you for the heads up@krystofwoldrich 🙇
I'll update this branch after that along with the review feedback.

@krystofwoldrichkrystofwoldrichforce-pushed thecapture-app-start-errors branch 3 times, most recently from2363742 to588ba6dCompareJune 12, 2025 15:26
Copy link
Contributor

@krystofwoldrichkrystofwoldrich left a comment

Choose a reason for hiding this comment

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

Thank you for the fixes. 🚀 It looks great!


###Features

- Add RNSentrySDK APIs support to@sentry/react-native/expo plugin ([#4633](https://github.com/getsentry/sentry-react-native/pull/4633))
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit. It would be nice to include an example code snippet and a small summary of what will the flag do.

@lucas-zimerman
Copy link
Collaborator

@sentry review

@lucas-zimerman
Copy link
Collaborator

@antonis should we restart this PR?

@antonis
Copy link
ContributorAuthor

@antonis should we restart this PR?

I don't expect this to have major conflicts with main/v7.

- Resolved conflicts by separating imports from logger and version modules- Preserved useNativeInit functionality from PR#4633- Fixed optional chain expressions in withSentryAndroid and withSentryIOS- Updated test mocks to use logger instead of utils
@antonisantonis changed the base branch fromcapture-app-start-errors tocapture-app-start-errors-v7December 15, 2025 15:11
@antonisantonis added the ready-to-mergeTriggers the full CI test suite labelDec 15, 2025
}

exportfunctionmodifyAppDelegate(config:ExpoConfig):ExpoConfig{
returnwithAppDelegate(config,asyncconfig=>{
Copy link

Choose a reason for hiding this comment

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

Bug: ThemodifyAppDelegate function incorrectly returns aPromise<ExpoConfig> instead ofExpoConfig due to anasync callback, breaking the synchronous call chain for iOS native initialization.
Severity: HIGH | Confidence: High

🔍Detailed Analysis

ThemodifyAppDelegate function is declared to return anExpoConfig but it uses anasync callback withwithAppDelegate, which causes the function to actually return aPromise<ExpoConfig>. The return value, stored inappDelegateCfc, is then used as if it were a synchronousExpoConfig object and passed towithDangerousMod. This occurs whenuseNativeInit is true. Passing a promise instead of the expected configuration object towithDangerousMod will cause the Sentry iOS native initialization to fail.

💡Suggested Fix

Remove theasync keyword from the callback passed towithAppDelegate inside themodifyAppDelegate function. This will make the function returnExpoConfig synchronously, matching its type signature and the Android implementation.

🤖Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AIagent.Verify if this is a real issue. If it is, propose a fix; if not, explain why it's notvalid.Location: packages/core/plugin/src/withSentryIOS.ts#L90Potential issue: The `modifyAppDelegate` function is declared to return an `ExpoConfig`but it uses an `async` callback with `withAppDelegate`, which causes the function toactually return a `Promise<ExpoConfig>`. The return value, stored in `appDelegateCfc`,is then used as if it were a synchronous `ExpoConfig` object and passed to`withDangerousMod`. This occurs when `useNativeInit` is true. Passing a promise insteadof the expected configuration object to `withDangerousMod` will cause the Sentry iOSnative initialization to fail.

Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID:7592441

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@seer-by-sentryseer-by-sentry[bot]seer-by-sentry[bot] left review comments

@lucas-zimermanlucas-zimermanlucas-zimerman approved these changes

@alwxalwxAwaiting requested review from alwxalwx is a code owner

+2 more reviewers

@krystofwoldrichkrystofwoldrichkrystofwoldrich approved these changes

@sentrysentry[bot]sentry[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

ready-to-mergeTriggers the full CI test suite

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AddRNSentrySDK APIs support to@sentry/react-native/expo plugin

4 participants

@antonis@krystofwoldrich@lucas-zimerman
[8]ページ先頭
©2009-2025 Movatter.jp