Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

♻️ Extract error helpers and rethrowAsync into core #33881

Merged
merged 10 commits into from
Apr 16, 2021
3 changes: 2 additions & 1 deletion 3p/3p.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,10 @@

// Note: loaded by 3p system. Cannot rely on babel polyfills.

import {devAssert, rethrowAsync, userAssert} from '../src/log';
import {devAssert, userAssert} from '../src/log';
import {hasOwn, map} from '../src/core/types/object';
import {isArray} from '../src/core/types';
import {rethrowAsync} from '../src/core/error';

/** @typedef {function(!Window, !Object)} */
let ThirdPartyFunctionDef;
Expand Down
3 changes: 2 additions & 1 deletion ads/vendors/yieldbot.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@

import {getMultiSizeDimensions} from '../../ads/google/utils';
import {loadScript, validateData} from '../../3p/3p';
import {rethrowAsync, user} from '../../src/log';
import {rethrowAsync} from '../../src/core/error';
import {user} from '../../src/log';

/**
* @param {!Window} global
Expand Down
2 changes: 2 additions & 0 deletions build-system/test-configs/dep-check-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ exports.rules = [
'3p/**->src/core/constants/amp-events.js',
'3p/**->src/core/data-structures/observable.js',
'3p/**->src/core/data-structures/promise.js',
'3p/**->src/core/error.js',
'3p/**->src/core/types/index.js',
'3p/**->src/core/types/string.js',
'3p/**->src/core/types/object.js',
Expand Down Expand Up @@ -119,6 +120,7 @@ exports.rules = [
mustNotDependOn: 'src/**/*.js',
allowlist: [
'ads/**->src/utils/dom-fingerprint.js',
'ads/**->src/core/error.js',
'ads/**->src/core/types/index.js',
'ads/**->src/core/types/object.js',
'ads/**->src/core/types/string.js',
Expand Down
9 changes: 2 additions & 7 deletions extensions/amp-a4a/0.1/amp-a4a.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,14 +37,9 @@ import {assertHttpsUrl, tryDecodeUriComponent} from '../../../src/url';
import {cancellation, isCancellation} from '../../../src/error-reporting';
import {createElementWithAttributes} from '../../../src/dom';
import {createSecureDocSkeleton, createSecureFrame} from './secure-frame';
import {
dev,
devAssert,
duplicateErrorIfNecessary,
user,
userAssert,
} from '../../../src/log';
import {dev, devAssert, user, userAssert} from '../../../src/log';
import {dict} from '../../../src/core/types/object';
import {duplicateErrorIfNecessary} from '../../../src/core/error';
import {
getAmpAdRenderOutsideViewport,
incrementLoadingAds,
Expand Down
3 changes: 2 additions & 1 deletion extensions/amp-analytics/0.1/amp-analytics.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,13 @@ import {LinkerManager} from './linker-manager';
import {RequestHandler, expandPostMessage} from './requests';
import {Services} from '../../../src/services';
import {Transport} from './transport';
import {dev, devAssert, rethrowAsync, user} from '../../../src/log';
import {dev, devAssert, user} from '../../../src/log';
import {dict, hasOwn} from '../../../src/core/types/object';
import {expandTemplate} from '../../../src/core/types/string';
import {getMode} from '../../../src/mode';
import {installLinkerReaderService} from './linker-reader';
import {isArray, isEnumValue} from '../../../src/core/types';
import {rethrowAsync} from '../../../src/core/error';

import {isIframed} from '../../../src/dom';
import {isInFie} from '../../../src/iframe-helper';
Expand Down
3 changes: 2 additions & 1 deletion extensions/amp-app-banner/0.1/amp-app-banner.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@

import {CSS} from '../../../build/amp-app-banner-0.1.css';
import {Services} from '../../../src/services';
import {dev, rethrowAsync, user, userAssert} from '../../../src/log';
import {dev, user, userAssert} from '../../../src/log';
import {dict} from '../../../src/core/types/object';
import {openWindowDialog, removeElement} from '../../../src/dom';
import {rethrowAsync} from '../../../src/core/error';

const TAG = 'amp-app-banner';
const OPEN_LINK_TIMEOUT = 1500;
Expand Down
3 changes: 2 additions & 1 deletion extensions/amp-fx-collection/0.1/amp-fx-collection.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,14 @@ import {
FxType, // eslint-disable-line no-unused-vars
getFxTypes,
} from './fx-type';
import {devAssert, rethrowAsync} from '../../../src/log';
import {devAssert} from '../../../src/log';
import {
installPositionBoundFx,
installScrollToggledFx,
} from './providers/fx-provider';
import {iterateCursor} from '../../../src/dom';
import {listen} from '../../../src/event-helper';
import {rethrowAsync} from '../../../src/core/error';

const TAG = 'amp-fx-collection';

Expand Down
2 changes: 1 addition & 1 deletion extensions/amp-playbuzz/0.1/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import {
removeFragment,
serializeQueryString,
} from '../../../src/url';
import {rethrowAsync} from './../../../src/log';
import {rethrowAsync} from './../../../src/core/error';

/**
* Returns a function, that, as long as it continues to be invoked, will not
Expand Down
2 changes: 1 addition & 1 deletion extensions/amp-render/1.0/test/test-amp-render.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import '../../../amp-script/0.1/amp-script';
import '../amp-render';
import * as BatchedJsonModule from '../../../../src/batched-json';
import {ActionInvocation} from '../../../../src/service/action-impl';
import {ActionTrust} from '../../../../src/action-constants';
import {ActionTrust} from '../../../../src/core/constants/action-constants';
import {Services} from '../../../../src/services';
import {htmlFor} from '../../../../src/static-template';
import {toggleExperiment} from '../../../../src/experiments';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,13 @@ import {
} from '../../../src/service/notification-ui-manager';
import {Services} from '../../../src/services';
import {addParamsToUrl, assertHttpsUrl} from '../../../src/url';
import {dev, rethrowAsync, user, userAssert} from '../../../src/log';
import {dev, user, userAssert} from '../../../src/log';
import {dict} from '../../../src/core/types/object';
import {
getServicePromiseForDoc,
registerServiceBuilderForDoc,
} from '../../../src/service';
import {rethrowAsync} from '../../../src/core/error';
import {toggle} from '../../../src/style';

const TAG = 'amp-user-notification';
Expand Down
2 changes: 0 additions & 2 deletions src/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ module.exports = {
'./preact/slot.js',
'./core/contextprops.js',
'./context/node.js',
'./context/scheduler.js',
'./context/values.js',
// TEMPORARY, follow tracking issue #33631
'./preact/component/3p-frame.js',
],
Expand Down
2 changes: 1 addition & 1 deletion src/context/scheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import {rethrowAsync} from '../log';
import {rethrowAsync} from '../core/error';

/**
* Creates a scheduling function that executes the callback based on the
Expand Down
2 changes: 1 addition & 1 deletion src/context/values.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
import {deepScan, findParent} from './scan';
import {pureDevAssert as devAssert} from '../core/assert';
import {pushIfNotExist, removeItem} from '../core/types/array';
import {rethrowAsync} from '../log';
import {rethrowAsync} from '../core/error';
import {throttleTail} from './scheduler';

const EMPTY_ARRAY = [];
Expand Down
3 changes: 2 additions & 1 deletion src/core/assert.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
USER_ERROR_SENTINEL,
elementStringOrPassThru,
} from './error-message-helpers';
import {includes} from './types/string';
import {isMinifiedMode} from './minified-mode';
import {remove} from './types/array';

Expand Down Expand Up @@ -50,7 +51,7 @@ function assertion(
}

// Include the sentinel string if provided and not already present
if (sentinel && !opt_message.includes(sentinel)) {
if (sentinel && !includes(opt_message, sentinel)) {
opt_message += sentinel;
}

Expand Down
83 changes: 83 additions & 0 deletions src/core/error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/**
* Copyright 2021 The AMP HTML Authors. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS-IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

/**
* Some exceptions (DOMException, namely) have read-only message.
* @param {!Error} error
* @return {!Error};
*/
export function duplicateErrorIfNecessary(error) {
const messageProperty = Object.getOwnPropertyDescriptor(error, 'message');
if (messageProperty && messageProperty.writable) {
return error;
}

const {message, stack} = error;
const e = new Error(message);
// Copy all the extraneous things we attach.
for (const prop in error) {
e[prop] = error[prop];
}
// Ensure these are copied.
e.stack = stack;
return e;
}

/**
* @param {...*} var_args
* @return {!Error}
* @visibleForTesting
*/
export function createErrorVargs(var_args) {
let error = null;
let message = '';
for (let i = 0; i < arguments.length; i++) {
const arg = arguments[i];
if (arg instanceof Error && !error) {
error = duplicateErrorIfNecessary(arg);
} else {
if (message) {
message += ' ';
}
message += arg;
}
}

if (!error) {
error = new Error(message);
} else if (message) {
error.message = message + ': ' + error.message;
}
return error;
}

/**
* Rethrows the error without terminating the current context. This preserves
* whether the original error designation is a user error or a dev error.
* @param {...*} var_args
*/
export function rethrowAsync(var_args) {
const error = createErrorVargs.apply(null, arguments);
setTimeout(() => {
// __AMP_REPORT_ERROR is installed globally per window in the entry point.
// It may not exist for Bento components without the runtime.
if (self.__AMP_REPORT_ERROR) {
self.__AMP_REPORT_ERROR(error);
}

throw error;
});
}
3 changes: 2 additions & 1 deletion src/custom-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,12 @@ import {
isCancellation,
reportError,
} from './error-reporting';
import {dev, devAssert, rethrowAsync, user, userAssert} from './log';
import {dev, devAssert, user, userAssert} from './log';
import {getIntersectionChangeEntry} from './utils/intersection-observer-3p-host';
import {getMode} from './mode';
import {getSchedulerForDoc} from './service/scheduler';
import {isExperimentOn} from './experiments';
import {rethrowAsync} from './core/error';
import {setStyle} from './style';
import {shouldBlockOnConsentByMeta} from './consent';
import {startupChunk} from './chunk';
Expand Down
2 changes: 1 addition & 1 deletion src/error-reporting.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,11 @@ import {Services} from './services';
import {
USER_ERROR_SENTINEL,
dev,
duplicateErrorIfNecessary,
isUserErrorEmbed,
isUserErrorMessage,
} from './log';
import {dict} from './core/types/object';
import {duplicateErrorIfNecessary} from './core/error';
import {experimentTogglesOrNull, getBinaryType, isCanary} from './experiments';
import {exponentialBackoff} from './exponential-backoff';
import {getMode} from './mode';
Expand Down
3 changes: 2 additions & 1 deletion src/friendly-iframe-embed.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import {Services} from './services';
import {Signals} from './utils/signals';
import {VisibilityState} from './core/constants/visibility-state';
import {cssText as ampSharedCss} from '../build/ampshared.css';
import {dev, devAssert, rethrowAsync, userAssert} from './log';
import {dev, devAssert, userAssert} from './log';
import {
disposeServicesForEmbed,
getTopWindow,
Expand All @@ -48,6 +48,7 @@ import {
setStyle,
setStyles,
} from './style';
import {rethrowAsync} from './core/error';
import {toWin} from './types';
import {urls} from './config';
import {whenContentIniLoad} from './ini-load';
Expand Down
65 changes: 1 addition & 64 deletions src/log.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
USER_ERROR_SENTINEL,
elementStringOrPassThru,
} from './core/error-message-helpers';
import {createErrorVargs, duplicateErrorIfNecessary} from './core/error';
import {findIndex, isArray} from './core/types/array';
import {getMode} from './mode';
import {internalRuntimeVersion} from './internal-version';
Expand Down Expand Up @@ -623,70 +624,6 @@ export class Log {
}
}

/**
* Some exceptions (DOMException, namely) have read-only message.
* @param {!Error} error
* @return {!Error};
*/
export function duplicateErrorIfNecessary(error) {
const messageProperty = Object.getOwnPropertyDescriptor(error, 'message');
if (messageProperty && messageProperty.writable) {
return error;
}

const {message, stack} = error;
const e = new Error(message);
// Copy all the extraneous things we attach.
for (const prop in error) {
e[prop] = error[prop];
}
// Ensure these are copied.
e.stack = stack;
return e;
}

/**
* @param {...*} var_args
* @return {!Error}
* @visibleForTesting
*/
export function createErrorVargs(var_args) {
let error = null;
let message = '';
for (let i = 0; i < arguments.length; i++) {
const arg = arguments[i];
if (arg instanceof Error && !error) {
error = duplicateErrorIfNecessary(arg);
} else {
if (message) {
message += ' ';
}
message += arg;
}
}

if (!error) {
error = new Error(message);
} else if (message) {
error.message = message + ': ' + error.message;
}
return error;
}

/**
* Rethrows the error without terminating the current context. This preserves
* whether the original error designation is a user error or a dev error.
* @param {...*} var_args
*/
export function rethrowAsync(var_args) {
const error = createErrorVargs.apply(null, arguments);
setTimeout(() => {
// reportError is installed globally per window in the entry point.
self.__AMP_REPORT_ERROR(error);
throw error;
});
}

/**
* Cache for logs. We do not use a Service since the service module depends
* on Log and closure literally can't even.
Expand Down
3 changes: 2 additions & 1 deletion src/service/cid-impl.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {GoogleCidApi, TokenStatus} from './cid-api';
import {Services} from '../services';
import {ViewerCidApi} from './viewer-cid-api';
import {base64UrlEncodeFromBytes} from '../utils/base64';
import {dev, rethrowAsync, user, userAssert} from '../log';
import {dev, user, userAssert} from '../log';
import {dict} from '../core/types/object';
import {getCookie, setCookie} from '../cookies';
import {getCryptoRandomBytesArray} from '../utils/bytes';
Expand All @@ -36,6 +36,7 @@ import {getSourceOrigin, isProxyOrigin, parseUrlDeprecated} from '../url';
import {isExperimentOn} from '../../src/experiments';
import {isIframed} from '../dom';
import {parseJson, tryParseJson} from '../json';
import {rethrowAsync} from '../core/error';
import {tryResolve} from '../core/data-structures/promise';

const ONE_DAY_MILLIS = 24 * 3600 * 1000;
Expand Down
Loading