From 53a7387db77b13b6791d511680eee0ed140d4721 Mon Sep 17 00:00:00 2001
From: Thibault Deckers <thibault.deckers@gmail.com>
Date: Mon, 5 Oct 2020 20:57:51 +0900
Subject: [PATCH] improved error logging

---
 .../aves/channel/calls/ImageDecodeTask.java   | 23 +++++++----
 .../aves/channel/calls/MetadataHandler.java   |  7 +---
 .../streams/ImageByteStreamHandler.java       |  6 ++-
 .../aves/decoder/AvesAppGlideModule.java      | 40 +++++++++++++++++++
 .../decoder/NoExifInterfaceGlideModule.java   | 23 -----------
 .../decoder/VideoThumbnailGlideModule.java    | 17 +-------
 lib/model/image_entry.dart                    |  4 +-
 lib/services/android_app_service.dart         |  2 +-
 lib/services/image_file_service.dart          |  6 +--
 .../app_icon_image_provider.dart              | 11 +++--
 .../image_providers/thumbnail_provider.dart   | 11 +++--
 .../image_providers/uri_image_provider.dart   |  7 +++-
 12 files changed, 91 insertions(+), 66 deletions(-)
 create mode 100644 android/app/src/main/java/deckers/thibault/aves/decoder/AvesAppGlideModule.java
 delete mode 100644 android/app/src/main/java/deckers/thibault/aves/decoder/NoExifInterfaceGlideModule.java

diff --git a/android/app/src/main/java/deckers/thibault/aves/channel/calls/ImageDecodeTask.java b/android/app/src/main/java/deckers/thibault/aves/channel/calls/ImageDecodeTask.java
index d9d24bba2..de8e1e2a5 100644
--- a/android/app/src/main/java/deckers/thibault/aves/channel/calls/ImageDecodeTask.java
+++ b/android/app/src/main/java/deckers/thibault/aves/channel/calls/ImageDecodeTask.java
@@ -52,10 +52,12 @@ public class ImageDecodeTask extends AsyncTask<ImageDecodeTask.Params, Void, Ima
     static class Result {
         Params params;
         byte[] data;
+        String errorDetails;
 
-        Result(Params params, byte[] data) {
+        Result(Params params, byte[] data, String errorDetails) {
             this.params = params;
             this.data = data;
+            this.errorDetails = errorDetails;
         }
     }
 
@@ -70,8 +72,8 @@ public class ImageDecodeTask extends AsyncTask<ImageDecodeTask.Params, Void, Ima
     protected Result doInBackground(Params... params) {
         Params p = params[0];
         Bitmap bitmap = null;
+        Exception exception = null;
         if (!this.isCancelled()) {
-            Exception exception = null;
             Integer w = p.width;
             Integer h = p.height;
             // fetch low quality thumbnails when size is not specified
@@ -97,13 +99,10 @@ public class ImageDecodeTask extends AsyncTask<ImageDecodeTask.Params, Void, Ima
             } catch (Exception e) {
                 exception = e;
             }
-
-            if (bitmap == null) {
-                Log.e(LOG_TAG, "failed to load thumbnail for uri=" + p.entry.uri + ", path=" + p.entry.path, exception);
-            }
         } else {
             Log.d(LOG_TAG, "getThumbnail with uri=" + p.entry.uri + " cancelled");
         }
+
         byte[] data = null;
         if (bitmap != null) {
             ByteArrayOutputStream stream = new ByteArrayOutputStream();
@@ -112,7 +111,15 @@ public class ImageDecodeTask extends AsyncTask<ImageDecodeTask.Params, Void, Ima
             bitmap.compress(Bitmap.CompressFormat.JPEG, 90, stream);
             data = stream.toByteArray();
         }
-        return new Result(p, data);
+
+        String errorDetails = null;
+        if (exception != null) {
+            errorDetails = exception.getMessage();
+            if (errorDetails != null && !errorDetails.isEmpty()) {
+                errorDetails = errorDetails.split("\n", 2)[0];
+            }
+        }
+        return new Result(p, data, errorDetails);
     }
 
     @RequiresApi(api = Build.VERSION_CODES.Q)
@@ -205,7 +212,7 @@ public class ImageDecodeTask extends AsyncTask<ImageDecodeTask.Params, Void, Ima
         if (result.data != null) {
             r.success(result.data);
         } else {
-            r.error("getThumbnail-null", "failed to get thumbnail for uri=" + uri + ", path=" + entry.path, null);
+            r.error("getThumbnail-null", "failed to get thumbnail for uri=" + uri + ", path=" + entry.path, result.errorDetails);
         }
     }
 }
diff --git a/android/app/src/main/java/deckers/thibault/aves/channel/calls/MetadataHandler.java b/android/app/src/main/java/deckers/thibault/aves/channel/calls/MetadataHandler.java
index 9f7a7372a..13647a302 100644
--- a/android/app/src/main/java/deckers/thibault/aves/channel/calls/MetadataHandler.java
+++ b/android/app/src/main/java/deckers/thibault/aves/channel/calls/MetadataHandler.java
@@ -326,11 +326,8 @@ public class MetadataHandler implements MethodChannel.MethodCallHandler {
             metadataMap.putAll(getVideoCatalogMetadataByMediaMetadataRetriever(uri));
         }
 
-        if (metadataMap.isEmpty()) {
-            result.error("getCatalogMetadata-failure", "failed to get catalog metadata for uri=" + uri + ", extension=" + extension, null);
-        } else {
-            result.success(metadataMap);
-        }
+        // report success even when empty
+        result.success(metadataMap);
     }
 
     private Map<String, Object> getCatalogMetadataByImageMetadataReader(String uri, String mimeType, String extension) {
diff --git a/android/app/src/main/java/deckers/thibault/aves/channel/streams/ImageByteStreamHandler.java b/android/app/src/main/java/deckers/thibault/aves/channel/streams/ImageByteStreamHandler.java
index 4cd43822a..6c856d477 100644
--- a/android/app/src/main/java/deckers/thibault/aves/channel/streams/ImageByteStreamHandler.java
+++ b/android/app/src/main/java/deckers/thibault/aves/channel/streams/ImageByteStreamHandler.java
@@ -128,7 +128,11 @@ public class ImageByteStreamHandler implements EventChannel.StreamHandler {
                     error("getImage-image-decode-null", "failed to get image from uri=" + uri, null);
                 }
             } catch (Exception e) {
-                error("getImage-image-decode-exception", "failed to get image from uri=" + uri, e.getMessage());
+                String errorDetails = e.getMessage();
+                if (errorDetails != null && !errorDetails.isEmpty()) {
+                    errorDetails = errorDetails.split("\n", 2)[0];
+                }
+                error("getImage-image-decode-exception", "failed to get image from uri=" + uri, errorDetails);
             } finally {
                 Glide.with(activity).clear(target);
             }
diff --git a/android/app/src/main/java/deckers/thibault/aves/decoder/AvesAppGlideModule.java b/android/app/src/main/java/deckers/thibault/aves/decoder/AvesAppGlideModule.java
new file mode 100644
index 000000000..d6c2c160b
--- /dev/null
+++ b/android/app/src/main/java/deckers/thibault/aves/decoder/AvesAppGlideModule.java
@@ -0,0 +1,40 @@
+package deckers.thibault.aves.decoder;
+
+import android.content.Context;
+import android.util.Log;
+
+import androidx.annotation.NonNull;
+
+import com.bumptech.glide.Glide;
+import com.bumptech.glide.GlideBuilder;
+import com.bumptech.glide.Registry;
+import com.bumptech.glide.annotation.GlideModule;
+import com.bumptech.glide.load.DecodeFormat;
+import com.bumptech.glide.load.resource.bitmap.ExifInterfaceImageHeaderParser;
+import com.bumptech.glide.module.AppGlideModule;
+import com.bumptech.glide.request.RequestOptions;
+
+import org.jetbrains.annotations.NotNull;
+
+@GlideModule
+public class AvesAppGlideModule extends AppGlideModule {
+    @Override
+    public void applyOptions(@NotNull Context context, @NonNull GlideBuilder builder) {
+        builder.setDefaultRequestOptions(new RequestOptions().format(DecodeFormat.PREFER_RGB_565));
+
+        // hide noisy warning (e.g. for images that can't be decoded)
+        builder.setLogLevel(Log.ERROR);
+    }
+
+    @Override
+    public void registerComponents(@NonNull Context context, @NonNull Glide glide, @NonNull Registry registry) {
+        // prevent ExifInterface error logs
+        // cf https://github.com/bumptech/glide/issues/3383
+        glide.getRegistry().getImageHeaderParsers().removeIf(parser -> parser instanceof ExifInterfaceImageHeaderParser);
+    }
+
+    @Override
+    public boolean isManifestParsingEnabled() {
+        return false;
+    }
+}
diff --git a/android/app/src/main/java/deckers/thibault/aves/decoder/NoExifInterfaceGlideModule.java b/android/app/src/main/java/deckers/thibault/aves/decoder/NoExifInterfaceGlideModule.java
deleted file mode 100644
index c209e4c80..000000000
--- a/android/app/src/main/java/deckers/thibault/aves/decoder/NoExifInterfaceGlideModule.java
+++ /dev/null
@@ -1,23 +0,0 @@
-package deckers.thibault.aves.decoder;
-
-import android.content.Context;
-
-import androidx.annotation.NonNull;
-
-import com.bumptech.glide.Glide;
-import com.bumptech.glide.Registry;
-import com.bumptech.glide.annotation.GlideModule;
-import com.bumptech.glide.load.resource.bitmap.ExifInterfaceImageHeaderParser;
-import com.bumptech.glide.module.LibraryGlideModule;
-
-@GlideModule
-public class NoExifInterfaceGlideModule extends LibraryGlideModule {
-    @Override
-    public void registerComponents(@NonNull Context context, @NonNull Glide glide, @NonNull Registry registry) {
-        super.registerComponents(context, glide, registry);
-        // prevent ExifInterface error logs
-        // cf https://github.com/bumptech/glide/issues/3383
-        glide.getRegistry().getImageHeaderParsers().removeIf(parser -> parser instanceof ExifInterfaceImageHeaderParser);
-    }
-}
-
diff --git a/android/app/src/main/java/deckers/thibault/aves/decoder/VideoThumbnailGlideModule.java b/android/app/src/main/java/deckers/thibault/aves/decoder/VideoThumbnailGlideModule.java
index ccabbfade..2912c1ebc 100644
--- a/android/app/src/main/java/deckers/thibault/aves/decoder/VideoThumbnailGlideModule.java
+++ b/android/app/src/main/java/deckers/thibault/aves/decoder/VideoThumbnailGlideModule.java
@@ -5,29 +5,16 @@ import android.content.Context;
 import androidx.annotation.NonNull;
 
 import com.bumptech.glide.Glide;
-import com.bumptech.glide.GlideBuilder;
 import com.bumptech.glide.Registry;
 import com.bumptech.glide.annotation.GlideModule;
-import com.bumptech.glide.load.DecodeFormat;
-import com.bumptech.glide.module.AppGlideModule;
-import com.bumptech.glide.request.RequestOptions;
+import com.bumptech.glide.module.LibraryGlideModule;
 
 import java.io.InputStream;
 
 @GlideModule
-public class VideoThumbnailGlideModule extends AppGlideModule {
-    @Override
-    public void applyOptions(@NonNull Context context, @NonNull GlideBuilder builder) {
-        builder.setDefaultRequestOptions(new RequestOptions().format(DecodeFormat.PREFER_RGB_565));
-    }
-
+public class VideoThumbnailGlideModule extends LibraryGlideModule {
     @Override
     public void registerComponents(@NonNull Context context, @NonNull Glide glide, @NonNull Registry registry) {
         registry.append(VideoThumbnail.class, InputStream.class, new VideoThumbnailLoader.Factory());
     }
-
-    @Override
-    public boolean isManifestParsingEnabled() {
-        return false;
-    }
 }
diff --git a/lib/model/image_entry.dart b/lib/model/image_entry.dart
index 04a692e3c..78540d5a8 100644
--- a/lib/model/image_entry.dart
+++ b/lib/model/image_entry.dart
@@ -318,8 +318,8 @@ class ImageEntry {
           locality: address.locality,
         );
       }
-    } catch (exception, stack) {
-      debugPrint('$runtimeType locate failed with path=$path coordinates=$coordinates exception=$exception\n$stack');
+    } catch (error, stackTrace) {
+      debugPrint('$runtimeType locate failed with path=$path coordinates=$coordinates error=$error\n$stackTrace');
     }
   }
 
diff --git a/lib/services/android_app_service.dart b/lib/services/android_app_service.dart
index 8d189505e..d2375e305 100644
--- a/lib/services/android_app_service.dart
+++ b/lib/services/android_app_service.dart
@@ -28,7 +28,7 @@ class AndroidAppService {
     } on PlatformException catch (e) {
       debugPrint('getAppIcon failed with code=${e.code}, exception=${e.message}, details=${e.details}');
     }
-    return Uint8List(0);
+    return null;
   }
 
   static Future<Map> getEnv() async {
diff --git a/lib/services/image_file_service.dart b/lib/services/image_file_service.dart
index f6fb234c5..996c3d217 100644
--- a/lib/services/image_file_service.dart
+++ b/lib/services/image_file_service.dart
@@ -100,12 +100,12 @@ class ImageFileService {
     } on PlatformException catch (e) {
       debugPrint('getImage failed with code=${e.code}, exception=${e.message}, details=${e.details}');
     }
-    return Future.sync(() => Uint8List(0));
+    return Future.sync(() => null);
   }
 
   static Future<Uint8List> getThumbnail(ImageEntry entry, double width, double height, {Object taskKey, int priority}) {
     if (entry.isSvg) {
-      return Future.sync(() => Uint8List(0));
+      return Future.sync(() => null);
     }
     return servicePolicy.call(
       () async {
@@ -120,7 +120,7 @@ class ImageFileService {
         } on PlatformException catch (e) {
           debugPrint('getThumbnail failed with code=${e.code}, exception=${e.message}, details=${e.details}');
         }
-        return Uint8List(0);
+        return null;
       },
 //      debugLabel: 'getThumbnail width=$width, height=$height entry=${entry.filenameWithoutExtension}',
       priority: priority ?? (width == 0 || height == 0 ? ServiceCallPriority.getFastThumbnail : ServiceCallPriority.getSizedThumbnail),
diff --git a/lib/widgets/common/image_providers/app_icon_image_provider.dart b/lib/widgets/common/image_providers/app_icon_image_provider.dart
index ac7a5ace4..a1a1ff3a4 100644
--- a/lib/widgets/common/image_providers/app_icon_image_provider.dart
+++ b/lib/widgets/common/image_providers/app_icon_image_provider.dart
@@ -1,4 +1,3 @@
-import 'dart:typed_data';
 import 'dart:ui' as ui show Codec;
 
 import 'package:aves/services/android_app_service.dart';
@@ -38,8 +37,14 @@ class AppIconImage extends ImageProvider<AppIconImageKey> {
   }
 
   Future<ui.Codec> _loadAsync(AppIconImageKey key, DecoderCallback decode) async {
-    final bytes = await AndroidAppService.getAppIcon(key.packageName, key.size);
-    return await decode(bytes ?? Uint8List(0));
+    try {
+      final bytes = await AndroidAppService.getAppIcon(key.packageName, key.size);
+      if (bytes == null) return null;
+      return await decode(bytes);
+    } catch (error) {
+      debugPrint('$runtimeType _loadAsync failed with packageName=$packageName, error=$error');
+      return null;
+    }
   }
 }
 
diff --git a/lib/widgets/common/image_providers/thumbnail_provider.dart b/lib/widgets/common/image_providers/thumbnail_provider.dart
index 7d0e58e3e..328b37f42 100644
--- a/lib/widgets/common/image_providers/thumbnail_provider.dart
+++ b/lib/widgets/common/image_providers/thumbnail_provider.dart
@@ -1,4 +1,3 @@
-import 'dart:typed_data';
 import 'dart:ui' as ui show Codec;
 
 import 'package:aves/model/image_entry.dart';
@@ -49,8 +48,14 @@ class ThumbnailProvider extends ImageProvider<ThumbnailProviderKey> {
   }
 
   Future<ui.Codec> _loadAsync(ThumbnailProviderKey key, DecoderCallback decode) async {
-    final bytes = await ImageFileService.getThumbnail(key.entry, extent, extent, taskKey: _cancellationKey);
-    return await decode(bytes ?? Uint8List(0));
+    try {
+      final bytes = await ImageFileService.getThumbnail(key.entry, extent, extent, taskKey: _cancellationKey);
+      if (bytes == null) return null;
+      return await decode(bytes);
+    } catch (error) {
+      debugPrint('$runtimeType _loadAsync failed with path=${entry.path}, error=$error');
+      return null;
+    }
   }
 
   @override
diff --git a/lib/widgets/common/image_providers/uri_image_provider.dart b/lib/widgets/common/image_providers/uri_image_provider.dart
index 04ca28379..46a7b48fa 100644
--- a/lib/widgets/common/image_providers/uri_image_provider.dart
+++ b/lib/widgets/common/image_providers/uri_image_provider.dart
@@ -1,5 +1,4 @@
 import 'dart:async';
-import 'dart:typed_data';
 import 'dart:ui' as ui show Codec;
 
 import 'package:aves/services/image_file_service.dart';
@@ -56,7 +55,11 @@ class UriImage extends ImageProvider<UriImage> {
           ));
         },
       );
-      return await decode(bytes ?? Uint8List(0));
+      if (bytes == null) return null;
+      return await decode(bytes);
+    } catch (error) {
+      debugPrint('$runtimeType _loadAsync failed with mimeType=$mimeType, uri=$uri, error=$error');
+      return null;
     } finally {
       unawaited(chunkEvents.close());
     }