Repository: getsentry/sentry-capacitor (ts)

Issue:
Disabling the SDK still results in native errors being sent
### Overview

Disabling the SDK still results in native errors being sent.

There are 2 scenarios:
- Scenario 1 is that `enabled` and `enableNative` flags at initialization do not stop Android native errors from being sent.
- Scenario 2 is that disabling the SDK at runtime does not stop Android and iOS native errors from being sent.

This occurs on hybrid iOS and hybrid Android which use the native SDKs to process and send the event (might be relevant). It is not an issue on Web wherein it works fine (it also works fine using `@sentry/browser` directly).

A repro can be found [here](https://github.com/KevinKelchen/sentry-capacitor-enable-sdk-issue).

### Environment

How do you use Sentry?
Sentry SaaS (sentry.io)

Which SDK and version?
Sentry Capacitor - 0.4.3

### Steps to Reproduce

See [Steps to Reproduce](https://github.com/KevinKelchen/sentry-capacitor-enable-sdk-issue#steps-to-reproduce).

### Expected Result

- Scenario 1
  - Expected setting `enabled: false` (and perhaps even `enableNative: false`) to be sufficient at stopping native errors from being sent on Android. `enabled: false` alone works as expected on iOS. `enabled: false` alone works as expected for Android *JavaScript* errors.
- Scenario 2
  - Expected setting `getCurrentHub().getClient().getOptions().enabled = false` (and perhaps even `(getCurrentHub().getClient().getOptions() as any).enableNative` with `(getCurrentHub().getClient().getOptions() as any).enableNativeCrashHandling)` at runtime to be sufficient at stopping native errors from being sent on Android and iOS.
  - Related: If we are to use `getCurrentHub().getClient().getOptions()` to disable the SDK at runtime, it would be nice if it had TypeScript typings that reflects the changes in the Capacitor SDK such as the additional native-related properties which do appear to be present at runtime.

### Actual Result

- Scenario 1
  - Setting `enabled: false` (even with `enableNative: false`) results in native errors being sent on Android.
- Scenario 2
  - Setting `getCurrentHub().getClient().getOptions().enabled = false` (even with `(getCurrentHub().getClient().getOptions() as any).enableNative` and `(getCurrentHub().getClient().getOptions() as any).enableNativeCrashHandling)` at runtime results in native errors being sent on Android and iOS.
  - Related: Using `getCurrentHub().getClient().getOptions()` to disable the SDK at runtime does not have TypeScript typings that reflects the changes in the Capacitor SDK such as the additional native-related properties which do appear to be present at runtime.

The TypeScript typings enhancement could be its own issue, but I erred on the side of including it in here for now in case there's a preference to keep all of this SDK enable/disable stuff in one place. Also, if there's a better approach for disabling the SDK at runtime that results in not using the untyped native-related properties of those APIs, then our use case for them may go away. 🙂

### Final Thoughts

Thank you so much for taking the time to investigate and/or work on this issue! 😀

Pull request description:
disable native sdk when enablenative is false
Close #137

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1b7e6ad..90d66a5 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,7 +4,8 @@
 
 ### Fixes
 
-- remove sdkProcessingMetadata from native events ([#139](https://github.com/getsentry/sentry-capacitor/pull/139))
+- Disable native sdk when enabled/enablenative is set to false ([#145](https://github.com/getsentry/sentry-capacitor/pull/145))
+- Remove sdkProcessingMetadata from native events ([#139](https://github.com/getsentry/sentry-capacitor/pull/139))
 
 ## 0.4.3
 
diff --git a/src/sdk.ts b/src/sdk.ts
index 6c2b92b..76aaf5f 100644
--- a/src/sdk.ts
+++ b/src/sdk.ts
@@ -26,12 +26,14 @@ export function init<O>(
     ...options,
   };
 
-  if (NATIVE.platform === 'web') {
+  if (finalOptions.enabled === false ||
+    NATIVE.platform === 'web') {
     finalOptions.enableNative = false;
     finalOptions.enableNativeNagger = false;
   } else {
-    finalOptions.enableNative = true;
-    finalOptions.enableNativeNagger = true;
+    // keep the original value if user defined it.
+    finalOptions.enableNativeNagger ??= true;
+    finalOptions.enableNative ??= true;
   }
 
   const capacitorHub = new Hub(undefined, new CapacitorScope());
@@ -46,7 +48,7 @@ export function init<O>(
             .replace(/^https?:\/\/localhost/, '')
             .replace(/^ng:\/\//, '')
             .replace(/^capacitor:\/\/localhost/, '');
-          
+
           const isNativeFrame = frame.filename === '[native code]' || frame.filename === 'native';
           const isReachableHost = /^https?:\/\//.test(frame.filename);
 


Test patch:
diff --git a/test/sdk.test.ts b/test/sdk.test.ts
index e31181f..daf62ff 100644
--- a/test/sdk.test.ts
+++ b/test/sdk.test.ts
@@ -103,4 +103,56 @@ describe('SDK Init', () => {
       expect.objectContaining(test[4]),
     );
   });
+
+
+  test('Does not Enable Native SDK if enabled is false', async () => {
+    NATIVE.platform = 'ios';
+
+    init({
+      enabled: false
+    }, (capacitorOptions: CapacitorOptions) => {
+      expect(capacitorOptions.enableNative).toBe(false);
+      expect(capacitorOptions.enableNativeNagger).toBe(false);
+    });
+  });
+
+  test('Keep enableNative if set on mobile', async () => {
+    NATIVE.platform = 'ios';
+
+    init({
+      enabled: true,
+      enableNative: false
+    }, (capacitorOptions: CapacitorOptions) => {
+      expect(capacitorOptions.enableNative).toBe(false);
+      expect(capacitorOptions.enableNativeNagger).toBe(true);
+      expect(capacitorOptions.enabled).toBe(true);
+    });
+  });
+
+  test('Keep enableNativeNagger if set on mobile', async () => {
+    NATIVE.platform = 'ios';
+
+    init({
+      enabled: true,
+      enableNative: false,
+      enableNativeNagger: false
+    }, (capacitorOptions: CapacitorOptions) => {
+      expect(capacitorOptions.enableNative).toBe(false);
+      expect(capacitorOptions.enableNativeNagger).toBe(false);
+      expect(capacitorOptions.enabled).toBe(true);
+    });
+  });
+
+  test('WEB has enableNative false by default', async () => {
+    NATIVE.platform = 'web';
+
+    init({
+      enabled: true,
+    }, (capacitorOptions: CapacitorOptions) => {
+      expect(capacitorOptions.enableNative).toBe(false);
+      expect(capacitorOptions.enableNativeNagger).toBe(false);
+      expect(capacitorOptions.enabled).toBe(true);
+    });
+  });
+
 });