Permalink
Browse files

Implement Deps.flush(_throwFirstError: true})

This makes the first error hit during flush time
to be thrown rather than logged. The flush process
then continues and all other errors are still logged,
and the flush process completes.
  • Loading branch information...
1 parent f778a2c commit 3e76fc9de417c1553d811817a3cd215003f61f24 @avital avital committed Feb 25, 2014
Showing with 56 additions and 33 deletions.
  1. +56 −33 packages/deps/deps.js
View
@@ -22,6 +22,15 @@ var _debugFunc = function () {
function () {}));
};
+var _throwOrLog = function (from, e) {
+ if (throwFirstError) {
+ throw e;
+ } else {
+ _debugFunc()("Exception from Deps " + from + " function:",
+ e.stack || e.message);
@mjgallag
mjgallag Feb 28, 2014

Is there any reason why you couldn't log BOTH the stack AND the message? I was debugging an issue with Safari and for whatever reason the line numbers in the stack trace logged to the console didn't seem to add up. It took me a while to realize I could set a break point here to see the full exception, and thus the message, which was very helpful in solving this particular issue. I definitely like the idea of throwing the first exception instead of eating it, as that would be great for debugging. But, still, you might as well log both the stack and the message of the exception if not throwing it.

+ }
+};
+
var nextId = 1;
// computations whose callbacks we should call at flush time
var pendingComputations = [];
@@ -34,6 +43,12 @@ var inFlush = false;
// Deps.nonreactive, which nullfies currentComputation even though
// an enclosing computation may still be running.
var inCompute = false;
+// `true` if the `_throwFirstError` option was passed in to the call
+// to Deps.flush that we are in. When set, throw rather than log the
+// first error encountered while flushing. Before throwing the error,
+// finish flushing (from a catch block), logging any subsequent
+// errors.
+var throwFirstError = false;
@glasser
glasser Feb 26, 2014 Member

Does this really need to be a global? Can't it just be a variable inside flush which is passed as an argument to _recompute (and indirectly to _throwOrLog)?

var afterFlushCallbacks = [];
@@ -159,20 +174,23 @@ _.extend(Deps.Computation.prototype, {
var self = this;
self._recomputing = true;
- while (self.invalidated && ! self.stopped) {
- try {
- self._compute();
- } catch (e) {
- _debugFunc()("Exception from Deps recompute:", e.stack || e.message);
+ try {
+ while (self.invalidated && ! self.stopped) {
+ try {
+ self._compute();
+ } catch (e) {
+ _throwOrLog("recompute", e);
+ }
+ // If _compute() invalidated us, we run again immediately.
+ // A computation that invalidates itself indefinitely is an
+ // infinite loop, of course.
+ //
+ // We could put an iteration counter here and catch run-away
+ // loops.
}
- // If _compute() invalidated us, we run again immediately.
- // A computation that invalidates itself indefinitely is an
- // infinite loop, of course.
- //
- // We could put an iteration counter here and catch run-away
- // loops.
+ } finally {
+ self._recomputing = false;
}
- self._recomputing = false;
}
});
@@ -227,7 +245,7 @@ _.extend(Deps.Dependency.prototype, {
_.extend(Deps, {
// http://docs.meteor.com/#deps_flush
- flush: function () {
+ flush: function (_opts) {
// Nested flush could plausibly happen if, say, a flush causes
// DOM mutation, which causes a "blur" event, which runs an
// app event handler that calls Deps.flush. At the moment
@@ -244,32 +262,37 @@ _.extend(Deps, {
inFlush = true;
willFlush = true;
+ throwFirstError = !! (_opts && _opts._throwFirstError);
- while (pendingComputations.length ||
- afterFlushCallbacks.length) {
-
- // recompute all pending computations
- var comps = pendingComputations;
- pendingComputations = [];
+ try {
+ while (pendingComputations.length ||
+ afterFlushCallbacks.length) {
- for (var i = 0, comp; comp = comps[i]; i++)
- comp._recompute();
+ // recompute all pending computations
+ while (pendingComputations.length) {
+ var comp = pendingComputations.shift();
+ comp._recompute();
+ }
- if (afterFlushCallbacks.length) {
- // call one afterFlush callback, which may
- // invalidate more computations
- var func = afterFlushCallbacks.shift();
- try {
- func();
- } catch (e) {
- _debugFunc()("Exception from Deps afterFlush function:",
- e.stack || e.message);
+ if (afterFlushCallbacks.length) {
+ // call one afterFlush callback, which may
+ // invalidate more computations
+ var func = afterFlushCallbacks.shift();
+ try {
+ func();
+ } catch (e) {
+ _throwOrLog("afterFlush function", e);
+ }
}
}
+ } catch (e) {
+ inFlush = false; // needed before calling `Deps.flush()` again
+ Deps.flush({_throwFirstError: false}); // finish flushing
+ throw e;
+ } finally {
+ willFlush = false;
+ inFlush = false;
}
-
- inFlush = false;
- willFlush = false;
},
// http://docs.meteor.com/#deps_autorun

0 comments on commit 3e76fc9

Please sign in to comment.