#445 fix causes uncaught promise rejection in chrome

[petewalker]

The fix in #445 causes an uncaught promise rejection in Chrome.

Chrome Version: 53.0.2785.116 (64-bit)

Reproduction:

1. Go to http://plnkr.co/edit/UwHURsu8rchPkTXoveTZ?p=preview
2. Open up Chrome Dev Tools
3. Refresh the page

Expected:

Page refreshes without error

Actual:

Page refresh is caught by uncaught rejection:

[petewalker]

Actually, is this a bug with chrome devtools? The exception does appear to be being caught - but I don't have "Pause On Caught Exceptions" ticked, as per the screenshot.

[jimitndiaye]

Having this same problem on Chrome

[M4ttsson]

Having the same problem, Chrome 53.0.2785.116 m (64-bit).

My angular 2 page works fine in Firefox but gets this error in Chrome:
"Failed to execute 'fetch' on 'Window': 1 argument required, but only 0 present."

[wizarrc]

It looks like chrome may have fixed their "bug"?? where they allowed an empty fetch call, because that did work for me before without hitting a promise rejection breakpoint while in debug mode before. I'm not sure which version of chrome I was on, but I am currently on Version 53.0.2785.116 m (64-bit). My previous fix using an empty string still seems to work in both Edge and IE.

global['fetch']('')

#436 (comment)

[hartsan]

We're having this problem, too, and I can confirm the wizarrc's fix solves the issue.

[wesleycho]

We're seeing this problem in only Edge 38.14393.0.0, where it crashes the script load.

[petewalker]

The problem with @wizarrc's solution - #446 (comment) - is that it introduces a side-effect - a GET request is made to the current page:

[wizarrc]

@petewalker I think the point of the fetch was to confirm functionality. I would be interested in a reliable way to test functionality that did not waste undue resources. I would imagine a local resource would be needed for each browser. Good catch. I want to add I didn't mean to suggest that this was a permanent solution, but rather a quick fix/workaround for those waiting for an update like myself. Nothing is more annoying than trying to test your project and it stops working after trying a different browser/updating npm packages.

Actually, is this a bug with chrome devtools? The exception does appear to be being caught - but I don't have "Pause On Caught Exceptions" ticked, as per the screenshot.

@petewalker It does look like chrome is in fact pausing on any error now without the checkbox selected when debugging tools are open that it used to just send to the console (very annoying). It's possible that it was rejecting the promise without throwing an error unlike the Edge Browser all this time in the past and no one ever noticed. The Edge Browser will not even attempt to fetch with a null parameter and instead immediately throw whereas Chrome will instead return a rejected promise.

[gpazo]

confirming on angular 2.0.0, zone-js 0.6.12, and chrome 53.0.2785.143

[crowmagnumb]

same problem confirmed on angular 2.0.1, zone-js 0.6.12, and chrome 53.0.2785.116

[juliemr]

I don't see this causing issues if 'Pause on Uncaught Exceptions' is unchecked, but I do see the pause if it is checked. Chrome 53.0.2785.143.

for those that are using an older version of zone.js, please try updating to the latest.

[petewalker]

It's occurring for me whether "Pause on Uncaught Exceptions" is checked or not. Chrome 53.0.2785.143, zone.js 0.6.25.

It appears that the Chrome implementation is throwing the error in a way that can't be caught in userland. Running the following in devtools with "Pause on exceptions" activated:

fetch()
fetch().catch(function(){});
fetch().then(function(){}, function(){});
try { fetch().catch(function(){}).then(function(){}, function(){}) } catch (e) { }

All result (for me) with the console pausing with:

Paused on promise rejection: 'TypeError: Failed to execute 'fetch' on 'Window': 1 argument required, but only 0 present.'

[corey-waymark]

Is there any update on this? It is still fantastically annoying to have to press continue every time I load an app with dev tools open...

[teone]

Still having the same issue with:

• Chrome Version 54.0.2840.98 (64-bit)
• Zone.js Version 0.7.2

No error is shown on the console...

Replacing

fetchPromise = global['fetch']();

with

fetchPromise = global['fetch']('');

seem to fix the issue, any help?

[petewalker]

@teone - please see my comment above: #446 (comment)
Whilst that does remove the problem, it introduces a side-effect, sending another GET request to the current page.
Non-essential HTTP requests would be bad for people on mobile data connections, or for pages where accessing them have other side effects.

[VorgaT1339]

Using same setup as teone I can confirm that issue still exists. Very annoying.

[SplayConsulting]

Yes, same issue for me too, very annoying

[petewalker]

Having look at this again, I'm pretty sure the call to fetch is there to allow patching of the .then method with Zone.js's own. To get access to the function to override it, they're making a dummy call to fetch. However, it doesn't look like there's a side-effect free way to do that in Chrome that's catchable when it throws an error. I think this has to be "fixed" upstream in Chrome.

[wizarrc]

@petewalker Do you think it's possible to lazy patch it on first use of fetch? If that were possible, as soon as a fetch is called, do the patch and make the legitimate live fetch call. Not sure how that would work though.

[petewalker]

@wizarrc Potentially. The only simple way I can think to do that would be to create a wrapper method for fetch. i.e.

let NativeFetch = global['fetch'];
global['fetch'] = function(url, init) {
    return NativeFetch(url, init).then(
        // ...
    );
}

I'm not that acquainted with the inner workings of Zone, so I'm not sure what implications something like this would have are. I suspect that by the time the promise is run it's already too late?

---

The really interesting thing is that the patching of the fetched Promise only occurs if the Promise returned by fetch isn't a native browser promise (because if it is, then it's already been overridden). But in Chrome's case (based on my testing), it does. So this code doesn't actually do anything for Chrome anyway.

Having had a quick play in Edge and Firefox, they both also seem to return a native promise - so I'd be curious to learn the circumstances in which this code does fire.

Perhaps polyfills? Or where someone has previously overridden the global Promise? Or are we looking at non-browser implementations of the Fetch API?

The change was introduced here: 5f1ea90

Unfortunately it's not attached to a particular issue, so I'm not sure what its history is...

[wizarrc]

@petewalker I had this problem previously before TypeScript 2.1 when I needed async await support to compile down to es5. In order to unlock this capability, I had to enable babel in the pipeline, which had to be loaded before zone.js. The pipeline was TypeScript (target es6) -> Babel (target es5) -> javascript. I believe the first thing that babel library did was override the Promise implementation (thus why I first received this weird error). To workaround this error during development I modified the zone.js during dev with an environment variable and set the fetch with empty string value. I since was able to remove that (babel and it's polyfill) altogether since TypeScript 2.1 came out and I am able to independently add polyfills when I need browser support and have my much needed async await support.

[esase]

So what exactly should I do for fix that exception?

[MichaelBuerge]

If I understand correctly, the call to fetch() is done to get a hold of its result promise and patch the constructor that created it if needed (in case fetch() is some polyfill using its own promise implementation).
So why not skip this altogether if the implementation of fetch() is native? (Assuming all native implementations of fetch() also return a native Promise, which I think is a reasonable assumption, or isn't it?)
Like so: MichaelBuerge@d94aa44

Fixes the issue for me (...which annoyed me enough to motivate me to do a little digging).
I think this is a clean solution or am I missing something?

[petewalker]

Yep, I believe your understanding is correct. It'd require some testing. I've tried it here in Chrome, Firefox and Safari, and it seems to be consistent.

It's a bit hacky - but until Chrome is changed upstream (if ever), I can't see another way around it.

Your fix isn't quite right, though - you're missing a typeof:

// Are we dealing with a non-native fetch-implementation?
// If so, also patch its result promise constructor.
var fetch = global['fetch'];
if (typeof fetch !== 'undefined' &&
    fetch.toString().indexOf('[native code]') == -1) {
    // ...
}

[MichaelBuerge]

Oh, darn typeof, sorry for that stupid mistake. I'm creating a new change: MichaelBuerge@8e20690

// Are we dealing with a non-native fetch-implementation?
// If so, also patch its result promise constructor.
var fetch = global['fetch'];
if (typeof fetch === 'function' &&
    fetch.toString().indexOf('[native code]') === -1) {
  // ...
}

I agree about it being a bit hacky. However, I do consider patching native constructors to be somewhat hacky to begin with, as it often leads to unexpected problems down the road. As evidenced by this issue, I might add.

Regarding testing: I just tested it manually in lots browsers (Chrome, FF, IE9/10/11, Edge) and didn't observe any problems.
I also ran npm run test and there are indeed 4 failing tests related to fetch. I'll look into that, but I first have to find my way around the test setup.

[MichaelBuerge]

Progress report:
My assumption that all native implementations of fetch() also return a native Promise doesn't hold up in FF. The result returned by fetch() doesn't use the normal Promise constructor.

Details:
I wrote a little probing function that probes the fetch result and writes a bunch of stuff to the console (MichaelBuerge@3133655).

See below for the full output from FF and Chrome. The important bits are (from FF):
NativePromise#then patched: true
fetchResult instanceof NativePromise: false
fetchResult.then == NativePromise.prototype.then: false

Conclusion: It doesn't work as I hoped and expected.
If anyone reading this knows more about FF internals than I do please enlighten me what reason there might have been not to use the normal Promise implementation.

---

FF:
NativePromise#then patched: true
fetchResult instanceof NativePromise: false
fetchResult instanceof ZoneAwarePromise: false
fetchResult.constructor == NativePromise: false
fetchResult.constructor == ZoneAwarePromise: false
fetchResult.constructor: function Promise() { [native code] }
fetchResult.then == NativePromise.prototype.then: false
fetchResult.then: function then() {
[native code]
}
fetchResult.then patched: false

Chrome:
NativePromise#then patched: true
fetchResult instanceof NativePromise: true
fetchResult instanceof ZoneAwarePromise: false
fetchResult.constructor == NativePromise: true
fetchResult.constructor == ZoneAwarePromise: true
fetchResult.constructor: function Promise() { [native code] }
fetchResult.then == NativePromise.prototype.then: true
fetchResult.then: function (onResolve, onReject) {
var nativePromise = this;
return new ZoneAwarePromise(function (resolve, reject) {
NativePromiseThen.call(nativePromise, resolve, reject);
})
.then(onResolve, onReject);
}
fetchResult.then patched: true

[JiaLiPassion]

I think to ignore the error, just unchecked the 'pause on uncaught exception' which described here,
http://stackoverflow.com/questions/32755631/can-i-prevent-that-chrome-v45-pauses-on-promise-rejections.
Not the checkbox, is the 'pause' icon.

[petewalker]

@JiaLiPassion Yes, that's true - but this code shouldn't be throwing an exception at all.

[JiaLiPassion]

@petewalker, yes, current situation is

• in MS edge, fetch() will cause type error because it need a parameter
• in chrome win10, fetch('about:blank') will cause 'fetch API don't support URL schema about:blank'
• in chrome other OS, fetch() will return a rejected promise.

Now it seems chrome dev tool will also pause on rejected promise.
In current situation, I don't know how to modify code to resolve the issue, because fetch API behave differently on different OS/browser.

[MichaelBuerge]

Took another look at this and found a solution.
Created pull request #622

Some noteworthy things I came across investigating this:

• FF: Replacing window.Promise doesn't have an effect on result promises returned by fetch(). FF will still use some internal promise constructor. (That was the reason my first attempt at solving this didn't work.)
• Still in FF: The polyfills used by angular/quickstart (which I used as a base for testing this in an actual Angular project) already replace window.Promise (haven't investigated why exactly that's necessary).
• The karma/selenium test environment also wraps the native promise implementation.

With all this patching on it was quite tricky figuring out what actually is going on.
This prompted me to create a prober to inspect the promise constructor and write a whole lot of output to the console.
I also created a minimal Angular app to employ fetch(). It's live here:
http://orino.ch/test/zone.js-fetch/
It also features the prober and writes a whole lot of output to the console.
Modified angular/quickstart and the prober function can be found here:
https://github.com/MichaelBuerge/quickstart-zone.js
https://github.com/MichaelBuerge/probePromise-zone.js

[petewalker]

See your PR was merged, @MichaelBuerge 👍 Looking forward to seeing this in a release..!

[vwzhang]

I have no problem with the test Plunker but do have problem within vscode. I am using code 1.9.1, zonejs 0.7.6, and chrome 56.0.2924.87. It seemed the problem is solved but I need to know what to do with my code.