Merge pull request dojo#29 from SitePenKenFranqueiro/emit-error

dgrid components should emit an error event on store I/O errors

DnD.js

@@ -9,13 +9,7 @@ function(List, declare, lang, Deferred, DnDSource, DnDManager, put){
 
 	// TODOs:
 	// * consider sending items rather than nodes to onDropExternal/Internal
-	// * consider declaring an extension to dojo.dnd.Source rather than
-	//   clobbering on every instance we create;
-	//   it makes extending/overriding this plugin seem a bit obtuse
-	//   * barring that, might at least want to use safeMixin here
-
-	// TODO:
-	// 	* consider moving GridDnDSource to an external module
+	// * consider emitting store errors via OnDemandList._trackError
 
 	var GridDnDSource = declare(DnDSource, {
 		grid: null,

Editor.js

@@ -140,7 +140,7 @@ return function(column, editor, editOn){
 					}
 					dirty[column.field] = object[column.field] = value;
 					if(column.autoSave){
-						grid.save();
+						grid._trackError("save");
 					}
 				}else{
 					// else keep the value the same

OnDemandList.js

@@ -1,4 +1,13 @@
 define(["dojo/_base/declare", "dojo/_base/lang", "dojo/_base/Deferred", "dojo/on", "put-selector/put", "./List"], function(declare, lang, Deferred, listen, put, List){
+
+function emitError(err){
+	// called by _trackError in context of list/grid, if an error is encountered
+	listen.emit(this.domNode, "error", { error: err });
+}
+
+// noop is used as Deferred callback when we're only interested in errors
+function noop(r){ return r; }
+
 return declare([List], {
 	create: function(params, srcNodeRef){
 		this.inherited(arguments);
@@ -81,7 +90,7 @@ return declare([List], {
 		var results = query(options);
 		var self = this;
 		// render the result set
-		return Deferred.when(this.renderArray(results, preloadNode, options), function(trs){
+		Deferred.when(this.renderArray(results, preloadNode, options), function(trs){
 			return Deferred.when(results.total || results.length, function(total){
 				// now we need to adjust the height and total count based on the first result set
 				var trCount = trs.length;
@@ -109,7 +118,10 @@ return declare([List], {
 				// can remove the loading node now
 				return trs;
 			});
-		}, console.error);
+		});
+
+		// return results so that callers can handle potential of async error
+		return results;
 	},
 	sortOrder: null,
 	sort: function(property, descending){
@@ -125,11 +137,13 @@ return declare([List], {
 		if(this.store){
 			// render the query
 			var self = this;
-			this.renderQuery(function(queryOptions){
-				if(self.sortOrder){
-					queryOptions.sort = self.sortOrder;
-				}
-				return self.store.query(self.query, queryOptions);
+			this._trackError(function(){
+				return self.renderQuery(function(queryOptions){
+					if(self.sortOrder){
+						queryOptions.sort = self.sortOrder;
+					}
+					return self.store.query(self.query, queryOptions);
+				});
 			});
 		}
 	},
@@ -145,8 +159,7 @@ return declare([List], {
 		var priorPreload, preloadNode = this.preloadNode;
 		var lastScrollTop = this.lastScrollTop;
 		this.lastScrollTop = visibleTop;
-
-
+
 		function removeDistantNodes(grid, preloadNode, distanceOff, traversal, below){
 			// we check to see the the nodes are "far off"
 			var farOffRemoval = grid.farOffRemoval;
@@ -281,9 +294,14 @@ return declare([List], {
 				var loadingNode = put(beforeNode, "-div.dgrid-loading[style=height:" + count * this.rowHeight + "px]");
 				// use the query associated with the preload node to get the next "page"
 				options.query = preloadNode.query;
+
 				// query now to fill in these rows
-				var results = preloadNode.query(options);
-				Deferred.when(this.renderArray(results, loadingNode, options),function(){
+				var results = this._trackError(function(){
+					return preloadNode.query(options);
+				});
+				if(results === undefined){ return; } // sync query failed
+
+				Deferred.when(this.renderArray(results, loadingNode, options), function(){
 						// can remove the loading node now
 						beforeNode = loadingNode.nextSibling;
 						loadingNode.parentNode.removeChild(loadingNode);
@@ -293,35 +311,82 @@ return declare([List], {
 							// so we don't "jump" in the scrolling position
 							scrollNode.scrollTop += beforeNode.offsetTop - keepScrollTo;
 						}
-				}, console.error);
+				});
 				preloadNode = preloadNode.previous;
 
 			}
 		}
 	},
 	getBeforePut: true,
-	save: function(){
-		var store = this.store;
-		var puts = [];
-		for(var id in this.dirty){
-			var put = (function(dirty){
-				return function(object){
-					// copy all the dirty properties onto the original
-					for(key in dirty){
-						object[key] = dirty[key];
-					}
-					// put it
-					store.put(object);
+	save: function() {
+		// Keep track of the store and puts
+		var self = this,
+			store = this.store,
+			dirty = this.dirty,
+			dfd = new Deferred(), promise = dfd.promise,
+			getFunc = function(id){
+				// returns a function to pass as a step in the promise chain,
+				// with the id variable closured
+				return this.getBeforePut ?
+					function(){ return store.get(id); } :
+					function(){ return self.row(id).data; };
+			};
+
+		// For every dirty item, grab the ID
+		for(var id in this.dirty) {
+			// Create put function to handle the saving of the the item
+			var put = (function(dirtyObj) {
+				// Return a function handler
+				return function(object) {
+					var key;
+					// Copy dirty props to the original
+					for(key in dirtyObj){ object[key] = dirtyObj[key]; }
+					// Put it in the store, returning the result/promise
+					return dojo.when(store.put(object), function() {
+						// Delete the item now that it's been confirmed updated
+						delete dirty[id];
+					});
 				};
-			})(this.dirty[id]);
-			puts.push(this.getBeforePut ?
-				// retrieve the full object from the store
-				Deferred.when(store.get(id), put) :
-				// just use the cached object
-				put(this.row(id).data));
+			})(dirty[id]);
+
+			// Add this item onto the promise chain,
+			// getting the item from the store first if desired.
+			promise = promise.then(getFunc(id)).then(put);
+		}
+
+		// Kick off and return the promise representing all applicable get/put ops.
+		// If the success callback is fired, all operations succeeded; otherwise,
+		// save will stop at the first error it encounters.
+		dfd.resolve();
+		return promise;
+	},
+
+	_trackError: function(func){
+		// summary:
+		//		Utility function to handle emitting of error events.
+		// func: Function|String
+		//		A function which performs some store operation, or a String identifying
+		//		a function to be invoked (sans arguments) hitched against the instance.
+		//		If sync, it can return a value, but may throw an error on failure.
+		//		If async, it should return a promise, which would fire the error
+		//		callback on failure.
+		// tags:
+		//		protected
+
+		var result;
+
+		if(typeof func == "string"){ func = lang.hitch(this, func); }
+
+		try{
+			result = func();
+		}catch(err){
+			// report sync error
+			emitError.call(this, err);
+			// TODO: should we re-throw? probably not, but callers may have to handle undefined.
 		}
-		this.dirty = {}; // clear map - contents are no longer dirty
-		return puts;
+
+		// wrap in when call to handle reporting of potential async error
+		return Deferred.when(result, noop, lang.hitch(this, emitError));
 	}
 });
 

Tree.js

@@ -49,7 +49,9 @@ return function(column){
 						};
 						query.level = target.level;
 						grid.renderQuery ?
-							grid.renderQuery(query, preloadNode) :
+							grid._trackError(function(){
+								return grid.renderQuery(query, preloadNode);
+							}) :
 							grid.renderArray(query({}), preloadNode);
 					}
 					// show or hide all the children

test/test_OnDemand.html

@@ -45,11 +45,6 @@
 			#grid2 .field-bool { /* checkbox */
 				width: 4em;
 			}
-			#save {
-				display:block;
-				clear:both;
-				margin-bottom: 20px;
-			}
 
 			#gridAddDel, #listAddDel {
 				float: left;
@@ -164,7 +159,11 @@
 						}
 					}, "listAddDel");
 
-					// buttons for 1st grid
+					// buttons / listeners for 1st grid
+
+					grid.on("error", function(evt) {
+						console.warn("error on grid: ", evt.error);
+					});
 
 					on(document.getElementById("setstore"), "click", function(){
 						console.log("setting store to colorStore: ", colorStore);
@@ -198,37 +197,43 @@
 
 					// buttons / listeners for 2nd grid
 
+					function updateDirty(){
+						setTimeout(function(){
+							// clear "dirty items" list after change has been processed
+							domConstruct.empty("dirty");
+							for(var rowId in grid2.dirty){
+								var node = domConstruct.create("div",
+									{ innerHTML: "rowId: "+rowId }, "dirty");
+							}
+						}, 0);
+					}
+
+					grid2.on("error", function(evt) {
+						console.warn("error on grid2: ", evt.error);
+					});
+
 					on(document.getElementById("save"), "click", function(){
 						console.log("save to store");
 						grid2.save();
-						domConstruct.empty("dirty");
+						updateDirty();
 					});
 
 					on(document.getElementById("setorigeditstore"), "click", function(){
 						grid2.setStore(testTypesStore, {});
-						domConstruct.empty("dirty");
+						updateDirty();
 					});
 
 					on(document.getElementById("seterrorputstore"), "click", function(){
 						grid2.setStore(putErrorStore, {});
-						domConstruct.empty("dirty");
+						updateDirty();
 					});
 
 					on(document.getElementById("setdeferrorputstore"), "click", function(){
 						grid2.setStore(defPutErrorStore, {});
-						domConstruct.empty("dirty");
+						updateDirty();
 					});
 
-					var dirtyListener = grid2.on("datachange", function(event){
-						setTimeout(function(){
-							// clear "dirty items" list after change has been processed
-							domConstruct.empty("dirty");
-							for(var rowId in grid2.dirty){
-								var node = domConstruct.create("div",
-									{ innerHTML: "rowId: "+rowId }, "dirty");
-							}
-						}, 0);
-					});
+					var dirtyListener = grid2.on("datachange", updateDirty);
 
 					// buttons for 3rd grid
 
@@ -242,6 +247,13 @@
 							smallColorStore.remove(id);
 						}
 					});
+
+					// hook up a listener for error events on the document to test bubbling
+					// (currently error events don't seem to bubble,
+					// but perhaps we want them to?)
+					on(document, "error", function(evt){
+						console.log("document received error: ", evt.error);
+					});
 				});
 		</script>
 	</head>
@@ -263,7 +275,14 @@ <h2>Simple test to show editor effect on store data</h2>
 		<div style="float:left;margin-left:20px;"><div><h3>Dirty Items</h3></div><div id="dirty"></div></div>
 		<div id="grid3"></div>
 		</div>
-		<button id="save">Save Changes</button>
+		<div style="clear:both"><button id="save">Save Changes</button></div>
+		<p>The following buttons switch between stores in the left grid above,
+			to test handling of errors on put operations.</p>
+		<p>Note that clicking Save will NOT emit an error event on the grid,
+			as direct invocations of save are expected to handle the returned promise
+			as they see fit.  However, modifying an editor field with autoSave enabled
+			will cause an error event to be emitted, since in this case it is
+			the grid which initiated the operation.</p>
 		<button id="setorigeditstore">Set to original store</button>
 		<button id="seterrorputstore">Set to store which causes error on put</button>
 		<button id="setdeferrorputstore">Set to store which rejects Deferred on put</button>