Issue 4579 - Wrap rejection reason in Error object

polyfill.js

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 42 matching lines @@
     {
       object = object[node];
 
       if (!object)
         return;
     }
 
     let func = object[name];
     object[name] = function(...args)
     {
+      let callStack = new Error().stack;
+
       if (typeof args[args.length - 1] == "function")
         return func.apply(object, args);
 
       // If the last argument is undefined, we drop it from the list assuming
       // it stands for the optional callback. We must do this, because we have
       // to replace it with our own callback. If we simply append our own
       // callback to the list, it won't match the signature of the function and
       // will cause an exception.
       if (typeof args[args.length - 1] == "undefined")
         args.pop();
 
       return new Promise((resolve, reject) =>
       {
         func.call(object, ...args, result =>
         {
           let error = browser.runtime.lastError;
           if (error)
+          {
+            // runtime.lastError on Chrome is a plain object with only a
+            // message property.

[Sebastian Noack, 2017/10/19 18:09:56]

Perhaps, this comment could be a little bit more explicit, mentioning that
lastError is already an Error instance on Microsoft Edge.

[Manish Jethani, 2017/10/20 00:04:23]

Done.

+            if (!(error instanceof Error))
+              error = new Error(error.message);
+
+            // Add a more helpful stack trace.
+            error.stack = callStack;

[Sebastian Noack, 2017/10/19 18:09:56]

How does the stack on Microsoft Edge (where browser.runtime.lastError is an
Error instance) look like? Does it make sense to replace it, or should this line
rather be moved inside the conditional block above?

[Manish Jethani, 2017/10/20 00:04:23]

I'm not sure, but I think we should just move it inside the condition and let
Ollie move it out if he needs the debugging info. This is really just for us
developers, I don't suppose anyone else is interested in the stack trace.

+
             reject(error);
+          }
           else
+          {
             resolve(result);
+          }
         });
       });
     };
   }
 
   function shouldWrapAPIs()
   {
     try
     {
       return !(browser.storage.local.get([]) instanceof Promise);
@@ skipping 19 matching lines @@
 
   // Workaround since HTMLCollection, NodeList, StyleSheetList, and CSSRuleList
   // didn't have iterator support before Chrome 51.
   // https://bugs.chromium.org/p/chromium/issues/detail?id=401699
   for (let object of [HTMLCollection, NodeList, StyleSheetList, CSSRuleList])
   {
     if (!(Symbol.iterator in object.prototype))
       object.prototype[Symbol.iterator] = Array.prototype[Symbol.iterator];
   }
 }