-
-
Notifications
You must be signed in to change notification settings - Fork 465
[QA] Fix potential ANRs due to default integrations #3778
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
Changes from 1 commit
d7e2ffc
fd2bc62
7561fe4
0870f09
e2cb750
4ffe903
41b11a9
40cbfea
0a924a0
e524d07
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -31,12 +31,12 @@ | |
| public final class NetworkBreadcrumbsIntegration implements Integration, Closeable { | ||
|
|
||
| private final @NotNull Context context; | ||
|
|
||
| private final @NotNull BuildInfoProvider buildInfoProvider; | ||
|
|
||
| private final @NotNull ILogger logger; | ||
| private volatile boolean isClosed; | ||
| private @Nullable SentryOptions options; | ||
|
|
||
| @TestOnly @Nullable NetworkBreadcrumbsNetworkCallback networkCallback; | ||
| @TestOnly @Nullable volatile NetworkBreadcrumbsNetworkCallback networkCallback; | ||
|
|
||
| public NetworkBreadcrumbsIntegration( | ||
| final @NotNull Context context, | ||
|
|
@@ -63,40 +63,71 @@ public void register(final @NotNull IHub hub, final @NotNull SentryOptions optio | |
| "NetworkBreadcrumbsIntegration enabled: %s", | ||
| androidOptions.isEnableNetworkEventBreadcrumbs()); | ||
|
|
||
| this.options = options; | ||
|
|
||
| if (androidOptions.isEnableNetworkEventBreadcrumbs()) { | ||
|
|
||
| // The specific error is logged in the ConnectivityChecker method | ||
| if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.LOLLIPOP) { | ||
| networkCallback = null; | ||
| logger.log(SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration requires Android 5+"); | ||
| if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.N) { | ||
| logger.log(SentryLevel.DEBUG, "NetworkCallbacks need Android N+."); | ||
| return; | ||
| } | ||
|
|
||
| networkCallback = | ||
| new NetworkBreadcrumbsNetworkCallback(hub, buildInfoProvider, options.getDateProvider()); | ||
| final boolean registered = | ||
| AndroidConnectionStatusProvider.registerNetworkCallback( | ||
| context, logger, buildInfoProvider, networkCallback); | ||
|
|
||
| // The specific error is logged in the ConnectivityChecker method | ||
| if (!registered) { | ||
| networkCallback = null; | ||
| logger.log(SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration not installed."); | ||
| return; | ||
| try { | ||
| options | ||
| .getExecutorService() | ||
| .submit( | ||
| new Runnable() { | ||
| @Override | ||
| public void run() { | ||
| // in case integration is closed before the task is executed, simply return | ||
| final @Nullable NetworkBreadcrumbsNetworkCallback callback = networkCallback; | ||
| if (isClosed || callback == null) { | ||
|
||
| networkCallback = null; | ||
| return; | ||
| } | ||
|
|
||
| final boolean registered = | ||
| AndroidConnectionStatusProvider.registerNetworkCallback( | ||
| context, logger, buildInfoProvider, callback); | ||
| if (registered) { | ||
| logger.log(SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration installed."); | ||
| addIntegrationToSdkVersion(getClass()); | ||
| } else { | ||
| logger.log(SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration not installed."); | ||
| // The specific error is logged by AndroidConnectionStatusProvider | ||
| } | ||
| } | ||
| }); | ||
| } catch (Throwable t) { | ||
| logger.log(SentryLevel.ERROR, "Error submitting NetworkBreadcrumbsIntegration task.", t); | ||
| } | ||
| logger.log(SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration installed."); | ||
| addIntegrationToSdkVersion(getClass()); | ||
| } | ||
| } | ||
|
|
||
| @Override | ||
| public void close() throws IOException { | ||
| if (networkCallback != null) { | ||
| AndroidConnectionStatusProvider.unregisterNetworkCallback( | ||
| context, logger, buildInfoProvider, networkCallback); | ||
| logger.log(SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration remove."); | ||
| isClosed = true; | ||
|
|
||
| try { | ||
| Objects.requireNonNull(options, "Options is required") | ||
| .getExecutorService() | ||
| .submit( | ||
| () -> { | ||
| final @Nullable NetworkBreadcrumbsNetworkCallback callback = networkCallback; | ||
| if (callback != null) { | ||
| AndroidConnectionStatusProvider.unregisterNetworkCallback( | ||
| context, logger, buildInfoProvider, callback); | ||
| logger.log(SentryLevel.DEBUG, "NetworkBreadcrumbsIntegration removed."); | ||
| } | ||
| networkCallback = null; | ||
| }); | ||
| } catch (Throwable t) { | ||
| logger.log(SentryLevel.ERROR, "Error submitting NetworkBreadcrumbsIntegration task.", t); | ||
| } | ||
| networkCallback = null; | ||
| } | ||
|
|
||
| @SuppressLint("ObsoleteSdkInt") | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've changed this to
N, as ourAndroidConnectionStatusProvider.registerNetworkCallbackitself only supports >=N