feat: improve error handling, avoid hardcoding string error codes, use pigeon for type-saftey

Author: EchoEllet

packages/image_picker/image_picker_macos/example/integration_test/image_picker_test.dart

@@ -3,10 +3,8 @@
 // found in the LICENSE file.
 
 import 'package:example/main.dart';
-import 'package:flutter/services.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:image_picker_macos/image_picker_macos.dart';
-import 'package:image_picker_macos/src/messages.g.dart';
 import 'package:image_picker_platform_interface/image_picker_platform_interface.dart';
 import 'package:integration_test/integration_test.dart';
 
@@ -44,19 +42,5 @@ void main() {
             reason: 'Pressing the toggle button should update it correctly');
       },
     );
-    testWidgets(
-      'multi-video selection is not implemented',
-      (WidgetTester tester) async {
-        final ImagePickerApi hostApi = ImagePickerApi();
-        await expectLater(
-          hostApi.pickVideos(GeneralOptions(limit: 2)),
-          throwsA(predicate<PlatformException>(
-            (PlatformException e) =>
-                e.code == 'UNIMPLEMENTED' &&
-                e.message == 'Multi-video selection is not implemented',
-          )),
-        );
-      },
-    );
   });
 }

packages/image_picker/image_picker_macos/example/macos/Runner.xcodeproj/project.pbxproj

@@ -3,7 +3,7 @@
 	archiveVersion = 1;
 	classes = {
 	};
-	objectVersion = 60;
+	objectVersion = 54;
 	objects = {
 
 /* Begin PBXAggregateTarget section */

packages/image_picker/image_picker_macos/lib/image_picker_macos.dart

@@ -5,6 +5,7 @@
 import 'package:file_selector_macos/file_selector_macos.dart';
 import 'package:file_selector_platform_interface/file_selector_platform_interface.dart';
 import 'package:flutter/foundation.dart';
+import 'package:flutter/services.dart';
 import 'package:image_picker_platform_interface/image_picker_platform_interface.dart';
 
 import 'src/messages.g.dart';
@@ -47,8 +48,6 @@ class ImagePickerMacOS extends CameraDelegatingImagePickerPlatform {
   /// Supports picking an image, multi-image, video, media, and multiple media.
   bool useMacOSPHPicker = false;
 
-  // TODO(EchoEllet): avoid using @visibleForTesting per https://github.com/flutter/flutter/blob/master/docs/contributing/Style-guide-for-Flutter-repo.md#avoid-using-visiblefortesting
-
   /// Return `true` if the current macOS version supports [useMacOSPHPicker].
   ///
   /// The [PHPicker](https://developer.apple.com/documentation/photokit/phpickerviewcontroller)
@@ -144,6 +143,8 @@ class ImagePickerMacOS extends CameraDelegatingImagePickerPlatform {
             _imageOptionsToImageSelectionOptions(options),
             GeneralOptions(limit: 1),
           ))
+              .getSuccessOrThrow()
+              .filePaths
               .firstOrNull;
           if (imagePath == null) {
             return null;
@@ -183,7 +184,10 @@ class ImagePickerMacOS extends CameraDelegatingImagePickerPlatform {
       case ImageSource.gallery:
         if (await shouldUsePHPicker()) {
           final String? videoPath =
-              (await _hostApi.pickVideos(GeneralOptions(limit: 1))).firstOrNull;
+              (await _hostApi.pickVideos(GeneralOptions(limit: 1)))
+                  .getSuccessOrThrow()
+                  .filePaths
+                  .firstOrNull;
           if (videoPath == null) {
             return null;
           }
@@ -228,12 +232,14 @@ class ImagePickerMacOS extends CameraDelegatingImagePickerPlatform {
     MultiImagePickerOptions options = const MultiImagePickerOptions(),
   }) async {
     if (await shouldUsePHPicker()) {
-      final List<String> images = await _hostApi.pickImages(
+      final List<String> images = (await _hostApi.pickImages(
         _imageOptionsToImageSelectionOptions(options.imageOptions),
         GeneralOptions(
           limit: options.limit ?? 0,
         ),
-      );
+      ))
+          .getSuccessOrThrow()
+          .filePaths;
       return images.map((String imagePath) => XFile(imagePath)).toList();
     }
     const XTypeGroup typeGroup =
@@ -267,15 +273,17 @@ class ImagePickerMacOS extends CameraDelegatingImagePickerPlatform {
   @override
   Future<List<XFile>> getMedia({required MediaOptions options}) async {
     if (await shouldUsePHPicker()) {
-      final List<String> images = await _hostApi.pickMedia(
+      final List<String> images = (await _hostApi.pickMedia(
         MediaSelectionOptions(
           imageSelectionOptions:
               _imageOptionsToImageSelectionOptions(options.imageOptions),
         ),
         GeneralOptions(
           limit: options.limit ?? (options.allowMultiple ? 0 : 1),
         ),
-      );
+      ))
+          .getSuccessOrThrow()
+          .filePaths;
       return images.map((String mediaPath) => XFile(mediaPath)).toList();
     }
     const XTypeGroup typeGroup = XTypeGroup(
@@ -297,3 +305,47 @@ class ImagePickerMacOS extends CameraDelegatingImagePickerPlatform {
     return files;
   }
 }
+
+extension _ImagePickerResultExt on ImagePickerResult {
+  /// Returns the result as an [ImagePickerSuccessResult], or throws a [PlatformException]
+  /// if the result is an [ImagePickerErrorResult].
+  ImagePickerSuccessResult getSuccessOrThrow() {
+    final ImagePickerResult result = this;
+    return switch (result) {
+      ImagePickerSuccessResult() => result,
+      ImagePickerErrorResult() => () {
+          final String errorMessage = switch (result.error) {
+            ImagePickerError.phpickerUnsupported =>
+              'PHPicker is only supported on macOS 13.0 or newer.',
+            ImagePickerError.windowNotFound =>
+              'No active window to present the picker.',
+            ImagePickerError.invalidImageSelection =>
+              'One of the selected items is not an image.',
+            ImagePickerError.invalidVideoSelection =>
+              'One of the selected items is not a video.',
+            ImagePickerError.imageLoadFailed =>
+              'An error occurred while loading the image.',
+            ImagePickerError.videoLoadFailed =>
+              'An error occurred while loading the video.',
+            ImagePickerError.imageConversionFailed =>
+              'Failed to convert the NSImage to TIFF data.',
+            ImagePickerError.imageSaveFailed =>
+              'Error saving the NSImage data to a file.',
+            ImagePickerError.imageCompressionFailed =>
+              'Error while compressing the Data of the NSImage.',
+            ImagePickerError.multiVideoSelectionUnsupported =>
+              'The multi-video selection is not supported.',
+          };
+          // TODO(EchoEllet): Replace PlatformException with a plugin-specific exception.
+          //  This is currently implemented to maintain compatibility with the existing behavior
+          //  of other implementations of `image_picker`. For more details, refer to:
+          //  https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#platform-exception-handling
+          throw PlatformException(
+            code: result.error.name,
+            message: errorMessage,
+            details: result.platformErrorMessage,
+          );
+        }(),
+    };
+  }
+}

packages/image_picker/image_picker_macos/lib/src/messages.g.dart

@@ -1,7 +1,7 @@
 // Copyright 2013 The Flutter Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
-// Autogenerated from Pigeon (v22.6.1), do not edit directly.
+// Autogenerated from Pigeon (v22.7.2), do not edit directly.
 // See also: https://pub.dev/packages/pigeon
 // ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import, unnecessary_parenthesis, prefer_null_aware_operators, omit_local_variable_types, unused_shown_name, unnecessary_import, no_leading_underscores_for_local_identifiers
 
@@ -29,6 +29,45 @@ List<Object?> wrapResponse(
   return <Object?>[error.code, error.message, error.details];
 }
 
+/// Possible error conditions for [ImagePickerApi] calls.
+enum ImagePickerError {
+  /// The current macOS version doesn't support [PHPickerViewController](https://developer.apple.com/documentation/photosui/phpickerviewcontroller)
+  /// which is supported on macOS 13+.
+  phpickerUnsupported,
+
+  /// Could not show the picker due to the missing window.
+  windowNotFound,
+
+  /// When a `PHPickerResult` can't load `NSImage`. This error should not be reached
+  /// as the filter in the `PHPickerConfiguration` is set to accept only images.
+  invalidImageSelection,
+
+  /// When a `PHPickerResult` is not a video. This error should not be reached
+  /// as the filter in the `PHPickerConfiguration` is set to accept only videos.
+  invalidVideoSelection,
+
+  /// Could not load the image object as `NSImage`.
+  imageLoadFailed,
+
+  /// Could not load the video data representation.
+  videoLoadFailed,
+
+  /// The image tiff representation could not be loaded from the `NSImage`.
+  imageConversionFailed,
+
+  /// The loaded `Data` from the `NSImage` could not be written as a file.
+  imageSaveFailed,
+
+  /// The image could not be compressed or the `NSImage` could not be created
+  /// from the compressed `Data`.
+  imageCompressionFailed,
+
+  /// The multi-video selection is not supported as it's not supported in
+  /// the app-facing package (`pickVideos` is missing).
+  /// The multi-video selection is supported when using `pickMedia` instead.
+  multiVideoSelectionUnsupported,
+}
+
 /// The common options between [ImageSelectionOptions], [VideoSelectionOptions]
 /// and [MediaSelectionOptions].
 class GeneralOptions {
@@ -132,24 +171,85 @@ class MediaSelectionOptions {
   }
 }
 
+sealed class ImagePickerResult {}
+
+class ImagePickerSuccessResult extends ImagePickerResult {
+  ImagePickerSuccessResult({
+    required this.filePaths,
+  });
+
+  /// The temporary file paths as a result of picking the images and/or videos.
+  List<String> filePaths;
+
+  Object encode() {
+    return <Object?>[
+      filePaths,
+    ];
+  }
+
+  static ImagePickerSuccessResult decode(Object result) {
+    result as List<Object?>;
+    return ImagePickerSuccessResult(
+      filePaths: (result[0] as List<Object?>?)!.cast<String>(),
+    );
+  }
+}
+
+class ImagePickerErrorResult extends ImagePickerResult {
+  ImagePickerErrorResult({
+    required this.error,
+    this.platformErrorMessage,
+  });
+
+  /// Potential error conditions for [ImagePickerApi] calls.
+  ImagePickerError error;
+
+  /// Additional error message from the platform side.
+  String? platformErrorMessage;
+
+  Object encode() {
+    return <Object?>[
+      error,
+      platformErrorMessage,
+    ];
+  }
+
+  static ImagePickerErrorResult decode(Object result) {
+    result as List<Object?>;
+    return ImagePickerErrorResult(
+      error: result[0]! as ImagePickerError,
+      platformErrorMessage: result[1] as String?,
+    );
+  }
+}
+
 class _PigeonCodec extends StandardMessageCodec {
   const _PigeonCodec();
   @override
   void writeValue(WriteBuffer buffer, Object? value) {
     if (value is int) {
       buffer.putUint8(4);
       buffer.putInt64(value);
-    } else if (value is GeneralOptions) {
+    } else if (value is ImagePickerError) {
       buffer.putUint8(129);
+      writeValue(buffer, value.index);
+    } else if (value is GeneralOptions) {
+      buffer.putUint8(130);
       writeValue(buffer, value.encode());
     } else if (value is MaxSize) {
-      buffer.putUint8(130);
+      buffer.putUint8(131);
       writeValue(buffer, value.encode());
     } else if (value is ImageSelectionOptions) {
-      buffer.putUint8(131);
+      buffer.putUint8(132);
       writeValue(buffer, value.encode());
     } else if (value is MediaSelectionOptions) {
-      buffer.putUint8(132);
+      buffer.putUint8(133);
+      writeValue(buffer, value.encode());
+    } else if (value is ImagePickerSuccessResult) {
+      buffer.putUint8(134);
+      writeValue(buffer, value.encode());
+    } else if (value is ImagePickerErrorResult) {
+      buffer.putUint8(135);
       writeValue(buffer, value.encode());
     } else {
       super.writeValue(buffer, value);
@@ -160,13 +260,20 @@ class _PigeonCodec extends StandardMessageCodec {
   Object? readValueOfType(int type, ReadBuffer buffer) {
     switch (type) {
       case 129:
-        return GeneralOptions.decode(readValue(buffer)!);
+        final int? value = readValue(buffer) as int?;
+        return value == null ? null : ImagePickerError.values[value];
       case 130:
-        return MaxSize.decode(readValue(buffer)!);
+        return GeneralOptions.decode(readValue(buffer)!);
       case 131:
-        return ImageSelectionOptions.decode(readValue(buffer)!);
+        return MaxSize.decode(readValue(buffer)!);
       case 132:
+        return ImageSelectionOptions.decode(readValue(buffer)!);
+      case 133:
         return MediaSelectionOptions.decode(readValue(buffer)!);
+      case 134:
+        return ImagePickerSuccessResult.decode(readValue(buffer)!);
+      case 135:
+        return ImagePickerErrorResult.decode(readValue(buffer)!);
       default:
         return super.readValueOfType(type, buffer);
     }
@@ -188,6 +295,8 @@ class ImagePickerApi {
 
   final String pigeonVar_messageChannelSuffix;
 
+  /// Returns whether [PHPickerViewController](https://developer.apple.com/documentation/photosui/phpickerviewcontroller)
+  /// is supported on the current macOS version.
   Future<bool> supportsPHPicker() async {
     final String pigeonVar_channelName =
         'dev.flutter.pigeon.image_picker_macos.ImagePickerApi.supportsPHPicker$pigeonVar_messageChannelSuffix';
@@ -217,7 +326,7 @@ class ImagePickerApi {
     }
   }
 
-  Future<List<String>> pickImages(
+  Future<ImagePickerResult> pickImages(
       ImageSelectionOptions options, GeneralOptions generalOptions) async {
     final String pigeonVar_channelName =
         'dev.flutter.pigeon.image_picker_macos.ImagePickerApi.pickImages$pigeonVar_messageChannelSuffix';
@@ -243,12 +352,12 @@ class ImagePickerApi {
         message: 'Host platform returned null value for non-null return value.',
       );
     } else {
-      return (pigeonVar_replyList[0] as List<Object?>?)!.cast<String>();
+      return (pigeonVar_replyList[0] as ImagePickerResult?)!;
     }
   }
 
   /// Currently, multi-video selection is unimplemented.
-  Future<List<String>> pickVideos(GeneralOptions generalOptions) async {
+  Future<ImagePickerResult> pickVideos(GeneralOptions generalOptions) async {
     final String pigeonVar_channelName =
         'dev.flutter.pigeon.image_picker_macos.ImagePickerApi.pickVideos$pigeonVar_messageChannelSuffix';
     final BasicMessageChannel<Object?> pigeonVar_channel =
@@ -273,11 +382,11 @@ class ImagePickerApi {
         message: 'Host platform returned null value for non-null return value.',
       );
     } else {
-      return (pigeonVar_replyList[0] as List<Object?>?)!.cast<String>();
+      return (pigeonVar_replyList[0] as ImagePickerResult?)!;
     }
   }
 
-  Future<List<String>> pickMedia(
+  Future<ImagePickerResult> pickMedia(
       MediaSelectionOptions options, GeneralOptions generalOptions) async {
     final String pigeonVar_channelName =
         'dev.flutter.pigeon.image_picker_macos.ImagePickerApi.pickMedia$pigeonVar_messageChannelSuffix';
@@ -303,7 +412,7 @@ class ImagePickerApi {
         message: 'Host platform returned null value for non-null return value.',
       );
     } else {
-      return (pigeonVar_replyList[0] as List<Object?>?)!.cast<String>();
+      return (pigeonVar_replyList[0] as ImagePickerResult?)!;
     }
   }
 }