Skip to content

amp-bind: Catch exceptions in mutatedAttributesCallback#8383

Merged
dreamofabear merged 2 commits intoampproject:masterfrom
dreamofabear:bind-callback-guard
Mar 27, 2017
Merged

amp-bind: Catch exceptions in mutatedAttributesCallback#8383
dreamofabear merged 2 commits intoampproject:masterfrom
dreamofabear:bind-callback-guard

Conversation

@dreamofabear
Copy link
Copy Markdown

Fixes #8137.

  • Catch and wrap exceptions thrown in mutatedAttributesCallback.
  • Prefix error messages with "amp-bind:" and clarify some error messages.

/to @jridgewell @kmh287

const applyPromises = this.boundElements_.map(boundElement => {
const {element, boundProperties} = boundElement;

// TODO(choumx): We should avoid triggering a mutation if there are the
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Typo? Also can you give an example (in a reply) of what you mean?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Oops, fixed typo.

I mean that on a page with 10 bindings and a state mutation only affects one of them, there will be 9 unnecessary measure/mutate passes scheduled that will early-out when checking if the expression result has changed. This seems pretty wasteful.

@dreamofabear dreamofabear merged commit e948042 into ampproject:master Mar 27, 2017
@dreamofabear dreamofabear deleted the bind-callback-guard branch March 27, 2017 16:48
lironzluf pushed a commit to lironzluf/amphtml that referenced this pull request Mar 28, 2017
* master: (34 commits)
  Prevent amp-carousel next/previous icons fade away on desktop (ampproject#8428)
  Turn on flag slidescroll-disable-css-snap” (ampproject#8436)
  Revert "temporarily turn off yarn (ampproject#8356)" (ampproject#8384)
  initial commit (ampproject#8404)
  Upgrades for Index Exchange amp-ad tags to report load statistics (ampproject#8054)
  amp-bind validation tweak (ampproject#8414)
  Fix an amp-instagram race condition (ampproject#8192)
  Use whitelist to restrict urlReplacement for scoped analytics element (ampproject#8360)
  Report active experiments in error logs (ampproject#8108)
  amp-bind: Catch exceptions in mutatedAttributesCallback (ampproject#8383)
  Fixing custom scroll-snap on IOS (ampproject#8391)
  Add experiment for using AmpContext class in integration.js (ampproject#8348)
  add (ampproject#8349)
  swipe api (ampproject#8357)
  skip 3 flaky tests (ampproject#8388)
  amp-bind: Expression complexity limit (ampproject#8321)
  add margin-bottom (ampproject#8350)
  Flying carpet: make container full viewport and center content (ampproject#8292)
  Service Registration: Document Click (ampproject#7882)
  Add a8ad (ampproject#8036)
  ...
mrjoro pushed a commit to mrjoro/amphtml that referenced this pull request Apr 28, 2017
)

* initial commit for callback guard

* fix typo
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.

amp-bind: Improve error handling for mutatedAttributesCallback

5 participants