-
Notifications
You must be signed in to change notification settings - Fork 17
feat: send user agent header #77
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
base: main
Are you sure you want to change the base?
feat: send user agent header #77
Conversation
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.
Looks good overall! Couple questions.
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.
Is it possible to somehow mock the version so it can be seen in at least one test? All current tests only verify unknown is added in the header.
| private fun getSdkVersion(): String { | ||
| return try { | ||
| // Try to get version from BuildConfig | ||
| val buildConfigClass = Class.forName("com.flagsmith.kotlin.BuildConfig") | ||
| val versionField = buildConfigClass.getField("VERSION_NAME") | ||
| versionField.get(null) as String | ||
| } catch (e: Exception) { | ||
| // Fallback to hardcoded version if BuildConfig is not available | ||
| "unknown" | ||
| } | ||
| } |
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.
Is the Android machinery used to obtain the Flagsmith client version more reliable than iOS'? I'm asking considering the equivalent pull request to iOS (Flagsmith/flagsmith-ios-client#95) falls back to a hardcoded version string when obtaining the lib version fails, and I wonder why the same strategy isn't used here as well. (I'm not asking we implement it)
Fixes https://foresight.slite.com/app/docs/vY7NnrbUCj35XE#ccb66bd3-d784-4e5d-bf6e-8559bb0a76d6
Previous review and subsequent fixes on the Foresight side: foresightmobile#1
Description
Added a User-Agent HTTP header to network calls. Uses the package's version at runtime, if it is not available sends it as "unknown"
Also added a PR template, once this merged new PRs will come with this PR template.
Regression Test Recommendations
Test that User-Agent header is present in all HTTP requests with correct format: flagsmith-swift-ios-sdk/
Test fallback to "unknown" when version is not discoverable at runtime
Validate User-Agent appears in both REST API calls and SSE connections
Type of Change
✨ New feature (non-breaking change which adds functionality)
🛠️ Bug fix (non-breaking change which fixes an issue)
❌ Breaking change (fix or feature that would cause existing functionality to change)
🧹 Code refactor
✅ Build configuration change
📝 Documentation
🗑️ Chore
Summary by CodeRabbit
New Features
Adds a dynamic User-Agent header to network requests that reflects the SDK/app version when available.
Real-time updates initialization now accepts Android context for more reliable behavior on devices.
Tests
New unit tests validating User-Agent composition and that the header is sent across various request types and failure scenarios.