Skip to content

add try/catch to calls to PerformanceObserver.prototype.observe and add tests#30554

Merged
jridgewell merged 6 commits intoampproject:masterfrom
erwinmombay:fix-navigation-obs-issue-2
Oct 12, 2020
Merged

add try/catch to calls to PerformanceObserver.prototype.observe and add tests#30554
jridgewell merged 6 commits intoampproject:masterfrom
erwinmombay:fix-navigation-obs-issue-2

Conversation

@erwinmombay
Copy link
Copy Markdown
Member

the performance tracking instance is initialized very early in the amp initialization lifecycle. A throw to a call to observe can occur and break the initialization of the whole runtime. Make it more resilient to this case by wrapping all calls to observe in a try catch block

@google-cla google-cla bot added the cla: yes label Oct 7, 2020
@erwinmombay erwinmombay force-pushed the fix-navigation-obs-issue-2 branch 3 times, most recently from 66cf2ed to 566ba89 Compare October 7, 2020 17:33
@erwinmombay erwinmombay force-pushed the fix-navigation-obs-issue-2 branch from 566ba89 to 6751698 Compare October 7, 2020 20:51
@erwinmombay erwinmombay requested a review from samouri October 8, 2020 18:27
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Oct 8, 2020

CLA assistant check
All committers have signed the CLA.

@erwinmombay erwinmombay force-pushed the fix-navigation-obs-issue-2 branch from eadfcfa to e8d9c98 Compare October 12, 2020 17:23
Copy link
Copy Markdown
Contributor

@jridgewell jridgewell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pushed a new commit.

@jridgewell jridgewell merged commit 968098a into ampproject:master Oct 12, 2020
ed-bird pushed a commit to ed-bird/amphtml that referenced this pull request Dec 10, 2020
…dd tests (ampproject#30554)

* add test for performance observer throws

* apply prettier

* consolidate observe calls into method

* Fix lints

* remove TAG reference

* Fix dev call

Co-authored-by: Justin Ridgewell <jridgewell@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants