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: Prepare for Android alpha + refactor the SDK to be used in conjunction with @sentry/angular #35

Merged
merged 11 commits into from
Jun 2, 2021

Conversation

jennmueng
Copy link
Member

@jennmueng jennmueng commented May 24, 2021

Refactors the SDK to be used in conjunction with @sentry/angular, users will need to install both @sentry/capacitor and @sentry/angular if using angular, or just @sentry/capacitor on its own. Currently only Android is still supported.

As this PR contains tons of removals and files, for ease of review, the most notable changes are in wrapper.ts, sdk.ts, and SentryCapacitor.java.

Notable Changes

  • New init strategy as described below. The SDK is now more lightweight and just extends a sibling SDK.
  • Adds support for Sentry.nativeCrash().
  • Catches all unexpected errors unlike before.

Usage:

For example, when using with angular:

Install via

yarn add @sentry/capacitor @sentry/angular

Set up via

// app.module.ts

import * as Sentry from "@sentry/capacitor";
import { init as sentryAngularInit, createErrorHandler }  from "@sentry/angular";

// Init by passing the sibling SDK's init as the second parameter.
Sentry.init({
  dsn: "...",
}, sentryAngularInit);

// Attach the Sentry error handler
@NgModule({
  providers: [
    {
      provide: ErrorHandler,
      useValue: createErrorHandler(),
    },
    // ...
  ],
  // ...
})

In theory installing alongside @sentry/react and @sentry/vue should work, but I have yet to test this so we don't officially support them yet. #40, #41

Source Maps

When using with ionic angular and building via ionic build, you can upload the www directory with sentry-cli to upload the sourcemaps to de-minify the stack trace.

sentry-cli releases files "{RELEASE}" upload-sourcemaps ./www --rewrite --dist "{DIST}"

In the near future we should introduce auto source map uploading at bulildtime: #39

@jennmueng jennmueng changed the title feat: Refactor the SDK to be used in conjunction with @sentry/angular feat: Prepare for Android alpha + refactor the SDK to be used in conjunction with @sentry/angular May 24, 2021
@jennmueng jennmueng added this to In progress in kanban via automation Jun 2, 2021
@jennmueng jennmueng self-assigned this Jun 2, 2021
@jennmueng jennmueng added this to In Progress in Mobile Platform Team Archived Jun 2, 2021
@jennmueng jennmueng moved this from In progress to To do in kanban Jun 2, 2021
@jennmueng jennmueng moved this from To do to In progress in kanban Jun 2, 2021
@jennmueng jennmueng marked this pull request as ready for review June 2, 2021 12:17
@jennmueng jennmueng requested a review from a team June 2, 2021 12:18
@jennmueng jennmueng moved this from In Progress to Waiting for Review in Mobile Platform Team Archived Jun 2, 2021
@@ -8,7 +8,9 @@
android:label="@string/app_name"
android:roundIcon="@mipmap/ic_launcher_round"
android:supportsRtl="true"
android:theme="@style/AppTheme">
android:theme="@style/AppTheme"
android:usesCleartextTraffic="true">
Copy link
Contributor

Choose a reason for hiding this comment

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

its a sample so it does not matter but are we calling an HTTP endpoint instead of HTTPS?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I had to add this to get the capacitor dev server to work

Copy link
Member

@bruno-garcia bruno-garcia left a comment

Choose a reason for hiding this comment

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

one nit but LGTM
:shipit:

@@ -62,14 +62,15 @@ public void startWithOptions(final PluginCall capOptions) {
SentryAndroid.init(
this.getContext(),
options -> {
if (capOptions.getData().has("debug") && capOptions.getBoolean("debug")) {
options.setDebug(true);
logger.setLevel(Level.INFO);
Copy link
Member

Choose a reason for hiding this comment

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

We usually have DEBUG by default. User can lower it if it's too noisy, but out of the box we give as much information as we can

Copy link
Contributor

Choose a reason for hiding this comment

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

logger.setLevel(Level.INFO); is only within the SentryCapacitor.java, its not the Android SDK logger, so its fine

@@ -62,14 +62,15 @@ public void startWithOptions(final PluginCall capOptions) {
SentryAndroid.init(
Copy link
Contributor

Choose a reason for hiding this comment

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

side note: bump the Android SDK to the latest GA, 5.0.0

Copy link
Member Author

Choose a reason for hiding this comment

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

Sounds good, after this PR I'll make one to bump Android and another to bump JS

@jennmueng jennmueng merged commit 0cd8cd3 into main Jun 2, 2021
@jennmueng jennmueng deleted the jenn/android-alpha branch June 2, 2021 16:02
kanban automation moved this from In progress to Done Jun 2, 2021
Mobile Platform Team Archived automation moved this from Waiting for Review to Done Jun 2, 2021
jennmueng added a commit that referenced this pull request Jun 3, 2021
Updates the readme with the new installation for angular and the init method from #35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants