Skip to content
This repository has been archived by the owner on Mar 13, 2018. It is now read-only.

Observe.js swallows exceptions #72

Open
liebrand opened this issue Sep 12, 2014 · 5 comments
Open

Observe.js swallows exceptions #72

liebrand opened this issue Sep 12, 2014 · 5 comments
Labels

Comments

@liebrand
Copy link

If a data observer callback has an exception, it is swallowed by Observe.js (only logged to console)

https://github.com/Polymer/observe-js/blob/master/src/observe.js#L764

I dont think polymer (or any framework for that matter) should ever swallow an exception. The app should be able to have a event listener on the window for 'error' and log / handle the errors in the app.

@jmesserly
Copy link
Contributor

I think the issue is: it can't let the exception bubble from that point, because it will break invariants in the code. A single exception will cause all kinds of invalid state in observable system and the application using it, and it totally depends on things like what the delivery order happened to be.

However it could catch the exception and rethrow it asynchronously. That would cause it to end up at the top level handlers like window.onerror.

@jmesserly jmesserly added the p1 label Sep 12, 2014
@liebrand
Copy link
Author

That would definitely be better than completely swallowing it; and since the data observer is already running in a separate micro task, it wouldn't hurt too much to have the exception in the next turn (ie the code is already running effectively asynchronously)

Although if you use .deliverChanges to get the changes handled synchronously, getting the exception in the next turn might be odd... Not a showstopper though

@jmesserly
Copy link
Contributor

by .deliverChanges(), did you mean .deliver()? https://github.com/Polymer/observe-js/blob/master/src/observe.js#L752 ... (just wondering in case there is another deliverChanges method I forgot about somewhere)

... but yeah, that's a good point. It would be nice to get the exception synchronously in that case. Perhaps it could be not caught there and the catch+async rethrow instead in places like https://github.com/Polymer/observe-js/blob/master/src/observe.js#L822 and https://github.com/Polymer/observe-js/blob/master/src/observe.js#L660 ... would need to study the control flow more carefully to be sure, but it seems like it would be possible

@liebrand
Copy link
Author

@liebrand
Copy link
Author

liebrand commented Apr 2, 2015

Just checking to see what became of this bug? The links (since they point to master) now no longer really line up. But we recently encountered this problem again. Where the private report_ function swallows an exception in one of the observers.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

2 participants