Google-api-javascript-client: auth2 instance as thenable may cause issues in Promise-based environments

[peterekjs]

Could anybody explain me reasons why a then method is attached to auth2 instance object, specially when it passes itself as argument into the callback function, as imho it may cause an infinite then recursion trap when connected to Promise constructs?

Let's take an example when I might want for some reasons achieve a true Promise behavior:

(new Promise((resolve, reject) => {
    resolve(
        gapi.auth2.init({...})
    );
    // same as gapi.auth2init({...}).then(authInstance => resolve(authInstance ));
})).then(...);

KABOOM, wonders can happen...

Sure, there is easy workaround by wrapping the content passed to resolve method into some other object, but even though I'm not sure if it is a lucky call to make the instance to be thenable.

P.S. To simulate what can happen, try this in the browser (or rather not):
```javascript
let o = {
then(cb) { cb(o); }
};
Promise.resolve(o).then(i => console.log('no way to get here', i));

[bochunz]

The return value of gapi.auth2.init is not a ES6 Promise, but a goog.Thenable.

[peterekjs]

Then maybe something similar to example below could at least prevent issues that might appear.

let stack = 0;
let o = {
    then(cb) {
        if (stack++) {
            stack = 0;
            throw new Error("Prevented recursive call");
        }
        cb(o);
    }
};
Promise.resolve(o).then(i => console.log('never gets here'));

[renarsvilnis]

Today spent half a day wondering why my application was getting froze on getAuthInstance, till debugged that GoogleAuth is a promise (using is-promise). Thus endless promise loop happens and hangs the browser.

const googleAuth = gapi.auth2.init({
  client_id: '...'
});

function getAuthInstance () {
  return googleAuth.then(() => { googleAuth; });
}

function signIn () {
  return getInstance()
    .then((googleAuth) => googleAuth.signIn())
    .then(onSignIn);
},

I find this polymorphic API design choice kinda frustrating. Presuming that API change is harder can you document the behavior, so others don't fall on the same trap as me and @jPeterek.

For anyone looking for a "solution". I wrapped goolgeAuth in a object in getAuthInstance and desctructed the object in signIn

function getAuthInstance () {
  return googleAuth.then(() => ({googleAuth}));
}

function signIn () {
  return getInstance()
    .then(({googleAuth}) => googleAuth.signIn())
    .then(onSignIn);
},

[TMSCH]

Hi @jPeterek and @renarsvilnis,
This is indeed confusing, we'll look into documenting this method better to prevent this issue.

@renarsvilnis, in the code example you provide, you should not call the signIn method after a promise resolution. Some browsers will prevent the popup from being opened if it is not synchronously opened after a user interaction.

You could use the gapi.auth2.getAuthInstance() method to get the GoogleAuth instance synchronously.

[renarsvilnis]

@TMSCH Thanks for the tip, will try to rewrite so that I allow clicks only once GoogleAuth initiates. Do you remember heart which browsers are those? Mobile or Dekstop? < 9IE?

[TMSCH]

@renarsvilnis I am not able to provide a list of browsers that would fail, it is not a very consistent issue (it can change from version to version). Hence it is safer to not open a popup asynchronously.

[TMSCH]

@jPeterek @renarsvilnis, we have updated the documentation to mention this issue: https://developers.google.com/identity/sign-in/web/reference#googleauththenoninit

[peterekjs]

@TMSCH Thank you!

[TMSCH]

@jPeterek would you mind closing the bug if you think the documentation is enough? We are unlikely to change this behavior as this would break the current API.

[philBrown]

Where has the documentation note gone? The link above no longer navigates to a named anchor :(

[TMSCH]

Here it is: https://developers.google.com/identity/sign-in/web/reference#googleauththenoninit-onerror. We recently updated the doc.

[philBrown]

That's not the only thing you've updated.

What does the then method resolve with now if there is no onInit passed?

[TMSCH]

What do you mean? If there's no onInit passed, it just returns a Promise that resolves with undefined.

[philBrown]

Thanks @TMSCH, that's pretty much what I was after.

Having the GoogleAuth object appear promise-like really makes it difficult to have some promise-based code resolve with it. It would be really nice if you could move the promise part into something like a $promise property like AngularJS does with its ngResource instances but anyway, there are workarounds as mentioned earlier in this thread.

[TMSCH]

@philBrown I agree with you, unfortunately this decision has been made long ago and now developers rely on it. Changing this would break production apps, so we are stuck with it... We're investigating solutions to avoid the infinite loop though, to address part of the issue.