Duplicate errors that are un-writable (#9046)

* Duplicate errors that are un-writable

Certain errors (`DOMException`s, really) have un-writable error
messages. This mucks with our code, since we rewrite it to signal user
errors, give context, etc.

If an error's message isn't writable, duplicate the error with one that
is.

* Lint

* Fix tests

* lint

* lint

* Fix error

extensions/amp-a4a/0.1/amp-a4a.js

@@ -31,7 +31,7 @@ import {
 } from '../../../src/friendly-iframe-embed';
 import {isLayoutSizeDefined} from '../../../src/layout';
 import {isAdPositionAllowed} from '../../../src/ad-helper';
-import {dev, user} from '../../../src/log';
+import {dev, user, duplicateErrorIfNecessary} from '../../../src/log';
 import {getMode} from '../../../src/mode';
 import {isArray, isObject, isEnumValue} from '../../../src/types';
 import {some} from '../../../src/utils/promise';
@@ -797,7 +797,9 @@ export class AmpA4A extends AMP.BaseElement {
       throw error;
     }
 
-    if (!error || !error.message) {
+    if (error && error.message) {
+      error = duplicateErrorIfNecessary(/** @type {!Error} */(error));
+    } else {
       error = new Error('unknown error ' + error);
     }
     if (opt_ignoreStack) {

src/error.js

@@ -23,6 +23,7 @@ import {
 import {
   USER_ERROR_SENTINEL,
   isUserErrorMessage,
+  duplicateErrorIfNecessary,
 } from './log';
 import {isProxyOrigin} from './url';
 import {isCanary, experimentTogglesOrNull} from './experiments';
@@ -88,6 +89,7 @@ export function reportError(error, opt_associatedElement) {
     let isValidError;
     if (error) {
       if (error.message !== undefined) {
+        error = duplicateErrorIfNecessary(/** @type {!Error} */(error));
         isValidError = true;
       } else {
         const origError = error;

src/log.js

@@ -381,6 +381,7 @@ export class Log {
    * @private
    */
   prepareError_(error) {
+    error = duplicateErrorIfNecessary(error);
     if (this.suffix_) {
       if (!error.message) {
         error.message = this.suffix_;
@@ -417,6 +418,30 @@ function pushIfNonEmpty(array, val) {
   }
 }
 
+/**
+ * Some exceptions (DOMException, namely) have read-only message.
+ * @param {!Error} error
+ * @return {!Error};
+ */
+export function duplicateErrorIfNecessary(error) {
+  const message = error.message;
+  const test = String(Math.random());
+  error.message = test;
+
+  if (error.message === test) {
+    error.message = message;
+    return error;
+  }
+
+  const e = new Error(error.message);
+  // Copy all the extraneous things we attach.
+  for (const prop in error) {
+    e[prop] = error[prop];
+  }
+  // Ensure these are copied.
+  e.stack = error.stack;
+  return e;
+}
 
 /**
  * @param {...*} var_args
@@ -429,14 +454,15 @@ function createErrorVargs(var_args) {
   for (let i = 0; i < arguments.length; i++) {
     const arg = arguments[i];
     if (arg instanceof Error && !error) {
-      error = arg;
+      error = duplicateErrorIfNecessary(arg);
     } else {
       if (message) {
         message += ' ';
       }
       message += arg;
     }
   }
+
   if (!error) {
     error = new Error(message);
   } else if (message) {

src/service/viewer-impl.js

@@ -19,7 +19,7 @@ import {findIndex} from '../utils/array';
 import {map} from '../utils/object';
 import {documentStateFor} from './document-state';
 import {registerServiceBuilderForDoc} from '../service';
-import {dev} from '../log';
+import {dev, duplicateErrorIfNecessary} from '../log';
 import {isIframed} from '../dom';
 import {
   parseQueryString,
@@ -934,6 +934,7 @@ function parseParams_(str, allParams) {
  */
 function getChannelError(opt_reason) {
   if (opt_reason instanceof Error) {
+    opt_reason = duplicateErrorIfNecessary(opt_reason);
     opt_reason.message = 'No messaging channel: ' + opt_reason.message;
     return opt_reason;
   }

test/functional/test-log.js

@@ -23,6 +23,7 @@ import {
   rethrowAsync,
   setReportError,
   user,
+  duplicateErrorIfNecessary,
 } from '../../src/log';
 import * as sinon from 'sinon';
 
@@ -531,6 +532,45 @@ describe('Logging', () => {
     });
   });
 
+  describe('error', () => {
+    let log;
+    let reportedError;
+
+    beforeEach(function() {
+      log = new Log(win, RETURNS_OFF);
+      setReportError(function(e) {
+        reportedError = e;
+      });
+    });
+
+    it('reuse errors', () => {
+      let error = new Error('test');
+
+      log.error('TAG', error);
+      expect(reportedError).to.equal(error);
+      expect(error.message).to.equal('test');
+
+      log.error('TAG', 'should fail', 'XYZ', error);
+      expect(reportedError).to.equal(error);
+      expect(error.message).to.equal('should fail XYZ: test');
+
+      // #8917
+      try {
+        // This is an intentionally bad query selector
+        document.body.querySelector('#');
+      } catch (e) {
+        error = e;
+      }
+
+      log.error('TAG', error);
+      expect(reportedError).not.to.equal(error);
+      expect(reportedError.message).to.equal(error.message);
+
+      log.error('TAG', 'should fail', 'XYZ', error);
+      expect(reportedError).not.to.equal(error);
+      expect(reportedError.message).to.contain('should fail XYZ:');
+    });
+  });
 
   describe('rethrowAsync', () => {
     let clock;
@@ -597,4 +637,40 @@ describe('Logging', () => {
       expect(isUserErrorMessage(error.message)).to.be.true;
     });
   });
+
+  describe('duplicateErrorIfNecessary', () => {
+    it('should not duplicate if message is writeable', () => {
+      const error = {message: 'test'};
+
+      expect(duplicateErrorIfNecessary(error)).to.equal(error);
+    });
+
+    it('should duplicate if message is non-writable', () => {
+      const error = {};
+      Object.defineProperty(error, 'message', {
+        value: 'test',
+        writable: false,
+      });
+
+      expect(duplicateErrorIfNecessary(error)).to.not.equal(error);
+    });
+
+    it('copies all the tidbits', () => {
+      const error = {
+        stack: 'stack',
+        args: [1, 2, 3],
+        associatedElement: error,
+      };
+
+      Object.defineProperty(error, 'message', {
+        value: 'test',
+        writable: false,
+      });
+
+      const duplicate = duplicateErrorIfNecessary(error);
+      expect(duplicate.stack).to.equal(error.stack);
+      expect(duplicate.args).to.equal(error.args);
+      expect(duplicate.associatedElement).to.equal(error.associatedElement);
+    });
+  });
 });