diff --git a/extensions/amp-a4a/0.1/amp-a4a.js b/extensions/amp-a4a/0.1/amp-a4a.js index e7bbc4f50e01..ce506af60ec0 100644 --- a/extensions/amp-a4a/0.1/amp-a4a.js +++ b/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) { diff --git a/src/error.js b/src/error.js index 9f93dcee3773..3fd1ba902454 100644 --- a/src/error.js +++ b/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; diff --git a/src/log.js b/src/log.js index 84a8452d4572..7ccfafd0f629 100644 --- a/src/log.js +++ b/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,7 +454,7 @@ 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 += ' '; @@ -437,6 +462,7 @@ function createErrorVargs(var_args) { message += arg; } } + if (!error) { error = new Error(message); } else if (message) { diff --git a/src/service/viewer-impl.js b/src/service/viewer-impl.js index eed97a768b16..a1aea38ceb24 100644 --- a/src/service/viewer-impl.js +++ b/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; } diff --git a/test/functional/test-log.js b/test/functional/test-log.js index be4d8b71378e..307e2320763f 100644 --- a/test/functional/test-log.js +++ b/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); + }); + }); });