Exclude errors generated from A4A iframe

build-system/tasks/presubmit-checks.js

@@ -342,6 +342,7 @@ var forbiddenTerms = {
  'extensions/amp-access/0.1/amp-access.js',
  'extensions/amp-experiment/0.1/variant.js',
  'extensions/amp-user-notification/0.1/amp-user-notification.js',
+ 'dist.3p/current/integration.js',
  ],
  },
  'getBaseCid': {
@@ -402,6 +403,7 @@ var forbiddenTerms = {
  'src/service/cid-impl.js',
  'extensions/amp-user-notification/0.1/amp-user-notification.js',
  'extensions/amp-app-banner/0.1/amp-app-banner.js',
+ 'dist.3p/current/integration.js',
  ],
  },
  'localStorage': {

builtins/amp-img.js

@@ -123,9 +123,10 @@ export class AmpImg extends BaseElement {
  // only read "Graphic" when using only 'alt'.
  if (this.element.getAttribute('role') == 'img') {
  this.element.removeAttribute('role');
- user().error('AMP-IMG', 'Setting role=img on amp-img elements breaks ' +
- 'screen readers please just set alt or ARIA attributes, they will ' +
- 'be correctly propagated for the underlying <img> element.');
+ user(this.element).error('AMP-IMG',
+ 'Setting role=img on amp-img elements breaks ' +
+ 'screen readers please just set alt or ARIA attributes, they will ' +
+ 'be correctly propagated for the underlying <img> element.');
  }
 
  this.propagateAttributes(ATTRIBUTES_TO_PROPAGATE, this.img_);

examples/analytics-error-reporting.amp.html

@@ -51,5 +51,10 @@ <h1>Lorem Ipsum</h1>
  }
  </script>
 </amp-analytics>
+
+<amp-pixel src="https://foo.com/tracker/foo"
+ layout="nodisplay"
+ referrerpolicy="referrer-fail"></amp-pixel>
+
 </body>
 </html>

src/error.js

@@ -23,6 +23,7 @@ import {
 import {
  USER_ERROR_SENTINEL,
  isUserErrorMessage,
+ isUserErrorEmbed,
  duplicateErrorIfNecessary,
  dev,
 } from './log';
@@ -97,7 +98,8 @@ let detectedJsEngine;
  */
 export function reportErrorForWin(win, error, opt_associatedElement) {
  reportError(error, opt_associatedElement);
- if (error && isUserErrorMessage(error.message) && !!win) {
+ if (error && !!win && isUserErrorMessage(error.message)
+ && !isUserErrorEmbed(error.message)) {
  reportErrorToAnalytics(/** @type {!Error} */(error), win);
  }
 }

src/log.js

@@ -33,13 +33,28 @@ const start = Date.now();
 export const USER_ERROR_SENTINEL = '\u200B\u200B\u200B';
 
 
+/**
+ * Four zero width space.
+ *
+ * @const {string}
+ */
+export const USER_ERROR_EMBED_SENTINEL = '\u200B\u200B\u200B\u200B';
+
+
 /**
  * @return {boolean} Whether this message was a user error.
  */
 export function isUserErrorMessage(message) {
  return message.indexOf(USER_ERROR_SENTINEL) >= 0;
 }
 
+/**
+ * @return {boolean} Whether this message was a iframe embed error.
+ */
+export function isUserErrorEmbed(message) {
+ return message.indexOf(USER_ERROR_EMBED_SENTINEL) >= 0;
+}
+
 
 /**
  * @enum {number}
@@ -64,6 +79,10 @@ export function setReportError(fn) {
 
 /**
  * Logging class.
+ * Use of sentinel string instead of a boolean to check user/dev errors
+ * because errors could be rethrown by some native code as a new error, and only a message would survive.
+ * Also, some browser don’t support a 5th error object argument in window.onerror. List of supporting browser can be found
+ * here: https://blog.sentry.io/2016/01/04/client-javascript-reporting-window-onerror.html
  * @final
  * @private Visible for testing only.
  */
@@ -494,11 +513,12 @@ export function rethrowAsync(var_args) {
 /**
  * Cache for logs. We do not use a Service since the service module depends
  * on Log and closure literally can't even.
- * @type {{user: ?Log, dev: ?Log}}
+ * @type {{user: ?Log, dev: ?Log, userForEmbed: ?Log}}
  */
 self.log = (self.log || {
  user: null,
  dev: null,
+ userForEmbed: null,
 });
 
 const logs = self.log;
@@ -531,7 +551,6 @@ export function resetLogConstructorForTesting() {
  logConstructor = null;
 }
 
-
 /**
  * Publisher level log.
  *
@@ -540,25 +559,45 @@ export function resetLogConstructorForTesting() {
  * 2. Development mode is enabled via `#development=1` or logging is explicitly
  * enabled via `#log=D` where D >= 1.
  *
+ * @param {!Element=} opt_element
  * @return {!Log}
  */
-export function user() {
+export function user(opt_element) {
+ let logger;
  if (logs.user) {
- return logs.user;
+ logger = logs.user;
+ } else {
+ logger = getUserLogger(USER_ERROR_SENTINEL);
  }
+ if (!!opt_element &&
+ isFromEmbed(logger.win, /** @type {!Element} */ (opt_element))) {
+ if (logs.userForEmbed) {
+ return logs.userForEmbed;
+ }
+ return logs.userForEmbed = getUserLogger(USER_ERROR_EMBED_SENTINEL);
+ } else {
+ return logs.user = logger;
+ }
+}
+
+/**
+ * Getter for user logger
+ * @param {string=} suffix
+ * @returns {!Log}
+ */
+function getUserLogger(suffix) {
  if (!logConstructor) {
  throw new Error('failed to call initLogConstructor');
  }
- return logs.user = new logConstructor(self, mode => {
+ return new logConstructor(self, mode => {
  const logNum = parseInt(mode.log, 10);
  if (mode.development || logNum >= 1) {
  return LogLevel.FINE;
  }
  return LogLevel.OFF;
- }, USER_ERROR_SENTINEL);
+ }, suffix);
 }
 
-
 /**
  * AMP development log. Calls to `devLog().assert` and `dev.fine` are stripped in
  * the PROD binary. However, `devLog().assert` result is preserved in either case.
@@ -587,3 +626,12 @@ export function dev() {
  return LogLevel.OFF;
  });
 }
+
+/**
+ * @param {!Window} win
+ * @param {!Element} element
+ * @returns {boolean} isEmbed
+ */
+export function isFromEmbed(win, element) {
+ return element.ownerDocument.defaultView != win;
+}

test/functional/test-log.js

@@ -18,12 +18,14 @@ import {
  Log,
  LogLevel,
  USER_ERROR_SENTINEL,
+ USER_ERROR_EMBED_SENTINEL,
  dev,
  isUserErrorMessage,
  rethrowAsync,
  setReportError,
  user,
  duplicateErrorIfNecessary,
+ isUserErrorEmbed,
 } from '../../src/log';
 import * as sinon from 'sinon';
 
@@ -673,5 +675,45 @@ describe('Logging', () => {
  expect(duplicate.associatedElement).to.equal(error.associatedElement);
  });
  });
+
+ describe('embed error', () => {
+ let sandbox;
+ let iframe;
+ let element;
+ let element1;
+ let element2;
+
+ beforeEach(() => {
+ sandbox = sinon.sandbox.create();
+ iframe = document.createElement('iframe');
+ document.body.appendChild(iframe);
+ });
+
+ afterEach(() => {
+ sandbox.restore();
+ document.body.removeChild(iframe);
+ });
+
+ it('should return logger for user-error', () => {
+ const error = user().createError();
+ expect(isUserErrorEmbed(error.message)).to.be.false;
+ expect(user(this.element).suffix_).to.equal(USER_ERROR_SENTINEL);
+ });
+
+ it('should return logger for embed-error', () => {
+ element = document.createElement('embed');
+ iframe.contentWindow.document.body.appendChild(element);
+ expect(user(element).suffix_).to.equal(USER_ERROR_EMBED_SENTINEL);
+ });
+
+ it('should not create extra identical logs', () => {
+ element1 = document.createElement('embed_1');
+ element2 = document.createElement('embed_2');
+ iframe.contentWindow.document.body.appendChild(element1);
+ iframe.contentWindow.document.body.appendChild(element2);
+ expect(user()).to.equal(user(this.element));
+ expect(user(element1)).to.equal(user(element2));
+ });
+ });
 });