feat(android): Add nativeStackAndroid support to NativeLinkedErrors#6278
feat(android): Add nativeStackAndroid support to NativeLinkedErrors#6278lucas-zimerman wants to merge 4 commits into
Conversation
Captures the JVM stack trace attached to rejected native module promises as a linked exception, so the Java cause of a rejected promise is reported alongside the JS error. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
🤖 This preview updates automatically when you update the PR. |
|
… into lz/nativeStackAndroid
| if (nativeStackAndroidException && linkedErrors.length + 1 < limit) { | ||
| linkedErrors.push(nativeStackAndroidException); | ||
| } |
There was a problem hiding this comment.
Bug: The limit check linkedErrors.length + 1 < limit is an off-by-one error. It incorrectly prevents adding the final nativeStackAndroid exception when the cause chain is at maximum depth.
Severity: LOW
Suggested Fix
Change the condition linkedErrors.length + 1 < limit to linkedErrors.length < limit. This will correctly allow adding one more exception, up to the specified limit.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/core/src/js/integrations/nativelinkederrors.ts#L58-L60
Potential issue: An off-by-one error exists in the limit check for linked errors. The
condition `linkedErrors.length + 1 < limit` prevents adding the `nativeStackAndroid`
exception when the `cause` chain already contains `limit - 1` items. In this scenario,
the check evaluates to `limit < limit`, which is false, causing the native exception to
be silently dropped even though adding it would not exceed the `limit`. This results in
incomplete error data when the exception chain is exactly at the maximum allowed depth.
Did we get this right? 👍 / 👎 to inform future reviews.
|
|
||
| ### Features | ||
|
|
||
| - Add `nativeStackAndroid` support to `NativeLinkedErrors`, capturing the JVM stack trace of rejected native module promises as a linked exception ([#6278](https://github.com/getsentry/sentry-react-native/issues/6278)) |
There was a problem hiding this comment.
nit
| - Add `nativeStackAndroid` support to `NativeLinkedErrors`, capturing the JVM stack trace of rejected native module promises as a linked exception ([#6278](https://github.com/getsentry/sentry-react-native/issues/6278)) | |
| - Add `nativeStackAndroid` support to `NativeLinkedErrors`, capturing the JVM stack trace of rejected native module promises as a linked exception ([#6278](https://github.com/getsentry/sentry-react-native/pull/6278)) |
| ### Features | ||
|
|
||
| - Add `nativeStackAndroid` support to `NativeLinkedErrors`, capturing the JVM stack trace of rejected native module promises as a linked exception ([#6278](https://github.com/getsentry/sentry-react-native/issues/6278)) | ||
|
|
There was a problem hiding this comment.
nit: Let's also remove the extra line
antonis
left a comment
There was a problem hiding this comment.
LGTM 🚀
Let's fix the changelog before merging
Captures the JVM stack trace attached to rejected native module promises as a linked exception, so the Java cause of a rejected promise is reported alongside the JS error.
📢 Type of change
📜 Description
Android Promise rejections were previously captured by Sentry, but lacked stacktrace on it.
💡 Motivation and Context
To close #3257
💚 How did you test it?
Locally with addition of promise rejection button on sample app
Before
https://sentry-sdks.sentry.io/issues/7547068866

After

https://sentry-sdks.sentry.io/issues/7547000366
📝 Checklist
sendDefaultPIIis enabled🔮 Next steps
Close #3257