Refactor Transaction to not rethrow errors
Rethrowing errors makes debugging harder. This makes it so that an exception in a render method can be caught using the purple stop sign (or the blue one, of course) in Chrome.
Ben Alpert committed
Jan 26, 2014 at 15:51 UTC
d300df51e1003ca293f269b5f04bd8eeeaec7eb4
1 file changed
+56
-34
src/utils/Transaction.js
+56
-34
@@ -27,7 +27,7 @@ var invariant = require('invariant');
27
* instantiates a transaction can provide enforcers of the invariants at
28
* creation time. The `Transaction` class itself will supply one additional
29
* automatic invariant for you - the invariant that any transaction instance
30
- * should not be ran while it is already being ran. You would typically create a
30
+ * should not be run while it is already being run. You would typically create a
31
* single instance of a `Transaction` for reuse multiple times, that potentially
32
* is used to wrap several different methods. Wrappers are extremely simple -
33
* they only require implementing two methods.
@@ -146,56 +146,69 @@ var Mixin = {
146
'is already an outstanding transaction.'
147
);
148
var memberStart = Date.now();
149
- var errorToThrow = null;
149
+ var errorThrown;
150
var ret;
151
try {
152
- this.initializeAll();
152
+ this._isInTransaction = true;
153
+ // Catching errors makes debugging more difficult, so we start with
154
+ // errorThrown set to true before setting it to false after calling
155
+ // close -- if it's still set to true in the finally block, it means
156
+ // one of these calls threw.
157
+ errorThrown = true;
158
+ this.initializeAll(0);
159
ret = method.call(scope, a, b, c, d, e, f);
154
- } catch (error) {
155
- // IE8 requires `catch` in order to use `finally`.
156
- errorToThrow = error;
160
+ errorThrown = false;
161
} finally {
162
var memberEnd = Date.now();
163
this.methodInvocationTime += (memberEnd - memberStart);
160
- try {
161
- this.closeAll();
162
- } catch (closeError) {
164
+ if (errorThrown) {
165
// If `method` throws, prefer to show that stack trace over any thrown
166
// by invoking `closeAll`.
165
- errorToThrow = errorToThrow || closeError;
167
+ try {
168
+ this.closeAll(0);
169
+ } catch (err) {
170
+ }
171
+ } else {
172
+ // Since `method` didn't throw, we don't want to silence the exception
173
+ // here.
174
+ this.closeAll(0);
175
}
167
- }
168
- if (errorToThrow) {
169
- throw errorToThrow;
176
+ this._isInTransaction = false;
177
}
178
return ret;
179
},
180
174
- initializeAll: function() {
175
- this._isInTransaction = true;
181
+ initializeAll: function(startIndex) {
182
var transactionWrappers = this.transactionWrappers;
183
var wrapperInitTimes = this.timingMetrics.wrapperInitTimes;
178
- var errorToThrow = null;
179
- for (var i = 0; i < transactionWrappers.length; i++) {
184
+ for (var i = startIndex; i < transactionWrappers.length; i++) {
185
var initStart = Date.now();
186
var wrapper = transactionWrappers[i];
187
try {
188
+ // Catching errors makes debugging more difficult, so we start with the
189
+ // OBSERVED_ERROR state before overwriting it with the real return value
190
+ // of initialize -- if it's still set to OBSERVED_ERROR in the finally
191
+ // block, it means wrapper.initialize threw.
192
+ this.wrapperInitData[i] = Transaction.OBSERVED_ERROR;
193
this.wrapperInitData[i] = wrapper.initialize ?
194
wrapper.initialize.call(this) :
195
null;
186
- } catch (initError) {
187
- // Prefer to show the stack trace of the first error.
188
- errorToThrow = errorToThrow || initError;
189
- this.wrapperInitData[i] = Transaction.OBSERVED_ERROR;
196
} finally {
197
var curInitTime = wrapperInitTimes[i];
198
var initEnd = Date.now();
199
wrapperInitTimes[i] = (curInitTime || 0) + (initEnd - initStart);
200
+
201
+ if (this.wrapperInitData[i] === Transaction.OBSERVED_ERROR) {
202
+ // The initializer for wrapper i threw an error; initialize the
203
+ // remaining wrappers but silence any exceptions from them to ensure
204
+ // that the first error is the one to bubble up.
205
+ try {
206
+ this.initializeAll(i + 1);
207
+ } catch (err) {
208
+ }
209
+ }
210
}
211
}
196
- if (errorToThrow) {
197
- throw errorToThrow;
198
- }
212
},
213
214
/**
@@ -204,36 +217,45 @@ var Mixin = {
217
* (`close`rs that correspond to initializers that failed will not be
218
* invoked).
219
*/
207
- closeAll: function() {
220
+ closeAll: function(startIndex) {
221
invariant(
222
this.isInTransaction(),
223
'Transaction.closeAll(): Cannot close transaction when none are open.'
224
);
225
var transactionWrappers = this.transactionWrappers;
226
var wrapperCloseTimes = this.timingMetrics.wrapperCloseTimes;
214
- var errorToThrow = null;
215
- for (var i = 0; i < transactionWrappers.length; i++) {
227
+ for (var i = startIndex; i < transactionWrappers.length; i++) {
228
var wrapper = transactionWrappers[i];
229
var closeStart = Date.now();
230
var initData = this.wrapperInitData[i];
231
+ var errorThrown;
232
try {
233
+ // Catching errors makes debugging more difficult, so we start with
234
+ // errorThrown set to true before setting it to false after calling
235
+ // close -- if it's still set to true in the finally block, it means
236
+ // wrapper.close threw.
237
+ errorThrown = true;
238
if (initData !== Transaction.OBSERVED_ERROR) {
239
wrapper.close && wrapper.close.call(this, initData);
240
}
223
- } catch (closeError) {
224
- // Prefer to show the stack trace of the first error.
225
- errorToThrow = errorToThrow || closeError;
241
+ errorThrown = false;
242
} finally {
243
var closeEnd = Date.now();
244
var curCloseTime = wrapperCloseTimes[i];
245
wrapperCloseTimes[i] = (curCloseTime || 0) + (closeEnd - closeStart);
246
+
247
+ if (errorThrown) {
248
+ // The closer for wrapper i threw an error; close the remaining
249
+ // wrappers but silence any exceptions from them to ensure that the
250
+ // first error is the one to bubble up.
251
+ try {
252
+ this.closeAll(i + 1);
253
+ } catch (e) {
254
+ }
255
+ }
256
}
257
}
258
this.wrapperInitData.length = 0;
233
- this._isInTransaction = false;
234
- if (errorToThrow) {
235
- throw errorToThrow;
236
- }
259
}
260
};
261