From 3481e6f33ff31da530b35ee2550b91fdf4221e1e Mon Sep 17 00:00:00 2001 From: Nathan Walker Date: Mon, 17 Dec 2018 01:33:12 -0800 Subject: [PATCH] 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 --- .../http/http-request/http-request.android.ts | 13 ++++- .../ui/image-cache/image-cache-common.ts | 50 ++++++++++++++++--- .../ui/image-cache/image-cache.android.ts | 21 +++++++- .../ui/image-cache/image-cache.d.ts | 36 ++++++++++++- .../ui/image-cache/image-cache.ios.ts | 15 ++++-- .../android/org.nativescript.widgets.d.ts | 2 + 6 files changed, 120 insertions(+), 17 deletions(-) diff --git a/tns-core-modules/http/http-request/http-request.android.ts b/tns-core-modules/http/http-request/http-request.android.ts index 0877e6d0d..ff986956f 100644 --- a/tns-core-modules/http/http-request/http-request.android.ts +++ b/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."); diff --git a/tns-core-modules/ui/image-cache/image-cache-common.ts b/tns-core-modules/ui/image-cache/image-cache-common.ts index b505a390d..3dbd50809 100644 --- a/tns-core-modules/ui/image-cache/image-cache-common.ts +++ b/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 = 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 = 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(event: "downloaded", callback: (args: definition.DownloadedData) => 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: "downloadError", callback: (args: definition.DownloadError) => void, thisArg?: any); } \ No newline at end of file diff --git a/tns-core-modules/ui/image-cache/image-cache.android.ts b/tns-core-modules/ui/image-cache/image-cache.android.ts index 0a6e4e94f..96118aeed 100644 --- a/tns-core-modules/ui/image-cache/image-cache.android.ts +++ b/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 { diff --git a/tns-core-modules/ui/image-cache/image-cache.d.ts b/tns-core-modules/ui/image-cache/image-cache.d.ts index 8d909bf31..2a697f964 100644 --- a/tns-core-modules/ui/image-cache/image-cache.d.ts +++ b/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; +} diff --git a/tns-core-modules/ui/image-cache/image-cache.ios.ts b/tns-core-modules/ui/image-cache/image-cache.ios.ts index 41f475c67..8224fcb56 100644 --- a/tns-core-modules/ui/image-cache/image-cache.ios.ts +++ b/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(); - + //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 => { - var image = UIImage.alloc().initWithData(response.content.raw); - that._onDownloadCompleted(request.key, image); + .then((response) => { + try { + 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); }); } diff --git a/tns-platform-declarations/android/org.nativescript.widgets.d.ts b/tns-platform-declarations/android/org.nativescript.widgets.d.ts index ae96b44f3..053e9c728 100644 --- a/tns-platform-declarations/android/org.nativescript.widgets.d.ts +++ b/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 {