Skip to content
This repository has been archived by the owner on Aug 30, 2023. It is now read-only.

Provide a way to avoid installation of Sentry's default UncaughtExceptionHandlerIntegration #382

Closed
techyourchance opened this issue Apr 30, 2020 · 5 comments
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@techyourchance
Copy link

It looks like Sentry always installs its own ExceptionHandler as default, even if I disable it in manifest and init in Application's onCreate(). As far as I can tell, there is no option to disable this functionality.

Use case:
I want to implement my own ExceptionHandler which will delegate to Android's default ExceptionHandler in some cases (without going through Sentry's one). However, since Sentry installs itself before I get a chance to grab Android's default, this use case can't be implemented.

What I tried:
I tried to iterate over Integrations list in Options when initializing and remove this integration manually, but got ConcurrentModificationException. Looks like Sentry iterates this list before the initialization is complete.

@techyourchance techyourchance changed the title Provide a way to disable Sentry's default UncaughtExceptionHandler Provide a way to disable Sentry's default UncaughtExceptionHandlerIntegration Apr 30, 2020
@techyourchance techyourchance changed the title Provide a way to disable Sentry's default UncaughtExceptionHandlerIntegration Provide a way to avoid installation of Sentry's default UncaughtExceptionHandlerIntegration Apr 30, 2020
@marandaneto marandaneto added the enhancement New feature or request label May 1, 2020
@triage-new-issues triage-new-issues bot removed the triage label May 1, 2020
@marandaneto
Copy link
Contributor

hey @techyourchance thanks for raising this issue, we definitely could improve things here, but there's a way of doing it.

1st, you'd need to disable the auto-init mode that uses a ContentProvider.

<meta-data android:name="io.sentry.auto-init" android:value="false" />

2nd, you'd need to remove the UncaughtExceptionHandlerIntegration from the integrations list.

For that, you'd need to init Sentry manually:

    SentryAndroid.init(
      getApplicationContext(),
      options -> {
        for (final Iterator<Integration> iterator = options.getIntegrations().iterator(); iterator.hasNext(); ) {
          final Integration integration = iterator.next();
          if (integration instanceof UncaughtExceptionHandlerIntegration) {
            iterator.remove();
          }
        }
      });

The ConcurrentModificationException is not a bug, but a Java thing.

so this would work if you are removing an item from an ArrayList but using the correct approach, eg using an iterator as I did, using a ListIterator, Collect to an aux. list and remove it after iterating thru all the items, Collection.removeIf is minApi >= 24, etc...

I'm just guessing here that you've tried something like this:

        for (final Integration item : options.getIntegrations()) {
          if (item instanceof UncaughtExceptionHandlerIntegration) {
            options.getIntegrations().remove(item);
          }
        }

Basically you'd be removing the item that you are iterating and this would cause the issue.

We feel your pain as this would be the first try of everyone, we'll document it properly, thanks.

@marandaneto marandaneto added the documentation Improvements or additions to documentation label May 1, 2020
@marandaneto
Copy link
Contributor

PR #384 is open with a proposal

@marandaneto marandaneto self-assigned this May 1, 2020
@techyourchance
Copy link
Author

Thanks for acknowledging this issue.
Quick note:
As I wrote, I disabled the init in manifest. Also, I understand how list iteration works and why I got that exception. What bothers me is the fact that it has been thrown.
As far as I understand, it means that there has been some entity which iterated over this list while I modified it. I'm not sure why would that happen, but it feels risky. Even if there is a "technically safe" way to remove an item from that list, since the list is already consumed by some entity, it might happen that the removal is too late. Worst case scenario if the timing of the removal matters and I plant race condition into client's code.
I think there should be a simple configuration option to disable default exception handler. Even better - implement decorator pattern in addition to full "disable". This way clients would be able to use your handler, but also plug their own and let yours delegate to theirs (instead of Android's default).
Lastly, you should not prevent the clients from getting a reference to Android's default exception handler if they'll ever need that.

@marandaneto
Copy link
Contributor

the SDK only iterates thru that list again after the callback is ran on SentryAndroid.init(context) {} and on Sentry.close().

I don't see how this could be possible. if it does, it's a bug but I can't reproduce it nor find it, my own assumption here is what I've posted in my previous comment.
That callback is exactly for this reason, it's the last chance for the user to change options, the SDK is not even enabled nor doing anything at that point.

It'd be very helpful if you share with us a sample repro that we can reproduce this issue, I've tried and could not, it was only possible when I was not using an iterator.

The decoration pattern for plugging a specific exception handler but still using Sentry is a good idea, we'll think about it, it might be useful, thanks for that.

If you decide to init the SDK manually, you have full control of the default exception handler though, so it's up to you on which order you want to init the SDKs, you can achieve that without a single line of code on our end.

Our auto-init function relies on ContentProvider, and as you might know, this class doesn't guarantee the order of execution along with other providers.

@marandaneto
Copy link
Contributor

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants