Skip to content
This repository was archived by the owner on Feb 22, 2018. It is now read-only.

Conversation

jbdeboer
Copy link
Contributor

No description provided.

lib/http.dart Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this is the right place for the zone. I think it should be in the root of angular bootstrap. Since it is not just HTTP futures which need to be zoned but any future anywhere in the codebase.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please make sure this works in dart2js.

@jbdeboer
Copy link
Contributor Author

I sat down with Misko and we hashed out a better approach. I'll update this PR when it is ready.

@jbdeboer
Copy link
Contributor Author

The runZoned logic now runs inside of scope.$apply.

If you wrap Futures in a scope.$apply, we will digest the scope whenever an asynchronous callback is run.

Copy link
Contributor

Choose a reason for hiding this comment

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

it you throw toThrow, then you are starting a new stack trace. What you want to do is to re-throw, which is just throw; with no arg.

but if toThrow is null then you eat exception? Can you explain this logic?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

toThrow is passing the exception from the onError handler out to the $apply function.

If we rethrow the exception in the onError handler, the runZoned runner picks it up and we never see it again.

The "toThrow == null" is there so we throw the first exception, since we can only throw one exception in the $apply function. [ I should write a test for this behaviour... ]

Copy link
Contributor

Choose a reason for hiding this comment

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

BTW, apparently throw; is deprecated. Gotta use "rethrow".

@mhevery
Copy link
Contributor

mhevery commented Jul 25, 2013

One comment, LGTM otherwise

@jbdeboer
Copy link
Contributor Author

Landed in 46c671f

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants