feat(image-cache): expose onError callback (#6458)

* feat(Cache): better error handling

* refactor(image-cache): add `error` parameter to `_onDownloadError`

Add DownloadError interface.

* refactor(image-cache): updates for iOS

Use arrow functions.
Remove an unnecessary `trace.write().

* refactor(image-cache): updates for Android

Update Android `constructor()`.
Move `key` and `image` check to `set()`.
Update `trace.write`.

* fix(image-cache): onError handling

tns-core-modules/http/http-request/http-request.android.ts

@@ -52,7 +52,10 @@ function ensureCompleteCallback() {
         onComplete: function (result: any, context: any) {
             // as a context we will receive the id of the request
             onRequestComplete(context, result);
-        }
+        },
+        onError: function (error: string, context: any) {
+            onRequestError(error, context);
+      },
     });
 }
 
@@ -148,6 +151,14 @@ function onRequestComplete(requestId: number, result: org.nativescript.widgets.A
     });
 }
 
+function onRequestError(error: string, requestId: number) {
+    var callbacks = pendingRequests[requestId];
+    delete pendingRequests[requestId];
+    if (callbacks) {
+        callbacks.rejectCallback(new Error(error));
+    }
+}
+
 function buildJavaOptions(options: http.HttpRequestOptions) {
     if (typeof options.url !== "string") {
         throw new Error("Http request must provide a valid url.");

tns-core-modules/ui/image-cache/image-cache-common.ts

@@ -6,10 +6,12 @@ export interface DownloadRequest {
     url: string;
     key: string;
     completed?: (image: any, key: string) => void;
+    error?: (key: string) => void;
 }
 
 export class Cache extends observable.Observable implements definition.Cache {
     public static downloadedEvent = "downloaded";
+    public static downloadErrorEvent = "downloadError";
 
     public placeholder: imageSource.ImageSource;
     public maxRequests = 5;
@@ -93,6 +95,20 @@ export class Cache extends observable.Observable implements definition.Cache {
         else {
             existingRequest.completed = newRequest.completed;
         }
+        if (existingRequest.error) {
+            if (newRequest.error) {
+                var existingError = existingRequest.error;
+                var stackError = function (key: string) {
+                    existingError(key);
+                    newRequest.error(key);
+                }
+
+                existingRequest.error = stackError;
+            }
+        }
+        else {
+            existingRequest.error = newRequest.error;
+        }
     }
 
     public get(key: string): any {
@@ -125,10 +141,7 @@ export class Cache extends observable.Observable implements definition.Cache {
     public _onDownloadCompleted(key: string, image: any) {
         var request = <DownloadRequest>this._pendingDownloads[key];
 
-        if (request.key && image) {
+        this.set(request.key, image);
-            this.set(request.key, image);
-        }
-
         this._currentDownloads--;
 
         if (request.completed) {
@@ -140,7 +153,29 @@ export class Cache extends observable.Observable implements definition.Cache {
                 eventName: Cache.downloadedEvent,
                 object: this,
                 key: key,
-                image: image 
+                image: image
+            });
+        }
+
+        delete this._pendingDownloads[request.key];
+
+        this._updateQueue();
+    }
+
+    public _onDownloadError(key: string, err: Error) {
+        var request = <DownloadRequest>this._pendingDownloads[key];
+        this._currentDownloads--;
+
+        if (request.error) {
+            request.error(request.key);
+        }
+
+        if (this.hasListeners(Cache.downloadErrorEvent)) {
+            this.notify({
+                eventName: Cache.downloadErrorEvent,
+                object: this,
+                key: key,
+                error: err
             });
         }
 
@@ -185,6 +220,7 @@ export class Cache extends observable.Observable implements definition.Cache {
     }
 }
 export interface Cache {
-    on(eventNames: string, callback: (args: observable.EventData) => void , thisArg?: any);
+    on(eventNames: string, callback: (args: observable.EventData) => void, thisArg?: any);
-    on(event: "downloaded", callback: (args: definition.DownloadedData) => void , thisArg?: any);
+    on(event: "downloaded", callback: (args: definition.DownloadedData) => void, thisArg?: any);
+    on(event: "downloadError", callback: (args: definition.DownloadError) => void, thisArg?: any);
 }

tns-core-modules/ui/image-cache/image-cache.android.ts

@@ -1,4 +1,5 @@
 import * as common from "./image-cache-common";
+import * as trace from "../../trace";
 
 var LruBitmapCacheClass;
 function ensureLruBitmapCacheClass() {
@@ -45,7 +46,17 @@ export class Cache extends common.Cache {
             onComplete: function (result: any, context: any) {
                 var instance = that.get();
                 if (instance) {
-                    instance._onDownloadCompleted(context, result)
+                    if (result) {
+                        instance._onDownloadCompleted(context, result);
+                    } else {
+                        instance._onDownloadError(context, new Error("No result in CompletionCallback"));
+                    }
+                }
+            },
+            onError: function (err: string, context: any) {
+                var instance = that.get();
+                if (instance) {
+                    instance._onDownloadError(context, new Error(err));
                 }
             }
         });
@@ -61,7 +72,13 @@ export class Cache extends common.Cache {
     }
 
     public set(key: string, image: any): void {
-        this._cache.put(key, image);
+        try {
+            if (key && image) {
+                this._cache.put(key, image);
+            }
+        } catch (err) {
+            trace.write("Cache set error: " + err, trace.categories.Error, trace.messageType.error);
+        }
     }
 
     public remove(key: string): void {

tns-core-modules/ui/image-cache/image-cache.d.ts

@@ -22,6 +22,10 @@ export interface DownloadRequest {
      * An optional function to be called when the download is complete.
      */
     completed?: (image: any, key: string) => void;
+    /**
+     * An optional function to be called if the download errors.
+     */
+    error?: (key: string) => void;
 }
 
 /**
@@ -32,6 +36,10 @@ export class Cache extends observable.Observable {
      * String value used when hooking to downloaded event.
      */
     public static downloadedEvent: string;
+    /**
+     * String value used when hooking to download error event.
+     */
+    public static downloadErrorEvent: string;
     /**
      * The image to be used to notify for a pending download request - e.g. loading indicator.
      */
@@ -82,12 +90,17 @@ export class Cache extends observable.Observable {
      * @param callback - Callback function which will be executed when event is raised.
      * @param thisArg - An optional parameter which will be used as `this` context for callback execution.
      */
-    on(eventNames: string, callback: (args: observable.EventData) => void , thisArg?: any);
+    on(eventNames: string, callback: (args: observable.EventData) => void, thisArg?: any);
 
     /**
      * Raised when the image has been downloaded.
      */
-    on(event: "downloaded", callback: (args: DownloadedData) => void , thisArg?: any);
+    on(event: "downloaded", callback: (args: DownloadedData) => void, thisArg?: any);
+
+    /**
+     * Raised if the image download errors.
+     */
+    on(event: "downloadError", callback: (args: DownloadError) => void, thisArg?: any);
 
     //@private
     /**
@@ -99,6 +112,11 @@ export class Cache extends observable.Observable {
      */
     _onDownloadCompleted(key: string, image: any);
     //@endprivate
+    /**
+     * @private
+     */
+    _onDownloadError(key: string, err: Error);
+    //@endprivate
 }
 
 /**
@@ -114,3 +132,17 @@ export interface DownloadedData extends observable.EventData {
      */
     image: imageSource.ImageSource;
 }
+
+/**
+ * Provides data for download error.
+ */
+export interface DownloadError extends observable.EventData {
+    /**
+     * A string indentifier of the cached image.
+     */
+    key: string;
+    /**
+     * Gets the error.
+     */
+    error: Error;
+}

tns-core-modules/ui/image-cache/image-cache.ios.ts

@@ -73,7 +73,7 @@ export class Cache extends common.Cache {
         super();
 
         this._cache = new NSCache<any, any>();
-        
+
         //this._delegate = NSCacheDelegateImpl.new();
         //this._cache.delegate = this._delegate;
 
@@ -83,11 +83,16 @@ export class Cache extends common.Cache {
     public _downloadCore(request: common.DownloadRequest) {
         ensureHttpRequest();
 
-        var that = this;
         httpRequest.request({ url: request.url, method: "GET" })
-            .then(response => {
+            .then((response) => {
-                var image = UIImage.alloc().initWithData(response.content.raw);
+                try {
-                that._onDownloadCompleted(request.key, image);
+                    var image = UIImage.alloc().initWithData(response.content.raw);
+                    this._onDownloadCompleted(request.key, image);
+                } catch (err) {
+                    this._onDownloadError(request.key, err);
+                }
+            }, (err) => {
+                this._onDownloadError(request.key, err);
             });
     }
 

tns-platform-declarations/android/org.nativescript.widgets.d.ts

@@ -5,10 +5,12 @@
                 export class CompleteCallback {
                     constructor(implementation: ICompleteCallback);
                     onComplete(result: Object, context: Object): void;
+                    onError(error: string, context: Object): void;
                 }
 
                 export interface ICompleteCallback {
                     onComplete(result: Object, context: Object): void;
+                    onError(error: string, context: Object): void;
                 }
 
                 export module Image {