feat: add separate flutter_stripe_identity package#2319
Conversation
Create a standalone Flutter federated plugin for Stripe Identity Verification, following the stripe-identity-react-native pattern. This approach keeps Identity separate from the main flutter_stripe package, avoiding conflicts with the React Native codebase sync. Package structure: - stripe_identity: Main facade (flutter_stripe_identity) - stripe_identity_platform_interface: Abstract interface + models - stripe_identity_ios: iOS native implementation (StripeIdentity SPM) - stripe_identity_android: Android native implementation - stripe_identity_web: Web stub (throws WebUnsupportedError) API: ```dart final result = await StripeIdentity.instance.presentIdentityVerificationSheet( verificationSessionId: 'vs_xxx', ephemeralKeySecret: 'ek_xxx', ); ``` Closes flutter-stripe#1819
📝 WalkthroughWalkthroughA Flutter plugin package architecture is introduced to enable Stripe Identity verification across iOS and Android. The implementation comprises a platform-agnostic public API, shared Dart model interfaces, native platform-specific implementations in Kotlin and Swift with method channel communication, and a comprehensive example application demonstrating the verification workflow. Changes
Sequence DiagramsequenceDiagram
participant App as Flutter App
participant StripeIdentity as StripeIdentity<br/>(Dart)
participant MethodChannel as Method Channel
participant Native as Native Platform<br/>(Kotlin/Swift)
participant Sheet as Verification Sheet<br/>(Stripe SDK)
App->>StripeIdentity: presentIdentityVerificationSheet(sessionId,<br/>ephemeralKey, brandLogo?)
StripeIdentity->>MethodChannel: invoke presentIdentityVerificationSheet<br/>with params JSON
MethodChannel->>Native: route method call to platform handler
Native->>Native: validate params & optionally decode logo
Native->>Sheet: create & present IdentityVerificationSheet
Sheet->>Sheet: user captures ID/selfie or cancels
Sheet-->>Native: return verification result
Native->>Native: map result to structured response<br/>(completed/canceled/failed)
Native-->>MethodChannel: send result dictionary
MethodChannel-->>StripeIdentity: receive native response
StripeIdentity->>StripeIdentity: deserialize to IdentityVerificationResult
StripeIdentity-->>App: return sealed result union
App->>App: pattern match on result type<br/>and update UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Actionable comments posted: 8
🤖 Fix all issues with AI agents
In `@packages/stripe_identity_android/android/build.gradle`:
- Around line 4-55: Replace the floating Stripe Identity version by pinning
ext.stripe_version to an exact tested release (e.g., change ext.stripe_version =
'22.2.+' to ext.stripe_version = '22.7.0' or another specific 22.x version) so
the dependency resolution for implementation
"com.stripe:identity:$stripe_version" becomes reproducible; update any related
documentation or comments if needed and ensure the chosen version is reflected
where ext.stripe_version is referenced.
In
`@packages/stripe_identity_android/android/src/main/kotlin/com/stripe/flutter/stripe_identity/StripeIdentityAndroidPlugin.kt`:
- Around line 72-86: The IdentityVerificationSheetManager instance retains
Activity/IdentityVerificationSheet references causing leaks; clear it when the
plugin loses its Activity. In onDetachedFromActivityForConfigChanges() and
onDetachedFromActivity() set the plugin's identityManager reference to null (in
addition to activity = null) — or if IdentityVerificationSheetManager exposes a
cleanup method, call that then null the identityManager — so the old manager and
its Activity/IdentityVerificationSheet references can be garbage collected.
In `@packages/stripe_identity_web/lib/stripe_identity_web.dart`:
- Line 6: There is a duplicate definition of the WebUnsupportedError class: keep
the module-level export 'src/web_unsupported_error.dart' and delete the inline
class definition named WebUnsupportedError (currently at lines 32-42) so the
symbol is provided only via the exported file; alternatively, if you prefer the
inline definition, remove the export statement `export
'src/web_unsupported_error.dart';` and keep the inline WebUnsupportedError, but
do not leave both.
In
`@packages/stripe_identity/example/android/gradle/wrapper/gradle-wrapper.properties`:
- Line 5: Update the Gradle distribution URL in gradle-wrapper.properties to a
fixed release that includes the bugfix (change the current distributionUrl value
"gradle-8.14-all.zip" to "gradle-8.14.1-all.zip" or later) so AGP code coverage
support is not broken; commit the updated gradle-wrapper.properties with the new
distributionUrl pointing to gradle-8.14.1-all.zip (or a newer 8.14.x release).
In `@packages/stripe_identity/example/ios/Runner.xcodeproj/project.pbxproj`:
- Around line 359-579: Remove the hard-coded DEVELOPMENT_TEAM entries by
deleting the lines "DEVELOPMENT_TEAM = 85S6FJH649;" from the
XCBuildConfiguration buildSettings blocks that configure the Runner target (the
blocks where PRODUCT_BUNDLE_IDENTIFIER =
com.stripe.flutter.stripeIdentityExample and the test-runner blocks), i.e. the
Debug/Release/Profile XCBuildConfiguration entries shown in the diff (the
buildSettings sections under those XCBuildConfiguration objects); leave the key
unset so signing can be configured locally or via xcconfig/Xcode instead.
In `@packages/stripe_identity/example/ios/Runner/Info.plist`:
- Around line 5-47: The Info.plist is missing the NSCameraUsageDescription key
which causes App Store rejection and runtime crashes when using
StripeIdentity.presentIdentityVerificationSheet(); add an
NSCameraUsageDescription entry to the plist with a clear, user-facing string
(e.g., "Needed to capture identity document and selfie for verification") so the
app prompts for camera permission before starting the identity verification
flow; ensure the new key is placed alongside other
CFBundle/UISupportedInterfaceOrientations keys in the same Info.plist block.
In
`@packages/stripe_identity/example/lib/screens/identity_verification_screen.dart`:
- Around line 79-91: The code trusts the HTTP response and JSON structure—add a
response.statusCode check after the post to throw a clear error (including
statusCode and response.body) for non-2xx responses, wrap jsonDecode in a
try/catch to surface JSON parse errors, and validate presence and types of
data['id'] and data['ephemeral_key_secret'] before casting; replace direct casts
(sessionId = data['id'] as String, ephemeralKeySecret =
data['ephemeral_key_secret'] as String) with safe extraction (null checks or
toString()) and throw a descriptive exception if required fields are missing or
wrong. Use the existing variables/endpoint names (response,
kApiUrl/create-verification-session, data, sessionId, ephemeralKeySecret) to
locate where to add these checks.
In `@packages/stripe_identity/example/lib/widgets/loading_button.dart`:
- Around line 43-60: The finally block in _loadFuture calls setState without
checking whether the State is still mounted, which can throw if the widget was
disposed during the async operation; update the finally block to only call
setState when mounted is true (i.e., if (mounted) setState(() { _isLoading =
false; })), and ensure any UI-specific operations done after the await (like
using ScaffoldMessenger) are either performed before potential disposal or
guarded by mounted as well.
🧹 Nitpick comments (10)
packages/stripe_identity/example/README.md (1)
1-16: Consider enhancing the README with Stripe Identity-specific instructions.This is a generic Flutter template README. For an example app demonstrating Stripe Identity verification, consider adding:
- Prerequisites (Stripe dashboard setup, Identity enabled)
- Backend server setup instructions for creating VerificationSessions
- Required permissions (camera access)
- How to configure the API URL in
config.dartpackages/stripe_identity/example/lib/screens/identity_verification_screen.dart (1)
67-72: Status color detection relies on string matching.Using
_status!.contains('completed')is fragile—if status messages change, colors won't match. Consider storing the result type separately or using an enum.♻️ Alternative approach using result type
// Store result type alongside message IdentityVerificationSheetResult? _result; String? _status; Color _getStatusColor() { return switch (_result) { IdentityVerificationCompleted() => Colors.green, IdentityVerificationCanceled() => Colors.orange, IdentityVerificationFailed() => Colors.red, null => Colors.grey, }; }packages/stripe_identity_ios/ios/stripe_identity_ios/Package.swift (1)
8-22: Consider usingfrominstead ofexactfor the Stripe iOS SDK dependency to allow patch updates.iOS 13.0 compatibility is confirmed for StripeIdentity in v25.0.1. However, using
exact: "25.0.1"blocks patch updates; v25.2.0 is already available. If reproducibility is not a strict requirement, switching tofrom: "25.0.1"allows bug and security fixes to be automatically applied.Suggested tweak
- dependencies: [ - .package(url: "https://github.com/stripe/stripe-ios-spm", exact: "25.0.1") - ], + dependencies: [ + .package(url: "https://github.com/stripe/stripe-ios-spm", from: "25.0.1") + ],packages/stripe_identity/example/android/gradle.properties (1)
1-1: Consider reducing JVM memory settings for the example app.The configured JVM arguments allocate 8GB heap and 4GB metaspace, which is excessive for a simple example application and may fail on developer machines with limited RAM. Typical Flutter projects use more conservative defaults.
💡 Suggested reduction
-org.gradle.jvmargs=-Xmx8G -XX:MaxMetaspaceSize=4G -XX:ReservedCodeCacheSize=512m -XX:+HeapDumpOnOutOfMemoryError +org.gradle.jvmargs=-Xmx4G -XX:MaxMetaspaceSize=512m -XX:ReservedCodeCacheSize=512m -XX:+HeapDumpOnOutOfMemoryErrorpackages/stripe_identity_android/lib/stripe_identity_android.dart (1)
1-3: Consider using the full path for clarity.The path uses
...which is less precise than the iOS counterpart. For consistency and easier navigation, consider specifying the full path.📝 Suggested improvement
// This file is intentionally empty. // The Android implementation is handled natively via Kotlin. -// See android/src/main/kotlin/.../StripeIdentityAndroidPlugin.kt +// See android/src/main/kotlin/com/stripe/flutter/stripe_identity/StripeIdentityAndroidPlugin.ktpackages/stripe_identity_platform_interface/pubspec.yaml (1)
7-9: Consider SDK/Flutter version constraints.The SDK constraint
>=3.8.1and Flutter constraint>=3.22.0are quite recent and may limit adoption for projects on older stable Flutter versions. If broader compatibility is desired, consider relaxing these constraints.packages/stripe_identity_android/android/src/main/kotlin/com/stripe/flutter/stripe_identity/IdentityVerificationSheetManager.kt (1)
48-77: Extract Stripe error codes from the throwable instead of hardcoding"failed".The
verificationResult.throwablein the Failed case may contain a Stripe exception with structured error codes (viaStripeError.code). Extracting these codes—with a fallback to"failed"for non-Stripe exceptions—would preserve diagnostic detail and help callers distinguish failure types without losing information.packages/stripe_identity/lib/flutter_stripe_identity.dart (1)
3-10: Consider exportingWebUnsupportedErrorfor consumers.Consumers may want to catch
WebUnsupportedErrorspecifically when handling web platform limitations. Currently, they would need to import fromstripe_identity_webdirectly.♻️ Suggested addition
export 'package:stripe_identity_platform_interface/stripe_identity_platform_interface.dart' show IdentityVerificationSheetParams, IdentityVerificationResult, IdentityVerificationCompleted, IdentityVerificationCanceled, IdentityVerificationFailed, IdentityVerificationError; +export 'package:stripe_identity_web/stripe_identity_web.dart' + show WebUnsupportedError; + export 'src/stripe_identity.dart';packages/stripe_identity/example/android/settings.gradle.kts (1)
3-8: Potential crash iflocal.propertiesdoesn't exist.The
file("local.properties").inputStream()call will throw aFileNotFoundExceptionif the file doesn't exist, which can happen in fresh clones before running Flutter commands. Consider adding a more graceful error message or existence check.♻️ Suggested improvement
val flutterSdkPath = run { val properties = java.util.Properties() - file("local.properties").inputStream().use { properties.load(it) } + val localPropertiesFile = file("local.properties") + require(localPropertiesFile.exists()) { + "local.properties not found. Run 'flutter pub get' first." + } + localPropertiesFile.inputStream().use { properties.load(it) } val flutterSdkPath = properties.getProperty("flutter.sdk") require(flutterSdkPath != null) { "flutter.sdk not set in local.properties" } flutterSdkPath }packages/stripe_identity_platform_interface/lib/src/method_channel_stripe_identity.dart (1)
13-34: Consider wrapping the method channel call for consistent error handling.The
invokeMethodcall can throwPlatformExceptionorMissingPluginExceptionif the native side fails or the plugin isn't registered. Currently, these exceptions propagate to the caller, which may be intentional for debugging. However, for consistency with the null-result handling pattern (returningIdentityVerificationFailed), you might consider catching these exceptions and mapping them toIdentityVerificationResult.failed.If propagating exceptions is the intended design (to distinguish platform setup issues from verification failures), this is fine as-is.
♻️ Optional: Wrap exceptions for consistent result type
`@override` Future<IdentityVerificationResult> presentIdentityVerificationSheet( IdentityVerificationSheetParams params, ) async { + final Map<dynamic, dynamic>? result; + try { - final result = await _methodChannel.invokeMethod<Map<dynamic, dynamic>>( + result = await _methodChannel.invokeMethod<Map<dynamic, dynamic>>( - 'presentIdentityVerificationSheet', - {'params': params.toJson()}, - ); + 'presentIdentityVerificationSheet', + {'params': params.toJson()}, + ); + } on PlatformException catch (e) { + return IdentityVerificationResult.failed( + error: IdentityVerificationError( + code: e.code, + message: e.message ?? 'Platform error occurred', + ), + ); + } if (result == null) {
| final response = await http.post( | ||
| Uri.parse('$kApiUrl/create-verification-session'), | ||
| headers: {'Content-Type': 'application/json'}, | ||
| ); | ||
|
|
||
| final data = jsonDecode(response.body); | ||
|
|
||
| if (data['error'] != null) { | ||
| throw Exception(data['error']); | ||
| } | ||
|
|
||
| final sessionId = data['id'] as String; | ||
| final ephemeralKeySecret = data['ephemeral_key_secret'] as String; |
There was a problem hiding this comment.
Add HTTP status code validation and safer JSON parsing.
The code assumes a successful HTTP response and directly parses the body. If the server returns a non-2xx status (e.g., 500), or malformed JSON, this could throw unclear exceptions.
🛡️ Proposed defensive improvements
final response = await http.post(
Uri.parse('$kApiUrl/create-verification-session'),
headers: {'Content-Type': 'application/json'},
);
+ if (response.statusCode < 200 || response.statusCode >= 300) {
+ throw Exception('Server error: ${response.statusCode}');
+ }
+
final data = jsonDecode(response.body);
if (data['error'] != null) {
throw Exception(data['error']);
}
- final sessionId = data['id'] as String;
- final ephemeralKeySecret = data['ephemeral_key_secret'] as String;
+ final sessionId = data['id'] as String?;
+ final ephemeralKeySecret = data['ephemeral_key_secret'] as String?;
+
+ if (sessionId == null || ephemeralKeySecret == null) {
+ throw Exception('Invalid server response: missing session data');
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| final response = await http.post( | |
| Uri.parse('$kApiUrl/create-verification-session'), | |
| headers: {'Content-Type': 'application/json'}, | |
| ); | |
| final data = jsonDecode(response.body); | |
| if (data['error'] != null) { | |
| throw Exception(data['error']); | |
| } | |
| final sessionId = data['id'] as String; | |
| final ephemeralKeySecret = data['ephemeral_key_secret'] as String; | |
| final response = await http.post( | |
| Uri.parse('$kApiUrl/create-verification-session'), | |
| headers: {'Content-Type': 'application/json'}, | |
| ); | |
| if (response.statusCode < 200 || response.statusCode >= 300) { | |
| throw Exception('Server error: ${response.statusCode}'); | |
| } | |
| final data = jsonDecode(response.body); | |
| if (data['error'] != null) { | |
| throw Exception(data['error']); | |
| } | |
| final sessionId = data['id'] as String?; | |
| final ephemeralKeySecret = data['ephemeral_key_secret'] as String?; | |
| if (sessionId == null || ephemeralKeySecret == null) { | |
| throw Exception('Invalid server response: missing session data'); | |
| } |
🤖 Prompt for AI Agents
In
`@packages/stripe_identity/example/lib/screens/identity_verification_screen.dart`
around lines 79 - 91, The code trusts the HTTP response and JSON structure—add a
response.statusCode check after the post to throw a clear error (including
statusCode and response.body) for non-2xx responses, wrap jsonDecode in a
try/catch to surface JSON parse errors, and validate presence and types of
data['id'] and data['ephemeral_key_secret'] before casting; replace direct casts
(sessionId = data['id'] as String, ephemeralKeySecret =
data['ephemeral_key_secret'] as String) with safe extraction (null checks or
toString()) and throw a descriptive exception if required fields are missing or
wrong. Use the existing variables/endpoint names (response,
kApiUrl/create-verification-session, data, sessionId, ephemeralKeySecret) to
locate where to add these checks.
| Future<void> _loadFuture() async { | ||
| setState(() { | ||
| _isLoading = true; | ||
| }); | ||
| final scaffoldMessenger = ScaffoldMessenger.of(context); | ||
|
|
||
| try { | ||
| await widget.onPressed!(); | ||
| } catch (e, s) { | ||
| log(e.toString(), error: e, stackTrace: s); | ||
| scaffoldMessenger.showSnackBar(SnackBar(content: Text('Error $e'))); | ||
| rethrow; | ||
| } finally { | ||
| setState(() { | ||
| _isLoading = false; | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
Check mounted before setState in the finally block.
If the widget is unmounted during the async operation (e.g., user navigates away), calling setState will throw an exception. Since errors are rethrown, the widget could be disposed before finally executes.
🛡️ Proposed fix
} finally {
+ if (mounted) {
setState(() {
_isLoading = false;
});
+ }
}🤖 Prompt for AI Agents
In `@packages/stripe_identity/example/lib/widgets/loading_button.dart` around
lines 43 - 60, The finally block in _loadFuture calls setState without checking
whether the State is still mounted, which can throw if the widget was disposed
during the async operation; update the finally block to only call setState when
mounted is true (i.e., if (mounted) setState(() { _isLoading = false; })), and
ensure any UI-specific operations done after the await (like using
ScaffoldMessenger) are either performed before potential disposal or guarded by
mounted as well.
remonh87
left a comment
There was a problem hiding this comment.
Looks good thank you this is a great addition some tasks that still needs to be done before we can ship it:
- Add readme to all the packages
- I do not think we should keep a stripe_identity_web package as this would suggest that we support web which we do not. Let's remove it and as a service we can throw in stripe identity package if someone calls it from the web.
I also would like to let @jonasbark check it from the native bindings side
| @@ -0,0 +1,24 @@ | |||
| name: stripe_identity_platform_interface | |||
| description: Platform interface for Stripe Identity Verification | |||
| version: 1.0.0 | |||
There was a problem hiding this comment.
maybe for the first version we should consider 0.1.0? What do you think @jonasbark ?
| default_package: stripe_identity_android | ||
| ios: | ||
| default_package: stripe_identity_ios | ||
| web: |
There was a problem hiding this comment.
let's not add web support if we cannot promise something working
|
@realmeylisdev can you address the comments in the pr? Also do you need help? If we do not hear from you in the next few weeks I will close the pr due to inactivity |
- Remove stripe_identity_web package entirely (per maintainer request);
add kIsWeb guard with UnsupportedError in main package instead
- Fix iOS sheet lifetime bug: retain IdentityVerificationSheet as
instance property to prevent ARC deallocation before callback
- Fix iOS thread safety: wrap completion handler in DispatchQueue.main.async
- Fix iOS parameter validation: use reject() instead of resolve() for errors
- Add iOS brandLogo image size validation (5 MB limit)
- Fix Android memory leak: clear identityManager after result delivery
and on activity detach
- Fix Android concurrent invocation guard (already_presenting error)
- Fix Android NPE: null-safe access on throwable.message/localizedMessage
- Fix Android parameter validation: use result.error() instead of
result.success(errorMap)
- Document @SuppressLint("RestrictedApi") justification
- Remove unused Android deps (lifecycle-runtime-ktx, kotlinx-coroutines)
- Version all packages to 0.1.0 (per maintainer suggestion)
- Add NSCameraUsageDescription to example Info.plist
- Remove hardcoded DEVELOPMENT_TEAM from Xcode project
- Add READMEs to all packages
- Improve example: type-safe status colors, const keywords
|
@remonh87 I've addressed all your review feedback:
Additionally fixed several bugs found during review:
Ready for re-review when you get a chance! |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/stripe_identity/example/lib/widgets/loading_button.dart (1)
51-59:⚠️ Potential issue | 🟡 MinorGuard UI updates with
mountedafter async gap (still unresolved).Line 56 calls
setStateinfinallywithout checking widget lifecycle. If the widget is disposed whileonPressedis running, this can throw.🛡️ Suggested fix
} finally { - setState(() { - _isLoading = false; - }); + if (mounted) { + setState(() { + _isLoading = false; + }); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stripe_identity/example/lib/widgets/loading_button.dart` around lines 51 - 59, The finally block calls setState(() { _isLoading = false; }) without checking widget lifecycle; guard that UI update by checking if (mounted) before calling setState so the widget hasn't been disposed during the async onPressed flow. Locate the async handler (onPressed) and the finally that references _isLoading and wrap the setState call in a mounted check to avoid calling setState on a disposed State object.
🧹 Nitpick comments (3)
packages/stripe_identity_ios/ios/stripe_identity_ios/Sources/stripe_identity_ios/StripeIdentityPlugin.swift (2)
5-9: Update documentation to reflect Flutter context.The typealias documentation refers to "JS promise" which appears to be copy-paste from React Native code. Consider updating to Flutter-appropriate terminology.
📝 Suggested documentation update
-/// Closure that bridge modules use to resolve the JS promise waiting for a result. +/// Closure used to resolve the Flutter method call with a success response. public typealias PromiseResolveBlock = (Any?) -> Void -/// Closure that bridge modules use to reject the JS promise waiting for a result. +/// Closure used to reject the Flutter method call with an error response. public typealias PromiseRejectBlock = (_ code: String, _ message: String, _ error: Error?) -> Void🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stripe_identity_ios/ios/stripe_identity_ios/Sources/stripe_identity_ios/StripeIdentityPlugin.swift` around lines 5 - 9, Update the doc comments for the typealiases PromiseResolveBlock and PromiseRejectBlock to reference Flutter/Dart futures instead of "JS promise" (this is copy-paste from React Native). Locate the declarations of PromiseResolveBlock and PromiseRejectBlock and change their comments to mention resolving/rejecting the Flutter/Dart Future or completing a platform result callback, providing Flutter-appropriate wording while keeping the parameter descriptions intact.
92-105: Silent failure when UIImage creation fails from valid base64 data.If
Data(base64Encoded:)succeeds butUIImage(data:)returns nil (e.g., corrupted or unsupported image format), the code silently continues without the brand logo. Consider logging a warning or returning an error to help developers debug configuration issues.📝 Optional: Add warning for failed image creation
if imageData.count > kMaxBrandLogoBytes { reject( "invalid_params", "brandLogo image data exceeds maximum size of \(kMaxBrandLogoBytes / (1024 * 1024)) MB", nil ) return } if let image = UIImage(data: imageData) { configuration = IdentityVerificationSheet.Configuration(brandLogo: image) + } else { + // Log warning: valid base64 but invalid image data + NSLog("[StripeIdentity] Warning: brandLogo base64 decoded but could not create UIImage") } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stripe_identity_ios/ios/stripe_identity_ios/Sources/stripe_identity_ios/StripeIdentityPlugin.swift` around lines 92 - 105, The code silently ignores failures when UIImage(data:) returns nil after Data(base64Encoded:) succeeds; update the brand logo handling in StripeIdentityPlugin (look for params["brandLogo"], brandLogoBase64, Data(base64Encoded:), UIImage(data:), and IdentityVerificationSheet.Configuration) to detect a nil UIImage and either call reject(...) with a clear "invalid_brand_logo" error (including a short message) or at minimum log a warning via the plugin logger before continuing; ensure you still enforce the kMaxBrandLogoBytes check and only set configuration = IdentityVerificationSheet.Configuration(brandLogo: image) when image is non-nil.packages/stripe_identity/lib/src/stripe_identity.dart (1)
84-89: Consider removing redundant web check or aligning error types.The
stripe_identity_webpackage already throwsWebUnsupportedErrorwhen invoked. This check provides early fail-fast behavior, which is reasonable, but uses a different error type (UnsupportedErrorvsWebUnsupportedError). Consider either:
- Removing this check to let the web platform handle it consistently, or
- Using the same
WebUnsupportedErrortype for consistency🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/stripe_identity/lib/src/stripe_identity.dart` around lines 84 - 89, The current early fail-fast uses kIsWeb and throws UnsupportedError; change it to throw WebUnsupportedError instead (importing it from the stripe_identity_web package or re-export if available) so the thrown error type matches the platform package. Locate the kIsWeb check in stripe_identity.dart where the code currently throws UnsupportedError and replace the thrown type/message with WebUnsupportedError (preserving the descriptive message), or remove the kIsWeb block if you prefer to delegate entirely to stripe_identity_web—prefer using WebUnsupportedError for consistency if keeping the early check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/stripe_identity/example/lib/widgets/loading_button.dart`:
- Around line 51-54: The catch block in the _loadFuture handler currently logs,
shows a SnackBar via scaffoldMessenger, then rethrows the exception which can
become an unhandled framework error because the button onPressed caller doesn't
await _loadFuture; remove the rethrow and instead finish handling the error
locally: keep log(e, stackTrace: s) and scaffoldMessenger.showSnackBar(...) but
do NOT rethrow; also ensure any local loading state (e.g. whatever
_setLoading/_isLoading or the widget state updated by _loadFuture) is reset so
the button/UI is left in a consistent state after the error.
---
Duplicate comments:
In `@packages/stripe_identity/example/lib/widgets/loading_button.dart`:
- Around line 51-59: The finally block calls setState(() { _isLoading = false;
}) without checking widget lifecycle; guard that UI update by checking if
(mounted) before calling setState so the widget hasn't been disposed during the
async onPressed flow. Locate the async handler (onPressed) and the finally that
references _isLoading and wrap the setState call in a mounted check to avoid
calling setState on a disposed State object.
---
Nitpick comments:
In
`@packages/stripe_identity_ios/ios/stripe_identity_ios/Sources/stripe_identity_ios/StripeIdentityPlugin.swift`:
- Around line 5-9: Update the doc comments for the typealiases
PromiseResolveBlock and PromiseRejectBlock to reference Flutter/Dart futures
instead of "JS promise" (this is copy-paste from React Native). Locate the
declarations of PromiseResolveBlock and PromiseRejectBlock and change their
comments to mention resolving/rejecting the Flutter/Dart Future or completing a
platform result callback, providing Flutter-appropriate wording while keeping
the parameter descriptions intact.
- Around line 92-105: The code silently ignores failures when UIImage(data:)
returns nil after Data(base64Encoded:) succeeds; update the brand logo handling
in StripeIdentityPlugin (look for params["brandLogo"], brandLogoBase64,
Data(base64Encoded:), UIImage(data:), and
IdentityVerificationSheet.Configuration) to detect a nil UIImage and either call
reject(...) with a clear "invalid_brand_logo" error (including a short message)
or at minimum log a warning via the plugin logger before continuing; ensure you
still enforce the kMaxBrandLogoBytes check and only set configuration =
IdentityVerificationSheet.Configuration(brandLogo: image) when image is non-nil.
In `@packages/stripe_identity/lib/src/stripe_identity.dart`:
- Around line 84-89: The current early fail-fast uses kIsWeb and throws
UnsupportedError; change it to throw WebUnsupportedError instead (importing it
from the stripe_identity_web package or re-export if available) so the thrown
error type matches the platform package. Locate the kIsWeb check in
stripe_identity.dart where the code currently throws UnsupportedError and
replace the thrown type/message with WebUnsupportedError (preserving the
descriptive message), or remove the kIsWeb block if you prefer to delegate
entirely to stripe_identity_web—prefer using WebUnsupportedError for consistency
if keeping the early check.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e2056f20-dcd7-43e0-aea3-4611c4f257e7
📒 Files selected for processing (18)
packages/stripe_identity/README.mdpackages/stripe_identity/example/ios/Runner.xcodeproj/project.pbxprojpackages/stripe_identity/example/ios/Runner/Info.plistpackages/stripe_identity/example/lib/main.dartpackages/stripe_identity/example/lib/screens/identity_verification_screen.dartpackages/stripe_identity/example/lib/widgets/loading_button.dartpackages/stripe_identity/lib/src/stripe_identity.dartpackages/stripe_identity/pubspec.yamlpackages/stripe_identity_android/README.mdpackages/stripe_identity_android/android/build.gradlepackages/stripe_identity_android/android/src/main/kotlin/com/stripe/flutter/stripe_identity/IdentityVerificationSheetManager.ktpackages/stripe_identity_android/android/src/main/kotlin/com/stripe/flutter/stripe_identity/StripeIdentityAndroidPlugin.ktpackages/stripe_identity_android/pubspec.yamlpackages/stripe_identity_ios/README.mdpackages/stripe_identity_ios/ios/stripe_identity_ios/Sources/stripe_identity_ios/StripeIdentityPlugin.swiftpackages/stripe_identity_ios/pubspec.yamlpackages/stripe_identity_platform_interface/README.mdpackages/stripe_identity_platform_interface/pubspec.yaml
✅ Files skipped from review due to trivial changes (12)
- packages/stripe_identity_ios/README.md
- packages/stripe_identity_platform_interface/README.md
- packages/stripe_identity_platform_interface/pubspec.yaml
- packages/stripe_identity_ios/pubspec.yaml
- packages/stripe_identity_android/README.md
- packages/stripe_identity/pubspec.yaml
- packages/stripe_identity/README.md
- packages/stripe_identity/example/ios/Runner/Info.plist
- packages/stripe_identity_android/android/build.gradle
- packages/stripe_identity/example/lib/screens/identity_verification_screen.dart
- packages/stripe_identity/example/ios/Runner.xcodeproj/project.pbxproj
- packages/stripe_identity_android/pubspec.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/stripe_identity/example/lib/main.dart
- packages/stripe_identity_android/android/src/main/kotlin/com/stripe/flutter/stripe_identity/IdentityVerificationSheetManager.kt
- packages/stripe_identity_android/android/src/main/kotlin/com/stripe/flutter/stripe_identity/StripeIdentityAndroidPlugin.kt
| } catch (e, s) { | ||
| log(e.toString(), error: e, stackTrace: s); | ||
| scaffoldMessenger.showSnackBar(SnackBar(content: Text('Error $e'))); | ||
| rethrow; |
There was a problem hiding this comment.
Avoid rethrow from button handler path; it can become unhandled.
Line 54 rethrows after showing a snackbar. In this flow, _loadFuture is triggered by onPressed and is not externally awaited (see packages/stripe_identity/example/lib/screens/identity_verification_screen.dart Line 77-131), so this can bubble as an unhandled framework exception.
🔧 Suggested fix
} catch (e, s) {
log(e.toString(), error: e, stackTrace: s);
scaffoldMessenger.showSnackBar(SnackBar(content: Text('Error $e')));
- rethrow;
+ // Swallow here: error is already surfaced via SnackBar and log.
} finally {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| } catch (e, s) { | |
| log(e.toString(), error: e, stackTrace: s); | |
| scaffoldMessenger.showSnackBar(SnackBar(content: Text('Error $e'))); | |
| rethrow; | |
| } catch (e, s) { | |
| log(e.toString(), error: e, stackTrace: s); | |
| scaffoldMessenger.showSnackBar(SnackBar(content: Text('Error $e'))); | |
| // Swallow here: error is already surfaced via SnackBar and log. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/stripe_identity/example/lib/widgets/loading_button.dart` around
lines 51 - 54, The catch block in the _loadFuture handler currently logs, shows
a SnackBar via scaffoldMessenger, then rethrows the exception which can become
an unhandled framework error because the button onPressed caller doesn't await
_loadFuture; remove the rethrow and instead finish handling the error locally:
keep log(e, stackTrace: s) and scaffoldMessenger.showSnackBar(...) but do NOT
rethrow; also ensure any local loading state (e.g. whatever
_setLoading/_isLoading or the widget state updated by _loadFuture) is reset so
the button/UI is left in a consistent state after the error.
|
Any updates on this? |
remonh87
left a comment
There was a problem hiding this comment.
lookgs good to me thanks for applying the fixes. I will cook some CI runners and will make sure we will launch it
|
@jonasbark I think we can integrate it but would be good if you can double check it. |
Summary
This PR adds Stripe Identity Verification support as a separate federated plugin, following the maintainer's guidance in #2318 and the pattern used by stripe-identity-react-native.
Closes #1819
Why Separate Package?
Per @jonasbark's feedback on #2318:
This approach:
stripe-identity-react-nativePackage Structure
API Usage
Key Differences from #2318
flutter.stripe/paymentsflutter.stripe_identity/identityTest Plan
WebUnsupportedErrorTesting Requirements
Summary by CodeRabbit