<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>