Skip to content

Commit

Permalink
[Auth] Fix halted initialization when proactive popup resolver init f…
Browse files Browse the repository at this point in the history
…ails (#5777)

* Fix halted initialization when proactive popup resolver init fails

* Fix lint

* Add changeset
  • Loading branch information
sam-gc committed Dec 2, 2021
1 parent 0818685 commit dc6b447
Show file tree
Hide file tree
Showing 7 changed files with 56 additions and 11 deletions.
5 changes: 5 additions & 0 deletions .changeset/red-actors-care.md
@@ -0,0 +1,5 @@
---
"@firebase/auth": patch
---

Fix errors during Auth initialization when the network is unavailable
5 changes: 4 additions & 1 deletion packages/auth/src/core/auth/auth_impl.ts
Expand Up @@ -135,7 +135,10 @@ export class AuthImpl implements AuthInternal, _FirebaseService {
// Initialize the resolver early if necessary (only applicable to web:
// this will cause the iframe to load immediately in certain cases)
if (this._popupRedirectResolver?._shouldInitProactively) {
await this._popupRedirectResolver._initialize(this);
// If this fails, don't halt auth loading
try {
await this._popupRedirectResolver._initialize(this);
} catch (e) { /* Ignore the error */ }
}

await this.initializeCurrentUser(popupRedirectResolver);
Expand Down
10 changes: 10 additions & 0 deletions packages/auth/src/platform_browser/auth.test.ts
Expand Up @@ -216,6 +216,16 @@ describe('core/auth/initializeAuth', () => {
expect(resolverInternal._initialize).to.have.been.called;
});

it('does not halt init if resolver fails', async () => {
const popupRedirectResolver = makeMockPopupRedirectResolver();
const resolverInternal: PopupRedirectResolverInternal = _getInstance(
popupRedirectResolver
);
sinon.stub(resolverInternal, '_shouldInitProactively').value(true);
sinon.stub(resolverInternal, '_initialize').rejects(new Error());
await expect(initAndWait(inMemoryPersistence, popupRedirectResolver)).not.to.be.rejected;
});

it('reloads non-redirect users', async () => {
sinon
.stub(_getInstance<PersistenceInternal>(inMemoryPersistence), '_get')
Expand Down
9 changes: 8 additions & 1 deletion packages/auth/src/platform_browser/iframe/gapi.test.ts
Expand Up @@ -33,13 +33,14 @@ use(chaiAsPromised);
describe('platform_browser/iframe/gapi', () => {
let library: typeof gapi;
let auth: TestAuth;
let loadJsStub: sinon.SinonStub;
function onJsLoad(globalLoadFnName: string): void {
_window().gapi = library as typeof gapi;
_window()[globalLoadFnName]();
}

beforeEach(async () => {
sinon.stub(js, '_loadJS').callsFake(url => {
loadJsStub = sinon.stub(js, '_loadJS').callsFake(url => {
onJsLoad(url.split('onload=')[1]);
return Promise.resolve(new Event('load'));
});
Expand Down Expand Up @@ -134,4 +135,10 @@ describe('platform_browser/iframe/gapi', () => {
);
expect(_loadGapi(auth)).not.to.eq(firstAttempt);
});

it('rejects if gapi itself does not load', async () => {
const error = new Error();
loadJsStub.rejects(error);
await expect(_loadGapi(auth)).to.be.rejectedWith(error);
});
});
2 changes: 1 addition & 1 deletion packages/auth/src/platform_browser/iframe/gapi.ts
Expand Up @@ -103,7 +103,7 @@ function loadGapi(auth: AuthInternal): Promise<gapi.iframes.Context> {
}
};
// Load GApi loader.
return js._loadJS(`https://apis.google.com/js/api.js?onload=${cbName}`);
return js._loadJS(`https://apis.google.com/js/api.js?onload=${cbName}`).catch(e => reject(e));
}
}).catch(error => {
// Reset cached promise to allow for retrial.
Expand Down
29 changes: 21 additions & 8 deletions packages/auth/src/platform_browser/popup_redirect.test.ts
Expand Up @@ -54,15 +54,26 @@ describe('platform_browser/popup_redirect', () => {
let auth: TestAuth;
let onIframeMessage: (event: GapiAuthEvent) => Promise<void>;
let iframeSendStub: sinon.SinonStub;
let loadGapiStub: sinon.SinonStub;

beforeEach(async () => {
auth = await testAuth();
resolver = new (browserPopupRedirectResolver as SingletonInstantiator<PopupRedirectResolverInternal>)();

sinon.stub(validateOrigin, '_validateOrigin').returns(Promise.resolve());
iframeSendStub = sinon.stub();
loadGapiStub = sinon.stub(gapiLoader, '_loadGapi');
setGapiStub();

sinon.stub(gapiLoader, '_loadGapi').returns(
sinon.stub(authWindow._window(), 'gapi').value({
iframes: {
CROSS_ORIGIN_IFRAMES_FILTER: 'cross-origin-iframes-filter'
}
});
});

function setGapiStub(): void {
loadGapiStub.returns(
Promise.resolve(({
open: () =>
Promise.resolve({
Expand All @@ -74,13 +85,7 @@ describe('platform_browser/popup_redirect', () => {
})
} as unknown) as gapi.iframes.Context)
);

sinon.stub(authWindow._window(), 'gapi').value({
iframes: {
CROSS_ORIGIN_IFRAMES_FILTER: 'cross-origin-iframes-filter'
}
});
});
}

afterEach(() => {
sinon.restore();
Expand Down Expand Up @@ -241,6 +246,14 @@ describe('platform_browser/popup_redirect', () => {
expect(resolver._initialize(secondAuth)).to.eq(secondPromise);
});

it('clears the cache if the initialize fails', async () => {
const error = new Error();
loadGapiStub.rejects(error);
await expect(resolver._initialize(auth)).to.be.rejectedWith(error);
setGapiStub(); // Reset the gapi load stub
await expect(resolver._initialize(auth)).not.to.be.rejected;
});

it('iframe event goes through to the manager', async () => {
const manager = (await resolver._initialize(auth)) as AuthEventManager;
sinon.stub(manager, 'onEvent').returns(true);
Expand Down
7 changes: 7 additions & 0 deletions packages/auth/src/platform_browser/popup_redirect.ts
Expand Up @@ -111,6 +111,13 @@ class BrowserPopupRedirectResolver implements PopupRedirectResolverInternal {

const promise = this.initAndGetManager(auth);
this.eventManagers[key] = { promise };

// If the promise is rejected, the key should be removed so that the
// operation can be retried later.
promise.catch(() => {
delete this.eventManagers[key];
});

return promise;
}

Expand Down

0 comments on commit dc6b447

Please sign in to comment.