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

Jump to bottom

template_binding: should be hardened against errors in bindings #17789

Closed
jmesserly opened this issue Mar 25, 2014 · 9 comments
Closed

template_binding: should be hardened against errors in bindings #17789

jmesserly opened this issue Mar 25, 2014 · 9 comments
Labels
area-pkg Used for miscellaneous pkg/ packages not associated with specific area- teams. closed-obsolete Closed as the reported issue is no longer relevant

Comments

@jmesserly
Copy link

evaluating a property path can throw, for various reasons (e.g. the getter throws).

as noted in the comment here:
https://codereview.chromium.org/211763002/diff/30001/pkg/observe/lib/src/path_observer.dart

Offhand, the places I'd be worried about would be these call trees:

  • dirtyCheckObservables
  • TemplateIterator
  • TemplateElementExtension.createInstance
  • maybe CompoundObserver

basically anything that processes a set of independent bindings in a list.
@sigmundch
Copy link
Member

We added a patch in r34456 to avoid breaking some internal invariants in TemplateIterator. The current behavior is not ideal though.

Here are some notes from our discussions. We would like to keep that [createInstance] fails in a synchronous way. This would reflect how createInstance works in the normal (non-error) case. When setting a model programatically, templates are expanded asynchronously, but when you call createInstance, templates are expanded synchronously. Internally the framework sets the model of the nested templates, but it expands them synchronously too.

In the case of errors, our current implementation fails synchronously for errors from bindings directly in the template content, but asynchronously for errors in nested templates. This is inconsistent.

One possible fix is to make sure that templateiterator can preserve its invariants even if you have to throw in the middle of _handleSplices, or make the code understand when it's being run in a synchronous context.

We should follow up by creating a repro in JS to get traction on a fix upstream first.

@sigmundch
Copy link
Member

Marked this as blocking #18377.

@jmesserly
Copy link
Author

Removed Area-Polymer label.
Added Area-Pkg label.

@sigmundch
Copy link
Member

Added this to the 1.6 milestone.
Removed Priority-Unassigned label.
Added Priority-Medium label.

@sigmundch
Copy link
Member

Removed this from the 1.6 milestone.
Added Polymer-P-1 label.

@sigmundch
Copy link
Member

Removed Polymer-P-1 label.
Added Polymer-Milestone-Next label.

@sigmundch
Copy link
Member

Added PolymerMilestone-Next label.

@sigmundch
Copy link
Member

Removed Polymer-Milestone-Next label.

@jmesserly
Copy link
Author

i think this is in pretty good shape now

Added AssumedStale label.

@jmesserly jmesserly added Type-Defect area-pkg Used for miscellaneous pkg/ packages not associated with specific area- teams. closed-obsolete Closed as the reported issue is no longer relevant labels Feb 4, 2015
@sigmundch sigmundch mentioned this issue Jun 3, 2015
Closed
@DartBot DartBot mentioned this issue Jun 5, 2015
Closed
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-pkg Used for miscellaneous pkg/ packages not associated with specific area- teams. closed-obsolete Closed as the reported issue is no longer relevant
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@jmesserly @sigmundch