Refactor getUserAgent method and improve logging#6
Refactor getUserAgent method and improve logging#6yzheka wants to merge 1 commit intoOpenpanel-dev:mainfrom
Conversation
On some devices WebView(context).settings.userAgentString can throw NullPointerException, preventing for fallback to System.getProperty("http.agent"). With this, if webview fails, system property will be fetched. If system property fetch fails too then no user-agent header will be added, letting system to decide
📝 WalkthroughWalkthroughModified the OpenPanel SDK to add verbose logging configuration via new Boolean fields, introduce a platform detection method, and refactor user-agent header initialization from eager to conditional based on nullability. Minor logging formatting adjustments applied across the codebase. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
OpenPanelSDK/src/main/java/com/dev/openpanelsdk/OpenPanel.kt (3)
262-262:⚠️ Potential issue | 🟠 MajorPublic
isAndroidEnvironment()always returnsfalse— this is an Android SDK.This method is exposed as a public API but hard-codes
false. Meanwhile, the companion object (Line 46) has a private implementation that actually performs the check but is never called. This will mislead any consumer relying on this method.Either delegate to the companion's implementation, or remove both methods if they serve no purpose in an Android-only SDK.
36-36:⚠️ Potential issue | 🟡 MinorNit: remove the inline comment
// Add this line.This appears to be a review instruction left in the code. Same applies to Line 79 and Line 369.
406-408:⚠️ Potential issue | 🟠 MajorVerbose logging leaks
openpanel-client-secretheader value.When
verboseis enabled, all headers (includingopenpanel-client-secret) are logged vialogVerboseat Line 408. This could expose secrets in logcat output. Consider redacting sensitive headers.
🤖 Fix all issues with AI agents
In `@OpenPanelSDK/src/main/java/com/dev/openpanelsdk/OpenPanel.kt`:
- Around line 56-60: The getUserAgent() implementation currently creates a
WebView via WebView(context) to read settings.userAgentString which leaks native
resources; replace that WebView usage with
WebSettings.getDefaultUserAgent(context) (available since API 17, safe given
minSdk 24) and keep the existing fallback to System.getProperty("http.agent") so
getUserAgent() first tries WebSettings.getDefaultUserAgent(context) inside
runCatching and then .getOrNull() ?: runCatching {
System.getProperty("http.agent") }.getOrNull(); ensure no WebView is
instantiated anywhere in getUserAgent().
🧹 Nitpick comments (2)
OpenPanelSDK/src/main/java/com/dev/openpanelsdk/OpenPanel.kt (2)
46-53: Dead code: privateisAndroidEnvironment()in companion object is never called.This method is unreferenced. If it's intended for future use, consider removing it until needed (YAGNI), or wire it into the public instance method on Line 262.
264-268:logVerboseinOpenPanelis defined but never called.The
verboseflag is properly threaded through toApi, butOpenPanel's ownlogVerboseis unused. Consider adding verbose logging tosend(),getUserAgent(), orinitto make the feature useful at the SDK level, or remove it until needed.
| private fun getUserAgent(): String? = runCatching { | ||
| WebView(context).settings.userAgentString | ||
| }.getOrNull() ?: runCatching { | ||
| System.getProperty("http.agent") | ||
| }.getOrNull() |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Android WebSettings.getDefaultUserAgent static method API level documentation
💡 Result:
WebSettings.getDefaultUserAgent(Context context) is a static method in android.webkit.WebSettings that returns the default User-Agent string used by a WebView. It was added in API level 17 (Android 4.2). An individual WebView instance may use a different UA if you override it via WebSettings.setUserAgentString(String). [1][2]
Docs (API reference): [2]
Signature
- Java:
public static String getDefaultUserAgent (Context context)[2] - Kotlin:
open static fun getDefaultUserAgent(context: Context!): String![1]
🏁 Script executed:
cat -n OpenPanelSDK/src/main/java/com/dev/openpanelsdk/OpenPanel.kt | sed -n '50,75p'Repository: Openpanel-dev/kotlin-sdk
Length of output: 1045
🏁 Script executed:
fd -e gradle -e properties | head -20Repository: Openpanel-dev/kotlin-sdk
Length of output: 85
🏁 Script executed:
find . -name "build.gradle*" -o -name "gradle.properties" | xargs grep -l "minSdkVersion\|compileSdk" 2>/dev/null | head -5Repository: Openpanel-dev/kotlin-sdk
Length of output: 122
🏁 Script executed:
cat OpenPanelSDK/build.gradle.kts | grep -i "minsdk\|compilesdk\|targetversion" -A 1Repository: Openpanel-dev/kotlin-sdk
Length of output: 112
🏁 Script executed:
cat app/build.gradle.kts | grep -i "minsdk\|compilesdk\|targetversion" -A 1Repository: Openpanel-dev/kotlin-sdk
Length of output: 134
Replace WebView(context) instantiation with WebSettings.getDefaultUserAgent(context) to avoid resource leak.
The current code instantiates a full WebView instance just to retrieve the user-agent string, and the instance is never destroyed — this leaks native resources. WebSettings.getDefaultUserAgent(context) is a static method (available since API 17) that returns the default UA string without allocating a WebView. Since the SDK's minSdk is 24, this is fully compatible.
Proposed fix
- private fun getUserAgent(): String? = runCatching {
- WebView(context).settings.userAgentString
- }.getOrNull() ?: runCatching {
+ private fun getUserAgent(): String? = runCatching {
+ WebSettings.getDefaultUserAgent(context)
+ }.getOrNull() ?: runCatching {
System.getProperty("http.agent")
}.getOrNull()🤖 Prompt for AI Agents
In `@OpenPanelSDK/src/main/java/com/dev/openpanelsdk/OpenPanel.kt` around lines 56
- 60, The getUserAgent() implementation currently creates a WebView via
WebView(context) to read settings.userAgentString which leaks native resources;
replace that WebView usage with WebSettings.getDefaultUserAgent(context)
(available since API 17, safe given minSdk 24) and keep the existing fallback to
System.getProperty("http.agent") so getUserAgent() first tries
WebSettings.getDefaultUserAgent(context) inside runCatching and then
.getOrNull() ?: runCatching { System.getProperty("http.agent") }.getOrNull();
ensure no WebView is instantiated anywhere in getUserAgent().
There was a problem hiding this comment.
@yzheka, glad you find it helpful! This change will prevent native resource leaks while still providing the same functionality. Let me know if you need any assistance with the implementation.
(\_/)
( •_•)
/ > 🔧
1 participant
On some devices WebView(context).settings.userAgentString can throw NullPointerException, preventing for fallback to System.getProperty("http.agent"). With this, if webview fails, system property will be fetched. If system property fetch fails too then no user-agent header will be added, letting system to decide
Summary by CodeRabbit
New Features
Improvements