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.

Sign up for GitHub

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

Crash the debug app when accessing a non-SDK API #13354

Open
wants to merge 3 commits into
base: trunk
Choose a base branch
from
+25 −0
Open

Crash the debug app when accessing a non-SDK API #13354

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6bd5d5a
Add a handler to force crash the app when accessing a non-SDK API
hichamboushaba Jan 20, 2025
b7c1061
Fix detekt issue
hichamboushaba Jan 20, 2025
004c2ea
Make sure the NonSdk violation is logged with correct level and stack…
hichamboushaba Jan 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions WooCommerce/src/debug/kotlin/com/woocommerce/android/NonSdkApiViolationHandler.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package com.woocommerce.android

import android.os.Build
import android.os.Process
import android.os.StrictMode.OnVmViolationListener
import android.os.strictmode.NonSdkApiUsedViolation
import android.os.strictmode.Violation
import android.util.Log
import androidx.annotation.RequiresApi
import kotlin.system.exitProcess

@RequiresApi(Build.VERSION_CODES.P)
@Suppress("MagicNumber")
class NonSdkApiViolationHandler : OnVmViolationListener {
override fun onVmViolation(v: Violation) {
kidinov marked this conversation as resolved.
Show resolved Hide resolved
if (v !is NonSdkApiUsedViolation) return
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it make sense to crash on all violations in debugging or are there some that we don't want too?

Copy link
Member Author

Choose a reason for hiding this comment

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

Personally I won't do this, IMO not all violations are worth an urgent fix, so it's case by case, so if the app crashes, then it'll block the other developer's workflows.
The Non-SDK is the exception because Google are fighting their usage which could lead to production crashes if it goes unnoticed.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe the fact that you found violations now when the code/libraries were added a long time ago, and nobody noticed that, may prove the point that early detection is good?

Log.e(
NonSdkApiViolationHandler::class.simpleName,
"Non-SDK API violation detected: ${v.message}\n${v.stackTraceToString()}"
)
Process.killProcess(Process.myPid())
exitProcess(10)
}
}
1 change: 1 addition & 0 deletions WooCommerce/src/debug/kotlin/com/woocommerce/android/WooCommerceDebug.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ class WooCommerceDebug : WooCommerce() {
.apply {
if (SystemVersionUtils.isAtLeastP()) {
detectNonSdkApiUsage()
penaltyListener(mainExecutor, NonSdkApiViolationHandler())
}
}
.build()
Expand Down
Loading