Bug 1541633 - Part 2: Give workers a way to cleanly pass metadata about errors to callers. r=jlast
authorLogan Smyth <loganfsmyth@gmail.com>
Fri, 23 Aug 2019 14:33:16 +0000
changeset 553352 48ab934cd025ee7e8cf95f165cc94d8cbf2fd1ea
parent 553351 b5efa9d76fc930aab97b87e5d5bf404197cbadf8
child 553353 cc36acbf92ad0ec0530d3225d174e96733171d3a
push id2165
push userffxbld-merge
push dateMon, 14 Oct 2019 16:30:58 +0000
treeherdermozilla-release@0eae18af659f [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjlast
bugs1541633
milestone70.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1541633 - Part 2: Give workers a way to cleanly pass metadata about errors to callers. r=jlast Differential Revision: https://phabricator.services.mozilla.com/D42934
devtools/client/debugger/dist/parser-worker.js
devtools/client/debugger/dist/pretty-print-worker.js
devtools/client/debugger/dist/search-worker.js
devtools/client/debugger/dist/vendors.js
devtools/client/debugger/packages/devtools-source-map/src/source-map.js
devtools/client/debugger/packages/devtools-utils/src/tests/worker-utils.js
devtools/client/debugger/packages/devtools-utils/src/worker-utils.js
devtools/client/framework/toolbox.js
devtools/client/shared/source-map/index.js
devtools/client/shared/source-map/worker.js
--- a/devtools/client/debugger/dist/parser-worker.js
+++ b/devtools/client/debugger/dist/parser-worker.js
@@ -6770,17 +6770,19 @@ WorkerDispatcher.prototype = {
           return;
         }
 
         this.worker.removeEventListener("message", listener);
         result.results.forEach((resultData, i) => {
           const [, resolve, reject] = items[i];
 
           if (resultData.error) {
-            reject(resultData.error);
+            const err = new Error(resultData.message);
+            err.metadata = resultData.metadata;
+            reject(err);
           } else {
             resolve(resultData.response);
           }
         });
       };
 
       this.worker.addEventListener("message", listener);
     };
@@ -6803,42 +6805,52 @@ function workerHandler(publicInterface) 
     } = msg.data;
     Promise.all(calls.map(args => {
       try {
         const response = publicInterface[method].apply(undefined, args);
 
         if (response instanceof Promise) {
           return response.then(val => ({
             response: val
-          }), // Error can't be sent via postMessage, so be sure to
-          // convert to string.
-          err => ({
-            error: err.toString()
-          }));
+          }), err => asErrorMessage(err));
         }
 
         return {
           response
         };
       } catch (error) {
-        // Error can't be sent via postMessage, so be sure to convert to
-        // string.
-        return {
-          error: error.toString()
-        };
+        return asErrorMessage(error);
       }
     })).then(results => {
       self.postMessage({
         id,
         results
       });
     });
   };
 }
 
+function asErrorMessage(error) {
+  if (typeof error === "object" && error && "message" in error) {
+    // Error can't be sent via postMessage, so be sure to convert to
+    // string.
+    return {
+      error: true,
+      message: error.message,
+      metadata: error.metadata
+    };
+  }
+
+  return {
+    error: true,
+    message: error == null ? error : error.toString(),
+    metadata: undefined
+  };
+}
+
 module.exports = {
   WorkerDispatcher,
   workerHandler
 };
 
 /***/ }),
 /* 15 */
 /***/ (function(module, exports) {
--- a/devtools/client/debugger/dist/pretty-print-worker.js
+++ b/devtools/client/debugger/dist/pretty-print-worker.js
@@ -608,17 +608,19 @@ WorkerDispatcher.prototype = {
           return;
         }
 
         this.worker.removeEventListener("message", listener);
         result.results.forEach((resultData, i) => {
           const [, resolve, reject] = items[i];
 
           if (resultData.error) {
-            reject(resultData.error);
+            const err = new Error(resultData.message);
+            err.metadata = resultData.metadata;
+            reject(err);
           } else {
             resolve(resultData.response);
           }
         });
       };
 
       this.worker.addEventListener("message", listener);
     };
@@ -641,42 +643,52 @@ function workerHandler(publicInterface) 
     } = msg.data;
     Promise.all(calls.map(args => {
       try {
         const response = publicInterface[method].apply(undefined, args);
 
         if (response instanceof Promise) {
           return response.then(val => ({
             response: val
-          }), // Error can't be sent via postMessage, so be sure to
-          // convert to string.
-          err => ({
-            error: err.toString()
-          }));
+          }), err => asErrorMessage(err));
         }
 
         return {
           response
         };
       } catch (error) {
-        // Error can't be sent via postMessage, so be sure to convert to
-        // string.
-        return {
-          error: error.toString()
-        };
+        return asErrorMessage(error);
       }
     })).then(results => {
       self.postMessage({
         id,
         results
       });
     });
   };
 }
 
+function asErrorMessage(error) {
+  if (typeof error === "object" && error && "message" in error) {
+    // Error can't be sent via postMessage, so be sure to convert to
+    // string.
+    return {
+      error: true,
+      message: error.message,
+      metadata: error.metadata
+    };
+  }
+
+  return {
+    error: true,
+    message: error == null ? error : error.toString(),
+    metadata: undefined
+  };
+}
+
 module.exports = {
   WorkerDispatcher,
   workerHandler
 };
 
 /***/ }),
 
 /***/ 32:
--- a/devtools/client/debugger/dist/search-worker.js
+++ b/devtools/client/debugger/dist/search-worker.js
@@ -270,17 +270,19 @@ WorkerDispatcher.prototype = {
           return;
         }
 
         this.worker.removeEventListener("message", listener);
         result.results.forEach((resultData, i) => {
           const [, resolve, reject] = items[i];
 
           if (resultData.error) {
-            reject(resultData.error);
+            const err = new Error(resultData.message);
+            err.metadata = resultData.metadata;
+            reject(err);
           } else {
             resolve(resultData.response);
           }
         });
       };
 
       this.worker.addEventListener("message", listener);
     };
@@ -303,42 +305,52 @@ function workerHandler(publicInterface) 
     } = msg.data;
     Promise.all(calls.map(args => {
       try {
         const response = publicInterface[method].apply(undefined, args);
 
         if (response instanceof Promise) {
           return response.then(val => ({
             response: val
-          }), // Error can't be sent via postMessage, so be sure to
-          // convert to string.
-          err => ({
-            error: err.toString()
-          }));
+          }), err => asErrorMessage(err));
         }
 
         return {
           response
         };
       } catch (error) {
-        // Error can't be sent via postMessage, so be sure to convert to
-        // string.
-        return {
-          error: error.toString()
-        };
+        return asErrorMessage(error);
       }
     })).then(results => {
       self.postMessage({
         id,
         results
       });
     });
   };
 }
 
+function asErrorMessage(error) {
+  if (typeof error === "object" && error && "message" in error) {
+    // Error can't be sent via postMessage, so be sure to convert to
+    // string.
+    return {
+      error: true,
+      message: error.message,
+      metadata: error.metadata
+    };
+  }
+
+  return {
+    error: true,
+    message: error == null ? error : error.toString(),
+    metadata: undefined
+  };
+}
+
 module.exports = {
   WorkerDispatcher,
   workerHandler
 };
 
 /***/ }),
 
 /***/ 15:
--- a/devtools/client/debugger/dist/vendors.js
+++ b/devtools/client/debugger/dist/vendors.js
@@ -1484,17 +1484,19 @@ WorkerDispatcher.prototype = {
           return;
         }
 
         this.worker.removeEventListener("message", listener);
         result.results.forEach((resultData, i) => {
           const [, resolve, reject] = items[i];
 
           if (resultData.error) {
-            reject(resultData.error);
+            const err = new Error(resultData.message);
+            err.metadata = resultData.metadata;
+            reject(err);
           } else {
             resolve(resultData.response);
           }
         });
       };
 
       this.worker.addEventListener("message", listener);
     };
@@ -1517,42 +1519,52 @@ function workerHandler(publicInterface) 
     } = msg.data;
     Promise.all(calls.map(args => {
       try {
         const response = publicInterface[method].apply(undefined, args);
 
         if (response instanceof Promise) {
           return response.then(val => ({
             response: val
-          }), // Error can't be sent via postMessage, so be sure to
-          // convert to string.
-          err => ({
-            error: err.toString()
-          }));
+          }), err => asErrorMessage(err));
         }
 
         return {
           response
         };
       } catch (error) {
-        // Error can't be sent via postMessage, so be sure to convert to
-        // string.
-        return {
-          error: error.toString()
-        };
+        return asErrorMessage(error);
       }
     })).then(results => {
       self.postMessage({
         id,
         results
       });
     });
   };
 }
 
+function asErrorMessage(error) {
+  if (typeof error === "object" && error && "message" in error) {
+    // Error can't be sent via postMessage, so be sure to convert to
+    // string.
+    return {
+      error: true,
+      message: error.message,
+      metadata: error.metadata
+    };
+  }
+
+  return {
+    error: true,
+    message: error == null ? error : error.toString(),
+    metadata: undefined
+  };
+}
+
 module.exports = {
   WorkerDispatcher,
   workerHandler
 };
 
 /***/ }),
 
 /***/ 15:
--- a/devtools/client/debugger/packages/devtools-source-map/src/source-map.js
+++ b/devtools/client/debugger/packages/devtools-source-map/src/source-map.js
@@ -357,18 +357,30 @@ async function getOriginalSourceText(
   const generatedSourceId = originalToGeneratedId(originalSource.id);
   const map = await getSourceMap(generatedSourceId);
   if (!map) {
     return null;
   }
 
   let text = map.sourceContentFor(originalSource.url);
   if (!text) {
-    text = (await networkRequest(originalSource.url, { loadFromCache: false }))
-      .content;
+    try {
+      const response = await networkRequest(originalSource.url, {
+        loadFromCache: false,
+      });
+      text = response.content;
+    } catch (err) {
+      // Wrapper logic renders a notification about the specific URL that
+      // failed to load, so we include it in the error metadata.
+      err.metadata = {
+        ...err.metadata,
+        url: originalSource.url,
+      };
+      throw err;
+    }
   }
 
   return {
     text,
     contentType: getContentType(originalSource.url || ""),
   };
 }
 
--- a/devtools/client/debugger/packages/devtools-utils/src/tests/worker-utils.js
+++ b/devtools/client/debugger/packages/devtools-utils/src/tests/worker-utils.js
@@ -109,14 +109,16 @@ describe("worker utils", () => {
     handler({ data: { id: 53, method: "doSomething", calls: [[]] } });
 
     await postMessagePromise;
 
     expect(postMessageMock.mock.calls[0][0]).toEqual({
       id: 53,
       results: [
         {
-          error: "Error: failed",
+          error: true,
+          message: "failed",
+          metadata: undefined,
         },
       ],
     });
   });
 });
--- a/devtools/client/debugger/packages/devtools-utils/src/worker-utils.js
+++ b/devtools/client/debugger/packages/devtools-utils/src/worker-utils.js
@@ -78,17 +78,19 @@ WorkerDispatcher.prototype = {
         }
 
         this.worker.removeEventListener("message", listener);
 
         result.results.forEach((resultData, i) => {
           const [, resolve, reject] = items[i];
 
           if (resultData.error) {
-            reject(resultData.error);
+            const err = new Error(resultData.message);
+            (err: any).metadata = resultData.metadata;
+            reject(err);
           } else {
             resolve(resultData.response);
           }
         });
       };
 
       this.worker.addEventListener("message", listener);
     };
@@ -107,30 +109,44 @@ function workerHandler(publicInterface: 
 
     Promise.all(
       calls.map(args => {
         try {
           const response = publicInterface[method].apply(undefined, args);
           if (response instanceof Promise) {
             return response.then(
               val => ({ response: val }),
-              // Error can't be sent via postMessage, so be sure to
-              // convert to string.
-              err => ({ error: err.toString() })
+              err => asErrorMessage(err)
             );
           }
           return { response };
         } catch (error) {
-          // Error can't be sent via postMessage, so be sure to convert to
-          // string.
-          return { error: error.toString() };
+          return asErrorMessage(error);
         }
       })
     ).then(results => {
       self.postMessage({ id, results });
     });
   };
 }
 
+function asErrorMessage(error) {
+  if (typeof error === "object" && error && "message" in error) {
+    // Error can't be sent via postMessage, so be sure to convert to
+    // string.
+    return {
+      error: true,
+      message: error.message,
+      metadata: error.metadata,
+    };
+  }
+
+  return {
+    error: true,
+    message: error == null ? error : error.toString(),
+    metadata: undefined,
+  };
+}
+
 module.exports = {
   WorkerDispatcher,
   workerHandler,
 };
--- a/devtools/client/framework/toolbox.js
+++ b/devtools/client/framework/toolbox.js
@@ -1142,21 +1142,21 @@ Toolbox.prototype = {
                 return null;
               });
             };
 
           case "getOriginalSourceText":
             return originalSource => {
               return target
                 .getOriginalSourceText(originalSource)
-                .catch(text => {
+                .catch(error => {
                   const message = L10N.getFormatStr(
                     "toolbox.sourceMapSourceFailure",
-                    text,
-                    originalSource.url
+                    error.message,
+                    error.metadata ? error.metadata.url : "<unknown>"
                   );
                   this.target.logWarningInPage(message, "source map");
                   // Also replace the result with the error text.
                   // Note that this result has to have the same form
                   // as whatever the upstream getOriginalSourceText
                   // returns.
                   return {
                     text: message,
--- a/devtools/client/shared/source-map/index.js
+++ b/devtools/client/shared/source-map/index.js
@@ -482,17 +482,19 @@ WorkerDispatcher.prototype = {
           return;
         }
 
         this.worker.removeEventListener("message", listener);
         result.results.forEach((resultData, i) => {
           const [, resolve, reject] = items[i];
 
           if (resultData.error) {
-            reject(resultData.error);
+            const err = new Error(resultData.message);
+            err.metadata = resultData.metadata;
+            reject(err);
           } else {
             resolve(resultData.response);
           }
         });
       };
 
       this.worker.addEventListener("message", listener);
     };
@@ -515,42 +517,52 @@ function workerHandler(publicInterface) 
     } = msg.data;
     Promise.all(calls.map(args => {
       try {
         const response = publicInterface[method].apply(undefined, args);
 
         if (response instanceof Promise) {
           return response.then(val => ({
             response: val
-          }), // Error can't be sent via postMessage, so be sure to
-          // convert to string.
-          err => ({
-            error: err.toString()
-          }));
+          }), err => asErrorMessage(err));
         }
 
         return {
           response
         };
       } catch (error) {
-        // Error can't be sent via postMessage, so be sure to convert to
-        // string.
-        return {
-          error: error.toString()
-        };
+        return asErrorMessage(error);
       }
     })).then(results => {
       self.postMessage({
         id,
         results
       });
     });
   };
 }
 
+function asErrorMessage(error) {
+  if (typeof error === "object" && error && "message" in error) {
+    // Error can't be sent via postMessage, so be sure to convert to
+    // string.
+    return {
+      error: true,
+      message: error.message,
+      metadata: error.metadata
+    };
+  }
+
+  return {
+    error: true,
+    message: error == null ? error : error.toString(),
+    metadata: undefined
+  };
+}
+
 module.exports = {
   WorkerDispatcher,
   workerHandler
 };
 
 /***/ }),
 
 /***/ 182:
--- a/devtools/client/shared/source-map/worker.js
+++ b/devtools/client/shared/source-map/worker.js
@@ -506,17 +506,19 @@ WorkerDispatcher.prototype = {
           return;
         }
 
         this.worker.removeEventListener("message", listener);
         result.results.forEach((resultData, i) => {
           const [, resolve, reject] = items[i];
 
           if (resultData.error) {
-            reject(resultData.error);
+            const err = new Error(resultData.message);
+            err.metadata = resultData.metadata;
+            reject(err);
           } else {
             resolve(resultData.response);
           }
         });
       };
 
       this.worker.addEventListener("message", listener);
     };
@@ -539,42 +541,52 @@ function workerHandler(publicInterface) 
     } = msg.data;
     Promise.all(calls.map(args => {
       try {
         const response = publicInterface[method].apply(undefined, args);
 
         if (response instanceof Promise) {
           return response.then(val => ({
             response: val
-          }), // Error can't be sent via postMessage, so be sure to
-          // convert to string.
-          err => ({
-            error: err.toString()
-          }));
+          }), err => asErrorMessage(err));
         }
 
         return {
           response
         };
       } catch (error) {
-        // Error can't be sent via postMessage, so be sure to convert to
-        // string.
-        return {
-          error: error.toString()
-        };
+        return asErrorMessage(error);
       }
     })).then(results => {
       self.postMessage({
         id,
         results
       });
     });
   };
 }
 
+function asErrorMessage(error) {
+  if (typeof error === "object" && error && "message" in error) {
+    // Error can't be sent via postMessage, so be sure to convert to
+    // string.
+    return {
+      error: true,
+      message: error.message,
+      metadata: error.metadata
+    };
+  }
+
+  return {
+    error: true,
+    message: error == null ? error : error.toString(),
+    metadata: undefined
+  };
+}
+
 module.exports = {
   WorkerDispatcher,
   workerHandler
 };
 
 /***/ }),
 
 /***/ 171:
@@ -1702,19 +1714,29 @@ async function getOriginalSourceText(ori
 
   if (!map) {
     return null;
   }
 
   let text = map.sourceContentFor(originalSource.url);
 
   if (!text) {
-    text = (await networkRequest(originalSource.url, {
-      loadFromCache: false
-    })).content;
+    try {
+      const response = await networkRequest(originalSource.url, {
+        loadFromCache: false
+      });
+      text = response.content;
+    } catch (err) {
+      // Wrapper logic renders a notification about the specific URL that
+      // failed to load, so we include it in the error metadata.
+      err.metadata = { ...err.metadata,
+        url: originalSource.url
+      };
+      throw err;
+    }
   }
 
   return {
     text,
     contentType: getContentType(originalSource.url || "")
   };
 }
 /**