<html>
<head>
<meta content="text/html; charset=ISO-8859-1"
http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix"><br>
Promise chains often look like your examples, but with an extra
feature:<br>
<br>
<blockquote>
<pre>something.then(result => {
<b>return</b> step1.then(result => {
<b>return</b> step2.then(result => {
<b>return</b> 42;
});
});
}).then(console.log, console.error);</pre>
</blockquote>
<br>
I added ".then(console.log, console.error);" to make it show what
happens and some "returns" to make it more obvious what was going
on.<br>
<br>
The result of the above code would be console.log(42); because<br>
* the result of a call to "then" is a promise of the return value
of the function<br>
* and promises internally resolve a promise of a promise of a
thing, to be a promise of a thing<br>
<br>
Clearly this can be (and often is) split up into several parts:<br>
<br>
<blockquote>
<pre>something.then(result => {
<b>return</b> step1.then(doStuff);
}).then(console.log, console.error);
function doStuff(result) {
<b>return</b> step2.then(doMoreStuff);
}
function doMoreStuff(result) {
<b>return</b> 42;
}
</pre>
</blockquote>
<br>
Looking at doStuff and doMoreStuff saying "Where is the error
handling?", misses that the error handling is in the thing that
called doStuff.<br>
<br>
The lack of a 'return' statement doesn't change this, because
without an explicit return, the "then" function just returns a
promise of undefined, which works like "Tell me when you're done".<br>
<br>
The ratio of promise uses to error handling is surprisingly
lopsided. I believe the GCLI code does error handling properly and
the ratio there is 23 places where errors are handled, and 124
places where 'then' is called, and not 1:1. I just did a very
quick scan through the devtools code, and I could believe that the
ratio was similar there too.<br>
<br>
Joe.<br>
<br>
On 10/08/2013 04:04, Mark Hammond wrote:<br>
</div>
<blockquote cite="mid:5205ADDB.6050407@gmail.com" type="cite">On
9/08/2013 8:32 PM, David Rajchenbach-Teller wrote:
<br>
<blockquote type="cite">Furthermore, I *strongly* suggest that we
r- any promise chain that does
<br>
not report errors, unless there is a comment explaining why we
do not.
<br>
</blockquote>
<br>
This concept of a "promise chain" escapes me. I understand that
conceptually, promise code should look like:
<br>
<br>
something.then(step1)
<br>
.then(step2)
<br>
.then(step3)
<br>
.then(null, Cu.reportError)
<br>
<br>
One nice promise chain and the final .then() looks reasonable.
But every time I try and use promises, they actually end up
looking like:
<br>
<br>
something.then(result => {
<br>
step1.then(result => {
<br>
step2.then(result => {
<br>
step3.then(result => {
<br>
});
<br>
});
<br>
});
<br>
})
<br>
<br>
in that case we have 4 "promise chains". Thus, we need 4 extra
.then(null, Cu.reportError) calls, making the code much larger and
harder to follow.
<br>
<br>
At first I thought it was just me, so I started digging around in
our code. I looked at non-test code in browser/base and toolkit/
(but stopped at devtools), and examined over 30 calls to .then() -
and couldn't find a *single* case of a promise chain. The closest
I could find was a few places where .then(null, Cu.reportError)
was correctly at the end, giving a promise chain of length 2.
<br>
<br>
So, in practice in our code, *every single* .then() call must have
.then(null, Cu.reportError) tacked onto the end. Given so few of
our existing ones do this, fixing this code would require nearly
doubling the number of .then() calls in the code.
<br>
<br>
While most don't have any error reporting, the ones that do
attempt to handle errors may or may not be correct, depending on
your subjective viewpoint. For example, code like:
<br>
<br>
.then(null, error => {
<br>
this.foo.bar = "error";
<br>
});
<br>
<br>
is still poor form IMO - in exceptional circumstances, this.foo
might be null, so a final .then() should still be used even if the
error looks extremely unlikely. This is a subjective call, and
they are hard to get right - is the final .then() really necessary
there? Would the "unless there is a comment" clause get us out of
this?
<br>
<br>
So - I couldn't find a single "promise chain", and the vast
majority of promises fail to handle errors correctly. Of the ones
that *do* handle errors correctly, overwhelmingly they do the
trivial .then(null, Cu.reportError). I could only find a few
cases that used more sophisticated error handling. Or to put it
another way - if promises magically reported errors, there would
only be a few onRejected handlers left in the code I examined and
the vast majority of our code would suddenly become correct
without any further changes.
<br>
<br>
IMO, something is very wrong with this picture. Please understand
I'm not looking to blame anyone or anything. Promise-based code,
at first glance, looks like a huge improvement over what we were
doing before. It just concerns me that to do it right means
bloating the code with onRejected handlers that just do what
should be done automatically anyway - and that clearly we've
failed to do this in the past, so I'm not confident we will
consistently do it correctly in the future.
<br>
<br>
I'm relatively new to promises so apologize if I got any of the
details above wrong. Also, I didn't do a thorough examination of
all promises in the tree, so I may have overlooked significant
chunks of code that don't suffer the problems I've mentioned above
(but if I have, please point it out to me so I can learn from
it!) This conversation is purely motivated by the fact that I
like compact code so my brain doesn't overflow, and that I
personally screw things up so often that without good, reliable
error reporting I fail to achieve anything at all :)
<br>
<br>
Cheers,
<br>
<br>
Mark
<br>
<br>
_______________________________________________
<br>
firefox-dev mailing list
<br>
<a class="moz-txt-link-abbreviated" href="mailto:firefox-dev@mozilla.org">firefox-dev@mozilla.org</a>
<br>
<a class="moz-txt-link-freetext" href="https://mail.mozilla.org/listinfo/firefox-dev">https://mail.mozilla.org/listinfo/firefox-dev</a>
<br>
</blockquote>
<br>
</body>
</html>