Skip to content

ref(logs): Intercept Native SDK logs#1249

Open
lucas-zimerman wants to merge 15 commits into
mainfrom
lz/intercept-js-errors
Open

ref(logs): Intercept Native SDK logs#1249
lucas-zimerman wants to merge 15 commits into
mainfrom
lz/intercept-js-errors

Conversation

@lucas-zimerman
Copy link
Copy Markdown
Collaborator

@lucas-zimerman lucas-zimerman commented May 18, 2026

📢 Type of change

  • Bugfix
  • New feature
  • Enhancement
  • Refactoring

📜 Description

Based on getsentry/sentry-react-native#5622
This PR redirects native Sentry logs to be visible along with the JavaScript logs

💡 Motivation and Context

Allow users to see all SDK logs on a unified way.

💚 How did you test it?

Android logs
image

iOS
image

📝 Checklist

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

🔮 Next steps

Close #1093

lucas-zimerman and others added 6 commits May 16, 2026 02:29
…gs to JS

Migrated from sentry-react-native#5622. Adds an `onNativeLog` option that
lets users surface native SDK logs (e.g. HTTP 413 transport errors) in the
browser console when `debug: true` is set.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…fyListeners

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@lucas-zimerman lucas-zimerman changed the title Lz/intercept js errors ref(logs): Intercept Native errors May 18, 2026
lucas-zimerman and others added 2 commits May 19, 2026 00:20
Co-authored-by: Claude <noreply@anthropic.com>
@lucas-zimerman lucas-zimerman changed the title ref(logs): Intercept Native errors ref(logs): Intercept Native SDK logs May 18, 2026
lucas-zimerman and others added 4 commits May 19, 2026 00:31
@lucas-zimerman lucas-zimerman marked this pull request as ready for review May 18, 2026 23:56
@lucas-zimerman lucas-zimerman requested a review from antonis May 18, 2026 23:56
Comment thread src/client.ts
if (nativeOptions.debug) {
const logHandler = nativeOptions.onNativeLog ?? defaultNativeLogHandler;
setupNativeLogListener(logHandler);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Re-init leaves log forwarding

Medium Severity

After init runs with debug: true, a later init without close() never tears down native log forwarding. sdkInit only calls setupNativeLogListener when debug is true and never calls stopNativeLogListener when it is false, and setupNativeLogListener ignores further calls while a listener is already registered.

Additional Locations (2)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 99baf27. Configure here.

Comment thread Package.resolved Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

q: Is the move of Package.resolved moved to root intentional?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

not really, let me rollback

Copy link
Copy Markdown
Contributor

@antonis antonis left a comment

Choose a reason for hiding this comment

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

Other than the package move LGTM

lucas-zimerman

This comment was marked as spam.

lucas-zimerman and others added 2 commits May 22, 2026 20:58
SDK libraries should not commit Package.resolved — SPM ignores it from
dependencies and only the consuming app's lock file is used.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@@ -1,23 +0,0 @@
{
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Tooling opted to not have this file

@lucas-zimerman lucas-zimerman requested a review from antonis May 22, 2026 20:04
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

There are 2 total unresolved issues (including 1 from previous review).

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 19cae50. Configure here.

androidLogger.log(level, message, args);

String formattedMessage =
(args == null || args.length == 0) ? message : String.format(message, args);
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Unprotected String.format can throw uncaught exception

Medium Severity

The String.format(message, args) calls in the log methods are not wrapped in try-catch. The underlying AndroidLogger.log() internally catches IllegalFormatException from malformed format strings (this was a known crash, sentry-java#1985), but the subsequent String.format call in CapSentryLogger lacks this same protection. If format specifiers in message don't match args, an IllegalFormatException will propagate uncaught into the SDK internals.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 19cae50. Configure here.

Copy link
Copy Markdown
Contributor

@antonis antonis left a comment

Choose a reason for hiding this comment

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

LGTM 🎸

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SDK Handling HTTP 413 [Capacitor]

2 participants