WTF and Zone.js do not like each other

https://github.com/angular/zone.js/issues/133

Go to: http://mhevery.github.io/github_issues/ and see Angular2 app running.
turn on the WTF extension "Enable Recording for this URL"
Observe that Angular2 app is no longer running.

I don't see any exception in the console. It seems as if the WTF extension is somehow breaking the System.js module loader and preventing it from loading Angular 2 app.

WTF monkey patches the XMLHttpRequest object.
Angular, through zone.js then monkey patches the XMLHttpRequest again.
system.js sends of a request to load the code code.
3.a) WTF sees the response, but when it tries to forward the request on, it fails
3.b) the syntem.js does not see the response.

So it seems like the WTF and zone.js do not like each other when monkeypatching.

Zone.js way of monkey patching:

WTF monkey patching code:

tracing-framework/src/wtf/trace/providers/xhrprovider.js

Line 75 in 661b203

wtf.trace.providers.XhrProvider.prototype.injectXhr_ = function() {

If you click the gear on the WTF HUD that's on the page and go to Providers at the bottom you can uncheck the XMLHttpRequest provider. Then the page works great. So it's definitely this causing a problem.

XHR lives here:
https://github.com/google/tracing-framework/blob/master/src/wtf/trace/providers/xhrprovider.js#L103

I actually swap out the entire XMLHttpRequest object with that version.

It subclasses BaseEventTarget, which adds the addEventListener/removeEventListener methods:
https://github.com/google/tracing-framework/blob/master/src/wtf/trace/eventtarget.js#L431

I'm not seeing anything that stands out as a possible problem.
It's possible patchProperty in zone.js is also messing with things (if you're using the on* properties instead of addEventListener).

WTF creates a new XHR implementation and replaces the global one.
Zone.js patches only EventTarget (not the XHR) since it assumes that XHR inherits from EventTarget
2a) as a result zone does not patch the WTF-XHR.

So the above is definitely a problem, but it should manifest as a broken VMTurn detection not as broken XHR. So I don't understand why it is a problem.

Oh, what a tangled web we weave...

Dom method interception like these two projects are doing is inherently hacky. It's impressive that they work on their own, but completely unsurprising that they don't compose well. And since they each have different techniques for different browsers, each will probably have its own issues...

That said... I think we can fix this particular case at least in chrome by making the xhr on* handlers enumerable.

This code:

https://github.com/angular/zone.js/blob/d8a7457cb5aefdde8ebaab9e72383d90321defd0/lib/utils.js#L128

iterates over the wtf xhr wrapper to populate the zone xhr wrapper. But the on* (onload, etc) properties are not enumerable, so forwarders don't get setup. This means that when the module loader sets onreadystatechange it never get propagated to the real xhr.

They're populated here:

https://github.com/google/tracing-framework/blob/master/src/wtf/trace/providers/xhrprovider.js#L239

Not sure why they are specified as not enumerable (the native impls are enumerable, apparently), maybe @benvanik remembers.

Note also that we only hit this code path because canPatchViaPropertyDescriptor returns false here:

https://github.com/angular/zone.js/blob/08c7084046f9c9ab8b25d739bbb398be8476cdfe/lib/patch/property-descriptor.js#L9

Making the element on* handlers configurable would be another approach to consider.

Another gotcha that I noticed: zone creates both a xhr wrapper AND modifies EventTarget.prototype, so you end up double-wrapping:

zoneXhr has-a (wtfXhr has-a (nativeXhr is-a (EventTarget, with monkey-patched add/remove eventlistener).

AND wtf implements on* handlers by delegating to add/removeEventListener, so you end up (after making the on* props enumerable) with:

zoneXhr.onload property setter wraps the method with the zone dispatch thing and passes that to wtfXhr.onload setter, which sets nativeXhr.onload, which in turn (via EventTarget.prototype hacking) re-wraps in another zone dispatch thing. Maybe that double is wrapping ok - I don't know. Something to watch out for though.

hah, that's insane 😖

I don't remember why they aren't enumerable - I know for some things I'd use Object.getOwnPropertyDescriptor and see what the browser was saying about its own stuff, but Chrome has historically been really bad about that path so I may have got bad info.

@rsturgell awesome analysis! Thanks do you have an ETA on a fix?

Amazing write-up, @rsturgell 👏

I've made the on* handlers enumerable and configurable in 00dd440

When will this be pushed to the chrome extension?

@mhevery - we do a manual push to the store every once in a while. @benvanik - looks like it's been a while, and there's some good fixes queued up. Mind doing a push?

any update?